Summary-screen changes and localization #1672
Conversation
|
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several updates across multiple files, primarily focusing on enhancing user interface components and updating CSS dependencies. Key changes include updating the stylesheet link for the health-related CSS from version Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
🧹 Outside diff range comments (7)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/AssumptionsList.js (1)
Line range hint
8-23: Consider separating data processing logic.The component currently handles both data transformation and presentation. Consider:
- Moving the dictionary creation logic to a separate utility function
- Adding input validation for the assumptions data structure
- Using TypeScript to better define the data structure
Example refactor:
const processAssumptions = (assumptionValues) => { if (!Array.isArray(assumptionValues)) return {}; return assumptionValues.reduce((acc, ob) => { const category = ob?.category || 'NA'; const key = ob?.key || 'NA'; const value = ob?.value || 'NA'; return { ...acc, [category]: [...(acc[category] || []), { [key]: value }] }; }, {}); };🧰 Tools
🪛 Biome
[error] 11-11: This let declares a variable that is only assigned once.
'dic' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/DataMgmtComponent.js (2)
Line range hint
9-17: Remove commented out code.The commented out console.log and data structure example should be removed as they add noise to the codebase and may become outdated.
- // console.log("totalfiles", uploadedFiles); - // { - // "filestoreId": "695334cb-5813-4892-a282-a46c075f0798", - // "resourceId": "8650d05f-92dc-4410-b96b-29b39876f7c5", - // "facilityname": "Population Template (19).xlsx", - // "type": "boundaryWithTarget", - // "inputFileType": "xlsx", - // "templateIdentifier": "Population" - // }
Line range hint
24-67: Reduce code duplication by extracting common logic.The download handler and file name logic is duplicated in both the population and facilities sections. Consider extracting this into a reusable function.
+const renderFileList = (files) => { + return files?.map((item) => { + const fileName = item?.filename ?? `FileNo${item?.filestoreId}`; + return ( + <FileComponent + title="" + fileName={fileName} + downloadHandler={() => { + Digit.Utils.campaign.downloadExcelWithCustomName({ + fileStoreId: item?.filestoreId, + customName: String(fileName) + }); + }} + /> + ); + }); +}; return ( <> <Card style={{marginBottom:"1rem"}}> <HeaderComp title="POPULATION" styles={{ color: "black" }} /> - {uploadedFiles?.map((item) => { - const fileName = item?.filename ? (item?.filename) : (`FileNo${item?.filestoreId}`) - return ( - <FileComponent - title="" - fileName={fileName} - downloadHandler={() => { - Digit.Utils.campaign.downloadExcelWithCustomName({ - fileStoreId: item?.filestoreId, - customName: String(fileName) - }); - }} - /> - ) - })} + {renderFileList(uploadedFiles)} </Card> <Card> <HeaderComp title="FACILITIES" styles={{ color: "black" }} /> - {uploadedFacilities?.map((item) => { - const fileName = item?.filename ? (item?.filename) : (`FileNo${item?.filestoreId}`) - return ( - <FileComponent - title="" - fileName={fileName} - downloadHandler={() => { - Digit.Utils.campaign.downloadExcelWithCustomName({ - fileStoreId: item?.filestoreId, - customName: String(fileName) - }); - }} - /> - ) - })} + {renderFileList(uploadedFacilities)} </Card> </> );health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/subBoundaryView.js (1)
Line range hint
1-93: Consider performance optimizations for the component.While the overall structure is good, here are some suggestions to improve performance and maintainability:
- Memoize the component to prevent unnecessary re-renders
- Extract the chip rendering logic into a separate component
- Move the button styles to a CSS file for better maintainability
Here's how you could implement these improvements:
-import React, { useState } from 'react' +import React, { useState, memo } from 'react' +const ChipList = memo(({ items, t }) => ( + items?.map((el, ind) => ( + <Chip + key={ind} + className="" + error="" + extraStyles={{}} + iconReq="" + hideClose={true} + text={t(el)} + /> + )) +)); -const SubBoundaryView = ({ title, arr, style }) => { +const SubBoundaryView = memo(({ title, arr, style }) => { // ... rest of the component code return ( <div> {arr && arr.length > 0 ? ( <Card type={"secondary"} style={style}> <HeaderComp title={title} /> <div className="digit-tag-container userAccessCell"> - {arr?.slice(0, 10)?.map((el, ind) => { - return ( - <Chip - key={ind} - className="" - error="" - extraStyles={{}} - iconReq="" - hideClose={true} - text={t(el)} - /> - ); - })} + <ChipList items={arr?.slice(0, 10)} t={t} /> {/* ... rest of the code */} </div> </Card> ) : null} </div> ); -} +});Also, consider moving the button styles to a CSS file:
/* microplan.scss */ .more-button { height: 2rem; min-width: 4.188rem; min-height: 2rem; padding: 0.5rem; justify-content: center; align-items: center; } .more-button-text { height: auto; font-size: 0.875rem; font-weight: 400; width: 100%; line-height: 16px; color: #C84C0E; }health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (1)
Line range hint
1-124: Consider improving maintainabilityA few suggestions to improve code maintainability:
- Extract hardcoded strings (labels, paths) into constants
- Add JSDoc or TypeScript type definitions for the configuration object
Example structure:
/** @typedef {Object} ColumnConfig * @property {string} label - The column label key * @property {string} jsonPath - Path to the data * @property {boolean} [additionalCustomization] - Whether custom rendering is needed */ const COLUMN_LABELS = { MICROPLAN_STATUS: 'MICROPLAN_STATUS', CAMPAIGN_DISEASE: 'CAMPAIGN_DISEASE', // ... other labels }; /** @type {ColumnConfig[]} */ const columns = [ { label: COLUMN_LABELS.MICROPLAN_STATUS, // ... } ];health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
Line range hint
27-236: Consider these improvements to enhance code maintainability
- Consider consolidating state management using useReducer for related states (showPopUp, chipPopUp, showToast).
- Extract duplicate Assign button logic into a reusable component.
- Add PropTypes for better type safety and documentation.
Here's a suggested implementation:
import PropTypes from 'prop-types'; // Define prop types UserAccess.propTypes = { category: PropTypes.string.isRequired, setData: PropTypes.func.isRequired, nationalRoles: PropTypes.array.isRequired }; // Extract Assign button component const AssignButton = ({ onClick }) => { const { t } = useTranslation(); return ( <Button variation="secondary" label={t(`ASSIGN`)} icon={"AddIcon"} onClick={onClick} /> ); }; // Simplify state management with reducer const initialState = { showPopUp: false, chipPopUp: false, showToast: null }; function reducer(state, action) { switch (action.type) { case 'SET_POPUP': return { ...state, showPopUp: action.payload }; case 'SET_CHIP_POPUP': return { ...state, chipPopUp: action.payload }; case 'SET_TOAST': return { ...state, showToast: action.payload }; default: return state; } } // In your component: const [state, dispatch] = useReducer(reducer, initialState);
- Simplify Toast type logic:
- type={showToast.key === "error" ? "error" : "success"} + type={showToast.key}🧰 Tools
🪛 Biome
[error] 215-215: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 215-215: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 217-217: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (1)
Line range hint
1-339: Consider code cleanup and standardization.The code could benefit from some general improvements:
- Remove commented-out code blocks if they're no longer needed
- Standardize the use of boolean props across all cards (use actual booleans instead of strings)
- Maintain consistent spacing between sections
Would you like me to provide specific examples of these improvements?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
health/micro-ui/web/micro-ui-internals/packages/css/package.jsonis excluded by!**/*.json
📒 Files selected for processing (14)
- health/micro-ui/web/micro-ui-internals/example/public/index.html (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/AssumptionsList.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/DataMgmtComponent.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FileComponent.js (3 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfigScreen.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaView.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/subBoundaryView.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (4 hunks)
- health/micro-ui/web/public/index.html (1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/AssumptionsList.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/DataMgmtComponent.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FileComponent.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfigScreen.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaView.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/subBoundaryView.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (1)
Pattern
**/*.js: check
📓 Learnings (1)
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (3)
Learnt from: siddhant-nawale-egov PR: egovernments/DIGIT-Frontend#876 File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392 Timestamp: 2024-06-14T14:10:38.086Z Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.Learnt from: siddhant-nawale-egov PR: egovernments/DIGIT-Frontend#876 File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392 Timestamp: 2024-10-08T20:11:07.772Z Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.Learnt from: siddhant-nawale-egov PR: egovernments/DIGIT-Frontend#876 File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392 Timestamp: 2024-10-08T20:11:12.539Z Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/AssumptionsList.js
[error] 11-11: This let declares a variable that is only assigned once.
'dic' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 49-49: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js
[error] 338-341: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 345-348: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
🔇 Additional comments (15)
health/micro-ui/web/micro-ui-internals/example/public/index.html (2)
19-19: 🧹 Nitpick (assertive)Verify the compatibility of the new CSS version.
The CSS version upgrade from 0.1.9 to 0.2.0 might introduce breaking changes. Please ensure all health module UI components render correctly with the new version.
Consider implementing the following improvements for better security and reliability:
- Add SRI (Subresource Integrity) hash:
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> + <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" + integrity_no="sha384-[actual-hash]" + crossorigin="anonymous" />
- Add a fallback in case the CDN fails:
+ <script> + function loadFallbackCSS(e) { + var link = document.createElement('link'); + link.rel = 'stylesheet'; + link.href = '/assets/digit-ui-health.css'; // Local fallback + document.head.appendChild(link); + } + </script> + <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" + onerror="loadFallbackCSS(event)" />
- Add preconnect hint for performance:
+ <link rel="preconnect" href="https://unpkg.com">
19-19: Run the following verification script to check for potential UI issues.✅ Verification successful
CSS version update appears consistent across the codebase
The version update to
[email protected]is already correctly reflected in both the main application (health/micro-ui/web/public/index.html) and the example application (health/micro-ui/web/micro-ui-internals/example/public/index.html). Only the README contains an outdated reference to version 0.1.0, which is documentation and doesn't affect runtime behavior.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any health module UI components that might be affected by CSS changes # Test 1: Look for health-related component files that might need testing echo "Searching for health module components..." fd -e js -e jsx . | rg -i "health.*/(components?|screens?)" # Test 2: Check for any hardcoded CSS version references that need updating echo "Checking for hardcoded CSS version references..." rg -i "[email protected]" # Test 3: Look for custom CSS classes that might conflict echo "Checking for potential CSS conflicts..." rg -i "\.health-"Length of output: 14343
Script:
#!/bin/bash # Let's check for any CSS imports and version references in the codebase # Check for any digit-ui-health-css version references echo "Checking for all digit-ui-health-css version references..." rg -i "digit-ui-health-css@" -A 1 -B 1 # Check for any CSS imports or links in HTML files echo "Checking CSS references in HTML files..." fd -e html -x cat {} | rg -i "link.*rel=\"stylesheet\".*health" # Look for any CSS class usage in health components echo "Checking CSS class usage in health components..." fd -e js -e jsx . | rg -i "health" | xargs rg -i "className.*health"Length of output: 1736
health/micro-ui/web/public/index.html (1)
14-14: Verify compatibility with the new CSS version.The upgrade from
0.1.9to0.2.0is a minor version bump that might introduce breaking changes. While the update aligns with the PR objectives, please ensure:
- The new version is compatible with other CSS dependencies:
- Visual regression testing has been performed to verify no unintended style changes.
Let's verify the changelog and compatibility:
✅ Verification successful
CSS version update appears safe to proceed
The version upgrade from 0.1.9 to 0.2.0 appears to be safe based on the following findings:
- The package.json shows proper version management and dependencies on standard CSS tools (tailwind, postcss).
- The package maintains compatibility with other CSS dependencies:
- The README doesn't indicate any breaking changes, and the package follows semantic versioning.
While no visual regression tests were found, the package's focused scope (health-specific CSS) and proper version management suggest minimal risk of conflicts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for breaking changes in the new CSS version # Test 1: Look for changelog or release notes echo "Checking for changelog or release notes..." fd -g "*CHANGELOG*" -g "*RELEASE*" # Test 2: Check for any visual regression testing configuration echo "Checking for visual regression testing setup..." fd -g "*visual*test*" -g "*regression*test*" -g "*screenshot*test*"Length of output: 572
Script:
#!/bin/bash # Let's try a different approach to find relevant information # Check for package.json files that might reference the CSS package echo "Checking package dependencies..." fd package.json --exec grep -l "@egovernments/digit-ui-health-css" # Look for test configurations echo "Checking for test configurations..." fd -e yaml -e json -e js --exec grep -l "test" {} # Search for visual testing related terms in the codebase echo "Searching for visual testing related code..." rg -i "visual.*(test|check|verify)" -g "!{node_modules,dist,build}/*" # Look for CSS version references echo "Searching for other CSS version references..." rg "@egovernments/digit-ui-health-css" -g "!{node_modules,dist,build}/*"Length of output: 5340
Script:
#!/bin/bash # Let's examine the package.json file we found to check version compatibility echo "Checking CSS package version details..." cat ./health/micro-ui/web/micro-ui-internals/packages/css/package.json # Look for any breaking changes or version information in the README echo "Checking README for version information..." cat ./health/micro-ui/web/micro-ui-internals/packages/css/README.md # Check for other CSS package dependencies echo "Checking other CSS dependencies..." rg "digit-ui.*css" ./health/micro-ui/web/public/index.htmlLength of output: 3299
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaView.js (2)
1-6: LGTM! Clean imports and well-structured prop defaults.The simplified imports and addition of default values for props improve the component's robustness by handling undefined cases gracefully.
6-12: LGTM! Clean component implementation.The component follows React best practices with proper use of translation hook and clean functional component structure.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/subBoundaryView.js (1)
19-19: LGTM! Card styling update is consistent.The Card component update with
type="secondary"and style prop follows the consistent styling approach mentioned in the PR objectives.health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (2)
112-114: 🧹 Nitpick (assertive)Verify hook implementation and consider documenting version strategy
The
customHookNamehas been moved to the tab level. This hook appears to be from a versioned module ("microplanv1").Consider adding a comment explaining:
- Why the hook was moved to tab level
- The versioning strategy for microplan hooks
✅ Verification successful
Based on the shell script results, I can now generate the final response:
Hook implementation is verified and properly versioned
The
useSavedMicroplanshook is:
- Properly exported from
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/useSavedMicroplans.js- Part of a consistent versioning strategy where
microplanv1namespace is used across the codebase- Used in multiple configurations including
MicroplanSearchConfig.jsandMyMicroplanSearchConfig.jsThe hook movement to tab level aligns with the overall architecture where other
microplanv1hooks are used for specific feature implementations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the hook is properly exported and implemented echo "Checking for hook implementation..." rg -A 10 "export.*useSavedMicroplans" --type js # Look for other version references echo "Checking for version references..." rg "microplanv[0-9]+" --type jsLength of output: 14386
82-97: 🛠️ Refactor suggestionVerify custom column implementations and add null checks
The
additionalCustomizationflags suggest custom rendering logic for these columns. Ensure that the corresponding customization implementations in UICustomizations.js handle all possible data scenarios.Consider adding null checks for nested properties. Example:
{ label:"CAMPAIGN_TYPE", - jsonPath:"additionalDetails.campaignType", + jsonPath:"additionalDetails?.campaignType", additionalCustomization:true }✅ Verification successful
Based on the search results, I can see that the custom column implementations for these fields exist in
UICustomizations.jsand proper null checks are already in place. Let me verify the specific implementations with another search:
Custom column implementations are properly handled with null checks
The code in UICustomizations.js shows that all the columns with
additionalCustomization:truehave proper implementations with null value handling:
- Each field (MICROPLAN_STATUS, CAMPAIGN_DISEASE, CAMPAIGN_TYPE, DISTIRBUTION_STRATEGY) has null checks using
if(value && value!="NA")- When the value is null/NA, it returns a div with "NA" text
- When value exists, it properly translates using the translation function
t(value)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the custom column implementations echo "Checking for column customization implementations..." rg -A 10 "additionalCustomization.*MICROPLAN_STATUS|CAMPAIGN_DISEASE|CAMPAIGN_TYPE|DISTIRBUTION_STRATEGY" --type js # Verify the existence of these nested paths in the response data echo "Checking actual API response structure..." rg -A 10 "PlanConfiguration.*disease|campaignType|resourceDistributionStrategyCode" --type jsLength of output: 343043
Script:
#!/bin/bash # Search for the specific customization implementations in UICustomizations.js rg -A 10 "MICROPLAN_STATUS|CAMPAIGN_DISEASE|CAMPAIGN_TYPE|DISTIRBUTION_STRATEGY" health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.jsLength of output: 983
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FileComponent.js (2)
18-18: Verify the existence of 'card-color' CSS class.The Card component uses a "card-color" class which should be defined in the stylesheet.
✅ Verification successful
The 'card-color' CSS class exists and is properly defined
The class is defined in
microplan.scsswith a light gray background color (#FAFAFA), which is appropriate for a secondary card component.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the card-color class definition rg -g "*.scss" -g "*.css" "card-color"Length of output: 134
Script:
#!/bin/bash # Let's check the actual definition of the card-color class rg -g "*.scss" -g "*.css" -A 5 "card-color"Length of output: 630
75-80: 🛠️ Refactor suggestionVerify the 'FileDownload' icon identifier.
The button's icon prop now uses a string identifier instead of a component. While the onClick change aligns with React conventions, we should verify that "FileDownload" is a valid icon identifier.
Consider maintaining consistency with other buttons in the file that use icon components:
-icon={"FileDownload"} +icon={<DownloadIcon styles={{ height: "1.25rem", width: "2.5rem" }} />}✅ Verification successful
The search results show that "FileDownload" is a valid icon identifier that exists in the codebase. It's exported from the svg-components package and is consistently used across multiple components in a similar way. The current usage in FileComponent.js matches the pattern seen in other files like UserUpload.js and UploadDataCustom.js where it's also used as
icon={"FileDownload"}.The 'FileDownload' icon identifier is valid and follows the established pattern.
The change from a component-based approach to using the string identifier "FileDownload" aligns with the codebase's conventions. The refactoring suggestion in the review comment should be withdrawn as it would actually deviate from the consistent pattern used across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of FileDownload icon rg "FileDownload" --type jsLength of output: 10302
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (1)
36-36: LGTM! Background color change maintains consistency.The background color change to #FAFAFA aligns with the card-color class, ensuring visual consistency across containers.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (2)
214-214: LGTM: Improved i18n key specificityThe change to use a dedicated translation key for the popup heading (
${category}_POPUP_HEADING) improves i18n by allowing different translations for the card header and popup heading contexts.
Line range hint
1-3: Verify CSS dependencies and stylingThe PR objectives mention CSS updates, but no direct CSS changes are visible in this file. Let's verify the styling dependencies.
✅ Verification successful
Let me gather more information about the CSS dependencies and their usage.
Based on the gathered information, I can now provide a final response:
CSS dependencies are properly integrated
The file correctly uses CSS classes from the microplanning stylesheet:
- Uses
userAccessCellclass for tag containers- Uses
roleComposerclass for component styling- Imports required components from
@egovernments/digit-ui-componentswhich include their own styles- The microplanning.scss file contains all necessary styling definitions for these classes
The CSS updates mentioned in the PR objectives are properly implemented through the component-level classes and the centralized microplanning stylesheet.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for related CSS files and their changes echo "Searching for related CSS files..." rg -l "userAccessCell|digit-tag-container|roleComposer" --type css # Check for any style-related imports echo "Checking style-related imports..." rg -l "tableCustomStyle|microplan\.scss" --type jsLength of output: 1320
Script:
#!/bin/bash # Check the content of the microplanning.scss file echo "Content of microplanning.scss:" cat health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss # Check how UserAccess.js uses the styles echo -e "\nChecking style usage in UserAccess.js:" rg "className" health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js # Check if there are any style imports in UserAccess.js echo -e "\nChecking style imports in UserAccess.js:" rg "import.*\.scss|import.*style" health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.jsLength of output: 58381
🧰 Tools
🪛 Biome
[error] 215-215: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 215-215: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 217-217: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (2)
180-180: LGTM: Props passing simplified.The simplified props passing removes unnecessary object nesting.
322-322: Verify the default active tab change.The default active tab has been changed from "DATA_MGMT" (card7) to "MICROPLAN_ASSUMPTIONS" (card2). Please confirm if this change in the default navigation flow is intentional and aligns with the user experience requirements.
✅ Verification successful
Let me gather more specific information about the navigation configuration in the microplan module.
Default tab change to "MICROPLAN_ASSUMPTIONS" appears intentional and consistent
The change from "DATA_MGMT" (card7) to "MICROPLAN_ASSUMPTIONS" (card2) as the default active tab appears to be a deliberate UX improvement. The code shows a clear navigation structure where "MICROPLAN_ASSUMPTIONS" is positioned as a core configuration section, making it a logical starting point for users. All referenced cards are properly defined in the configuration with their respective purposes clearly mapped.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any related configuration or documentation about the navigation flow rg -g '!*.{lock,svg,css}' -i "activeByDefault|default.*tab|navigation.*flow"Length of output: 5987
Script:
#!/bin/bash # Search for card configurations and their mappings in the microplan module rg -g '!*.{lock,svg,css}' "card[0-9]" health/micro-ui/web/micro-ui-internals/packages/modules/microplan/ -A 2 -B 2Length of output: 11020
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaView.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaView.js
Show resolved
Hide resolved
...o-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfigScreen.js
Outdated
Show resolved
Hide resolved
...o-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfigScreen.js
Show resolved
Hide resolved
...o-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfigScreen.js
Show resolved
Hide resolved
...icro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js
Show resolved
Hide resolved
...icro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js
Outdated
Show resolved
Hide resolved
...icro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js
Show resolved
Hide resolved
...icro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js
Outdated
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js
Outdated
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js
Outdated
Show resolved
Hide resolved
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js
Outdated
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js
Outdated
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/example/public/index.html
Outdated
Show resolved
Hide resolved
abishekTa-egov
dismissed stale reviews from nabeelmd-eGov and coderabbitai[bot]
via
b467bec
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss
Outdated
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss
Outdated
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (1)
Line range hint
1-321: Clean up commented code and resolve component duplication.
- There are several blocks of commented-out code that should be removed for better maintainability.
- The
FormulaConfigScreencomponent appears in bothcard5andcard8. This duplication might cause confusion or maintenance issues.Consider:
- Removing all commented code blocks
- Consolidating the duplicate
FormulaConfigScreenusage into a single card or clarifying the purpose of each instance through clear naming
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (2 hunks)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
Pattern
**/*.js: checkhealth/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (1)
Pattern
**/*.js: check
🔇 Additional comments (3)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js (2)
99-99: Use boolean instead of string for noCardStyle prop.This issue was previously identified. React properties that represent boolean values should use actual booleans rather than strings.
179-179: Verify FormulaConfigScreen prop handling.The props are now being passed directly as
customPropsinstead of being wrapped in an object. Please ensure that theFormulaConfigScreencomponent is updated to handle this prop structure correctly.✅ Verification successful
Props are correctly handled in FormulaConfigScreen component
The FormulaConfigScreen component is already set up to receive and handle
customPropsdirectly. The component's definition shows it destructurescustomPropsfrom its props and uses it appropriately:const FormulaConfigScreen = ({ customProps }) => { // Uses customProps correctly in useState initialization const [planConfigurations, setPlanConfigurations] = useState(customProps?.sessionData?.FORMULA_CONFIGURATION?.formulaConfiguration?.formulaConfigValues); // ... }🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for FormulaConfigScreen component definition and prop usage ast-grep --pattern 'const FormulaConfigScreen = ({ $_ }) => { $$$ }'Length of output: 7379
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js (1)
141-141: LGTM! Translation implemented as suggested.The change correctly implements the translation for "NA" value as recommended in the previous review.
...cro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/UICustomizations.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range comments (1)
health/micro-ui/web/public/index.html (1)
Line range hint
8-14: Consider improving CSS dependency management.The current implementation has several potential areas for improvement:
- Consider using SRI (Subresource Integrity) hashes for CDN resources
- Consider implementing a fallback mechanism in case the CDN is unavailable
- Version ranges could be used instead of exact versions to ease updates
Example implementation with SRI and fallback:
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" integrity="sha384-HASH" crossorigin="anonymous" onerror="this.onerror=null;this.href='/fallback/health-css.css';" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
health/micro-ui/web/micro-ui-internals/packages/css/package.jsonis excluded by!**/*.json
📒 Files selected for processing (4)
- health/micro-ui/web/micro-ui-internals/example/public/index.html (1 hunks)
- health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (1 hunks)
- health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (2 hunks)
- health/micro-ui/web/public/index.html (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (1)
Pattern
**/*.js: check
📓 Learnings (1)
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (3)
Learnt from: siddhant-nawale-egov PR: egovernments/DIGIT-Frontend#876 File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392 Timestamp: 2024-06-14T14:10:38.086Z Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.Learnt from: siddhant-nawale-egov PR: egovernments/DIGIT-Frontend#876 File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392 Timestamp: 2024-10-08T20:11:07.772Z Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.Learnt from: siddhant-nawale-egov PR: egovernments/DIGIT-Frontend#876 File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392 Timestamp: 2024-10-08T20:11:12.539Z Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.
🔇 Additional comments (11)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
19-19: LGTM: CSS version update is consistent.The update to version 0.1.10 of
@egovernments/digit-ui-health-cssaligns with the team's directive and maintains consistency across files.health/micro-ui/web/public/index.html (1)
14-14: LGTM! Verify the CSS version compatibility.The health CSS module version update from 0.1.9 to 0.1.10 looks good. However, let's verify there are no breaking changes.
✅ Verification successful
Version update is consistent across the codebase
The CSS version update to 0.1.10 is correctly synchronized across all relevant files:
- Both index.html files are using version 0.1.10
- The package.json in the CSS package itself is at version 0.1.10
- No other files need updates
While the README.md shows older version examples (0.1.0), this is just documentation and doesn't affect functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any related CSS version updates in other files # and potential breaking changes in the changelog # Test 1: Look for other files that might need the same version update rg -l "@egovernments/digit-ui-health-css@0\.1\.[0-9]+" # Test 2: Look for any package.json files that might reference this dependency fd "package.json" --exec grep -l "digit-ui-health-css"Length of output: 341
Script:
#!/bin/bash # Let's check the versions in the found files and package.json echo "=== Checking index.html files ===" rg "@egovernments/digit-ui-health-css@" health/micro-ui/web/public/index.html health/micro-ui/web/micro-ui-internals/example/public/index.html echo -e "\n=== Checking package.json ===" cat health/micro-ui/web/micro-ui-internals/packages/css/package.json echo -e "\n=== Checking README.md ===" cat health/micro-ui/web/micro-ui-internals/packages/css/README.mdLength of output: 3320
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss (5)
36-36: LGTM: Background color change improves visual contrast.The new background color (#FAFAFA) provides better visual separation while maintaining a subtle appearance.
131-136: LGTM: Well-structured flex container implementation.The flex layout with space-between and centered alignment provides a clean header structure.
183-185: LGTM with existing suggestions.
191-198: LGTM with existing suggestions.
138-170: 🧹 Nitpick (assertive)Consider adding ARIA roles for better accessibility.
The table-like structure is well-implemented, but could benefit from ARIA roles for better screen reader support.
Add appropriate ARIA roles to improve accessibility:
.as-table-like { display: table; width: 100%; border-collapse: collapse; table-layout: fixed; + role: "table"; } .as-table-row { display: table-row; border-bottom: 1px solid #D6D5D4; + role: "row"; } .as-table-cell { display: table-cell; padding: 10px; vertical-align: top; box-sizing: border-box; + role: "cell"; }Likely invalid or redundant comment.
health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js (4)
885-885:⚠️ Potential issueFix inconsistent spacing in conditional statements
Add a space after
&&to maintain consistent formatting inif(value &&value!="NA"){.Apply this diff:
-if(value &&value!="NA"){ +if(value && value!="NA"){Likely invalid or redundant comment.
896-896:⚠️ Potential issueFix inconsistent spacing in conditional statements
Ensure consistent spacing by adding a space after
&&in the conditionif(value &&value!="NA"){.Apply this diff:
-if(value &&value!="NA"){ +if(value && value!="NA"){Likely invalid or redundant comment.
885-885:⚠️ Potential issueUse strict equality operator
'!=='Ensure consistent use of strict inequality operators by replacing
'!='with'!=='in the conditionif(value && value!="NA"){.Apply this diff:
-if(value && value!="NA"){ +if(value && value!=="NA"){Likely invalid or redundant comment.
896-896:⚠️ Potential issueUse strict equality operator
'!=='Update the condition to use
'!=='for strict comparison, preventing type coercion issues.Apply this diff:
-if(value && value!="NA"){ +if(value && value!=="NA"){Likely invalid or redundant comment.
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js (1)
Pattern
**/*.js: check
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js
[error] 338-341: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 345-348: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
...icro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js
Show resolved
Hide resolved
Made summary screen datamgmt , and css changes, localiztions in user-access-roles
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style
Chores