ui changes and validations for boundary added #2175
Conversation
📝 WalkthroughWalkthroughThis pull request encompasses updates to the health module's web interface, focusing on styling resources and component improvements. The changes include updating the CSS stylesheet version from Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/AddOrEditMapping.js (2)
Line range hint
83-120: Enhance form validation and user feedback.The MultiSelectDropdown is marked as mandatory but lacks proper validation feedback. Consider adding error states and messages when validation fails.
Here's a suggested implementation:
<MultiSelectDropdown variant="nestedmultiselect" props={{ className: "data-mapping-dropdown" }} t={t} + error={selectedBoundary?.length === 0} + errorMessage={t("PLEASE_SELECT_BOUNDARY")} options={ Object.values( (allSelectedBoundary?.filter((i) => i.type === selectedLevel?.boundaryType) || []).reduce((acc, item) => { // ... existing code }, {}) ) || [] } optionsKey={"code"} selected={selectedBoundary ? selectedBoundary : []} onClose={(value) => { const boundariesInEvent = value?.map((event) => event?.[1]); + if (!boundariesInEvent?.length) { + return; // Prevent setting empty values for mandatory field + } const values = boundariesInEvent?.map((i) => i?.code)?.join(",") const updatedData = { ...newdata, [t(column.name)]: values }; setNewData(updatedData); setSelectedBoundary(boundariesInEvent); }} // ... other props />🧰 Tools
🪛 Biome (1.9.4)
[error] 82-82: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 84-84: A form label must be associated with an input.
Consider adding a
fororhtmlForattribute to the label element or moving the input element to inside the label element.(lint/a11y/noLabelWithoutControl)
Line range hint
19-26: Improve component structure and error handling.Consider the following improvements:
- Extract the boundary transformation logic into a separate utility function
- Add error handling for UUID generation
- Separate data transformation from UI rendering
Here's a suggested implementation:
+const transformBoundaryData = (allSelectedBoundary, selectedLevel) => { + if (!allSelectedBoundary?.length || !selectedLevel) return []; + return Object.values( + allSelectedBoundary + .filter((i) => i.type === selectedLevel.boundaryType) + .reduce((acc, item) => { + const { parent, code, type } = item; + if (!acc[parent]) { + acc[parent] = { code: parent, options: [] }; + } + acc[parent].options.push({ code, type, parent }); + return acc; + }, {}) + ); +}; +const generateUUID = () => { + try { + return crypto.randomUUID(); + } catch (error) { + console.error('Failed to generate UUID:', error); + return Date.now().toString(); // Fallback + } +}; useImperativeHandle(ref, () => ({ getData: () => { - return typeOfOperation === "add" ? {...newdata, editable:true, id: crypto.randomUUID()} : {...newdata, editable:true, id: curData?.id}; + return typeOfOperation === "add" + ? {...newdata, editable: true, id: generateUUID()} + : {...newdata, editable: true, id: curData?.id}; }, }));Then update the MultiSelectDropdown options prop:
-options={ - Object.values( - (allSelectedBoundary?.filter((i) => i.type === selectedLevel?.boundaryType) || []).reduce((acc, item) => { - // ... existing reduction logic - }, {}) - ) || [] -} +options={transformBoundaryData(allSelectedBoundary, selectedLevel)}🧰 Tools
🪛 Biome (1.9.4)
[error] 66-66: A form label must be associated with an input.
Consider adding a
fororhtmlForattribute to the label element or moving the input element to inside the label element.(lint/a11y/noLabelWithoutControl)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.js (1)
Line range hint
1032-1098: Improve UI component consistency and accessibility.The UI changes show inconsistent button variations and styling:
- Some buttons use "secondary" variation while others use "teritiary"
- The popup wrapper width is hardcoded
Consider extracting button variations into constants and using CSS variables for widths:
+ const BUTTON_VARIATIONS = { + SECONDARY: 'secondary', + PRIMARY: 'primary' + }; - variation={"secondary"} + variation={BUTTON_VARIATIONS.SECONDARY} + // In your CSS + :root { + --popup-max-width: 35rem; + } + className={"custom-pop-up-mapping"} + style={{ maxWidth: 'var(--popup-max-width)' }}health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaign.scss (1)
Line range hint
272-295: Add accessibility improvements to the popup.The popup implementation could benefit from accessibility enhancements:
.digit-popup-wrapper.custom-pop-up-mapping { max-width: 35rem; + /* Ensure popup is keyboard navigable */ + &:focus { + outline: 2px solid #0b4b66; + outline-offset: 2px; + } + + /* Add ARIA attributes in the corresponding JSX */ + /* aria-modal="true" */ + /* aria-labelledby="popup-title" */ + /* role="dialog" */ .digit-popup-footer { padding-top: 0 !important;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ 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 (6)
health/micro-ui/web/micro-ui-internals/example/public/index.html(1 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaign.scss(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/AddOrEditMapping.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMapping.js(16 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.js(3 hunks)health/micro-ui/web/public/index.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/AddOrEditMapping.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMapping.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/AddOrEditMapping.js
[error] 66-66: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 84-84: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMapping.js
[error] 50-52: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 556-556: This let declares a variable that is only assigned once.
'validate' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 796-796: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
[error] 815-815: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
[error] 941-941: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
[error] 960-960: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
⏰ Context from checks skipped due to timeout of 10000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (2)
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
15-15: Verify CSS version compatibility.The health CSS package version has been updated to 0.2.40.
Run this script to check for breaking changes:
✅ Verification successful
CSS package version 0.2.40 is properly synchronized ✓
Both HTML files are using the same version of the CSS package, and the package is accessible via unpkg CDN.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for breaking changes in the CSS package # Get the changelog or release notes curl -s "https://unpkg.com/@egovernments/[email protected]/CHANGELOG.md" || \ curl -s "https://raw.githubusercontent.com/egovernments/DIGIT-Frontend/develop/micro-ui/web/micro-ui-internals/packages/css/CHANGELOG.md" # Verify the package exists and is accessible curl -s -o /dev/null -w "%{http_code}" "https://unpkg.com/@egovernments/[email protected]/dist/index.css"Length of output: 469
Script:
#!/bin/bash # Search for other references to the CSS package rg -l "digit-ui-health-css@" --type htmlLength of output: 143
Script:
#!/bin/bash # Check CSS package versions in both files echo "=== health/micro-ui/web/public/index.html ===" rg "digit-ui-health-css@" health/micro-ui/web/public/index.html echo -e "\n=== health/micro-ui/web/micro-ui-internals/example/public/index.html ===" rg "digit-ui-health-css@" health/micro-ui/web/micro-ui-internals/example/public/index.htmlLength of output: 634
health/micro-ui/web/public/index.html (1)
13-13: LGTM! Consistent version update.The health CSS package version update matches the change in example/public/index.html, maintaining consistency across the application.
.../web/micro-ui-internals/packages/modules/campaign-manager/src/components/AddOrEditMapping.js
Show resolved
Hide resolved
...web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.js
Outdated
Show resolved
Hide resolved
...web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMapping.js
Outdated
Show resolved
Hide resolved
...web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMapping.js
Outdated
Show resolved
Hide resolved
...web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadDataMapping.js
Outdated
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaign.scss
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ 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 (2)
health/micro-ui/web/micro-ui-internals/example/public/index.html(1 hunks)health/micro-ui/web/public/index.html(1 hunks)
⏰ Context from checks skipped due to timeout of 10000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
health/micro-ui/web/public/index.html (1)
13-13: Version update is consistent, but clarification needed.The CSS version update to
0.2.42is consistent with the other file. However, the PR objectives mention UI changes and validations for boundary, but we only see CSS version updates in these files.Could you please clarify:
- Where are the boundary validation changes implemented?
- Are there additional files that contain these changes?
Choose the appropriate template for your PR:
Feature PR
Feature Request
JIRA ID
Module
Description
Related Issues
Bugfix PR
Bugfix Request
JIRA ID
Module
Description
Root Cause
Related Issues
Release PR
Summary by CodeRabbit
Style Updates
UI Improvements
Bug Fixes