FEATURE/HCMPRE-1709: Accessibility Dropdown #2087
Conversation
📝 WalkthroughWalkthroughThis pull request introduces updates to the health module's UI components, focusing on enhancing filter functionality and updating stylesheet versions. The changes include updating the CSS stylesheet version in multiple HTML files, adding a new CSS class for dropdown spacing, and refactoring the Changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)Pattern 📓 Learnings (1)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)🪛 Biome (1.9.4)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js[error] 59-59: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). (lint/suspicious/noPrototypeBuiltins) [error] 58-58: This let declares a variable that is only assigned once. 'key' is never reassigned. Safe fix: Use const instead. (lint/style/useConst) [error] 151-151: 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. (lint/correctness/useJsxKeyInIterable) 🔇 Additional comments (2)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (2)
Remove commented-out code if no longer needed - // setSelectedValue(selectedValue); // Clear the selection
Use const and avoid Object.prototype.hasOwnProperty Below is an example fix: -for (let key in filterValues) {
- if (filterValues[key] && typeof filterValues[key] === 'object' && filterValues[key].hasOwnProperty('code')) {
+for (const key in filterValues) {
+ if (filterValues[key] && typeof filterValues[key] === 'object' && Object.hasOwn(filterValues[key], 'code')) {
filtersToApply[key] = filterValues[key].code;
} else {
filtersToApply[key] = filterValues[key];
}
}⛔ Skipped due to learnings🧰 Tools🪛 Biome (1.9.4)[error] 59-59: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). (lint/suspicious/noPrototypeBuiltins) [error] 58-58: This let declares a variable that is only assigned once. 'key' is never reassigned. Safe fix: Use const instead. (lint/style/useConst) 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 (
|
abishekTa-egov
changed the title
![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 (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Line range hint
497-509: Duplicated conditional logic
Similar logic checking ifselectedFilter.status === "VALIDATED"appears in multiple places. Consider refactoring into a helper function to avoid duplication.
📜 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 (5)
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(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js(3 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js(15 hunks)health/micro-ui/web/public/index.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
Pattern **/*.js: check
📓 Learnings (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1847
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js:308-314
Timestamp: 2024-11-18T04:35:51.535Z
Learning: In `PlanInbox.js`, the variable `planWithCensus?.StatusCount[selectedFilter]` is always defined in the given context, so null checks are unnecessary.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1562
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js:57-57
Timestamp: 2024-11-12T10:40:17.779Z
Learning: In the `InboxFilterWrapper` component, `optionsKey` should always be set to `"name"` because options are transformed and created within the component.
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
[error] 375-375: 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)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
[error] 141-141: 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)
🔇 Additional comments (12)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (3)
3-4: Adherence to the retrieved learnings about optionsKey looks good
The usage of "name" as the optionKey on the RadioButtons and Dropdown ensures consistency with the local transformation of options.
58-58: Good use of filterValues when applying filters
Passing the entire filterValues object to the parent (props.onApplyFilters) aligns with the updated multi-filter approach.
64-65: Appropriate reset of filterValues in clearFilters
The reset logic correctly clears all filter properties, ensuring a consistent default filter state.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (7)
36-36: Initialization of selectedFilter with multiple properties
Using an object structure for filters allows for better scalability and organization of multiple filtering criteria.
Line range hint 123-135: Status-based UI handling
The logic to hide or show tabs based on selectedFilter.status especially for "VALIDATED" is clear. Ensure that any future statuses also get appropriately handled here.
174-178: Passing new filter fields to usePlanSearchWithCensus
Expanding the filter fields (onRoadCondition, terrain, securityQ1, securityQ2) ensures comprehensive filtering. Confirm that these parameter names match the backend expectation.
213-216: Consistent usage of filter properties
The object destructuring approach for including new fields (terrain, onRoadCondition, etc.) keeps the data consistent with the updated selectedFilter.
384-393: Validate the selected state in workflow
Ensure that the selectedFilter.status is present in the workflowData?.states array to avoid runtime errors if an unexpected status is set.
511-517: Combining filter updates
Merging the incoming filterValue with the existing prev object ensures incremental updates. This approach is well-suited for multi-filter criteria.
531-536: Safe handling of activeFilter when clearing
Setting selectedFilter.status to the first key of activeFilter is reasonable. However, ensure activeFilter always has at least one key to avoid errors.
health/micro-ui/web/micro-ui-internals/example/public/index.html (1)
15-15: Version update of the CSS stylesheet
The update from 0.2.32 to 0.2.33 is straightforward and presumably includes minor style adjustments for the health UI.
health/micro-ui/web/public/index.html (1)
13-13: CSS version alignment across the codebase
Ensuring that all references to the digit-ui-health-css stylesheet use the same version (0.2.33) supports predictable styling across all environments.
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
Outdated
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
Outdated
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
Outdated
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
Outdated
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
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: 3
♻️ Duplicate comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
123-129: 🛠️ Refactor suggestionEnsure consistent optionsKey usage across dropdowns.
Based on previous learnings,
optionsKeyshould be set to "name" consistently across all dropdowns. Currently, some dropdowns use "code" which could lead to inconsistent behavior.-optionKey={"code"} +optionKey={"name"}Also applies to: 135-141, 157-163
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
Pattern **/*.js: check
📓 Learnings (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1562
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js:57-57
Timestamp: 2024-11-12T10:40:17.779Z
Learning: In the `InboxFilterWrapper` component, `optionsKey` should always be set to `"name"` because options are transformed and created within the component.
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
[error] 62-62: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 61-61: This let declares a variable that is only assigned once.
'key' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 154-154: 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)
🔇 Additional comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
145-166:
Add missing key prop to mapped components.
React requires a unique key prop for elements in an array to optimize rendering and maintain component state correctly.
return (
- <LabelFieldPair vertical>
+ <LabelFieldPair vertical key={`security-question-${index}`}>
<TextBlock body={t(`MP_SECURITY_QUESTION ${index + 1}`)} />Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 154-154: 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)
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
Outdated
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
Outdated
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
Outdated
Show resolved
Hide resolved
…/DIGIT-Frontend into FEATURE/HCMPRE-1709
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (2)
Line range hint
123-135: Watch out for repeated toggle logic
This useEffect toggles the tab whenstatusis "VALIDATED". Notice that similar logic also appears later (lines 497-509), potentially duplicating behavior.
Line range hint
497-509: Consolidate duplicate validation logic
This block repeats the “VALIDATED” handling logic seen at lines 123-135. Consider extracting into a helper function or consolidating the checks to avoid invocation errors and ease maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js(4 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js(15 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Pattern **/*.js: check
📓 Learnings (2)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (1)
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1562
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js:57-57
Timestamp: 2024-11-12T10:40:17.779Z
Learning: In the `InboxFilterWrapper` component, `optionsKey` should always be set to `"name"` because options are transformed and created within the component.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1847
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js:308-314
Timestamp: 2024-11-18T04:35:51.535Z
Learning: In `PlanInbox.js`, the variable `planWithCensus?.StatusCount[selectedFilter]` is always defined in the given context, so null checks are unnecessary.
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
[error] 59-59: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 58-58: This let declares a variable that is only assigned once.
'key' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 151-151: 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/PlanInbox.js
[error] 375-375: 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)
🔇 Additional comments (12)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (9)
36-36: Initialize multi-field filter object
Storing multiple filter criteria in a single state object is a neat approach for managing them collectively.
375-375: Simplify the ternary expression
You can replace selectedFilter?.status ? true : false with !!selectedFilter?.status for succinctness.
🧰 Tools
🪛 Biome (1.9.4)
[error] 375-375: 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)
393-393: Fallback assignment looks good
Using availableActions || [] ensures no null references and maintains a consistent list structure.
430-430: Safe default for assigned count
Using planWithCensus?.StatusCount[selectedFilter?.status] || 0 gracefully handles missing map keys. This is a reasonable fallback behavior.
438-438: Dependency array
Updating on changes to planWithCensus, selectedFilter?.status, and activeLink ensures the effect re-runs appropriately.
444-444: Re-fetch conditions
Watching selectedFilter?.status, activeLink, censusJurisdiction, and limitAndOffset is logical to refresh data upon these changes.
511-517: Merging filters
Merging new filter properties into previous state ensures incremental updates without losing other fields. Good approach.
531-536: Verify partial filter clearing
By only resetting the status to the first active key, other fields like onRoadCondition or terrain remain set. If the intention is to clear all fields, consider resetting the entire filter object.
807-813: Consider a more scalable approach to status-action checks
As more statuses or actions may be introduced, a configuration-driven or map-based approach could reduce repeated conditionals. This has been noted previously.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js (3)
79-84: Implementation of handleDropdownChange is clear and concise
This function effectively updates filter values for multiple dropdowns. The approach is modular and easy to maintain.
151-151: Add a unique key prop to the mapped <LabelFieldPair> element
A unique key improves React’s rendering efficiency and prevents potential reordering issues. This comment mirrors a prior suggestion, so it is marked as a duplicate.
- <LabelFieldPair vertical>
+ <LabelFieldPair vertical key={index}>
...
</LabelFieldPair>🧰 Tools
🪛 Biome (1.9.4)
[error] 151-151: 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)
58-59: 🛠️ Refactor suggestion
Use const and Object.hasOwn() instead of let and hasOwnProperty
Declaring key as let is unnecessary since it's never reassigned, and calling hasOwnProperty directly can be replaced with Object.hasOwn() to avoid potential prototype pollution hazards.
- for (let key in filterValues) {
- if (filterValues[key] && typeof filterValues[key] === 'object' && filterValues[key].hasOwnProperty('name')) {
+ for (const key in filterValues) {
+ if (filterValues[key] && typeof filterValues[key] === 'object' && Object.hasOwn(filterValues[key], 'name')) {
filtersToApply[key] = filterValues[key].name;
} else {
filtersToApply[key] = filterValues[key];
}
}⛔ Skipped due to learnings
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1562
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js:57-57
Timestamp: 2024-11-12T10:40:17.779Z
Learning: In the `InboxFilterWrapper` component, `optionsKey` should always be set to `"name"` because options are transformed and created within the component.
🧰 Tools
🪛 Biome (1.9.4)
[error] 59-59: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 58-58: This let declares a variable that is only assigned once.
'key' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
Show resolved
Hide resolved
...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/InboxFilterWrapper.js
Show resolved
Hide resolved
FEATURE/HCMPRE-1709: For Single Select Dropdowns(Village road condition, Village terrain, Civil unrest occurrence, Security forces patrolling) + integrate with MDMS to show list of accessibility details
Summary by CodeRabbit
Release Notes
New Features
Styling
.gap-between-dropdownsclass for better dropdown layout.Bug Fixes