-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Popup for userAccess boundary area and toast message for assign unassign #1831
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request involve updates to several components within the microplan module. Key modifications include the addition of success toast notifications in the 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range comments (5)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MyMicroplanSearchConfig.js (1)
Line range hint
1-89
: Consider enhancing the search configuration structure.While the current implementation works, there are several opportunities for improvement:
- The commented code should be removed to improve maintainability
- The validation pattern for
microplanName
is commented out but might be useful to prevent invalid inputs- The custom hook name could be moved to a constants file for better maintainability
Consider applying these improvements:
- // THIS CONFIG FOR SUPERVISOR TAGGED USER + // Configuration for supervisor-tagged users const defaultSearchValues = { microplanName: "", }; - // { - // label: "Phone number", - // isMandatory: false, - // key: "Phone number", - // type: "number", - // disable: false, - // populators: { name: "mobileNumber", error: "sample error message", validation: { min: 0, max: 999999999 } }, - // },Also, consider adding input validation:
populators: { name: "microplanName", + validation: { pattern: /^[a-zA-Z0-9\s-_]+$/i }, + error: "MP_INVALID_MICROPLAN_NAME", },health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (2)
Line range hint
233-239
: Consider adding debounce and minimum length validation to searchThe current implementation might trigger unnecessary API calls as it lacks:
- Minimum length validation for search queries
- Debouncing to prevent rapid consecutive API calls
Consider implementing these improvements:
+ const debouncedSearch = Digit.Utils.debounce((query) => { + if (query?.length >= 2 || !query) { + setSearchQuery(query); + } + }, 500); const handleSearch = (query) => { - setSearchQuery(query); + debouncedSearch(query); };
Line range hint
1-324
: Consider architectural improvements for better maintainabilityThe component handles multiple responsibilities (API calls, state management, UI rendering). Consider these architectural improvements:
- Extract API-related logic into a custom hook:
const useUserAccessAPI = (params) => { // Move API calls and related state here return { data, loading, error, actions }; };
- Create a separate component for the data table configuration:
const UserAccessTable = ({ data, onRowAction }) => { // Move columns and table-related logic here return <DataTable {...props} />; };This would improve maintainability and make the code more testable.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FacilityPopup.js (2)
Line range hint
287-292
: Enhance error handling implementationThe current error handling uses a generic message and doesn't utilize the error details from the API response. Consider implementing more specific error handling:
onError: async (result) => { - // setDownloadError(true); setSelectedRows([]); - setShowToast({ key: "error", label: t("ERROR_WHILE_UPDATING_PLANFACILITY"), transitionTime: 5000 }); + const errorMessage = result?.response?.data?.error?.message || t("ERROR_WHILE_UPDATING_PLANFACILITY"); + console.error("Plan facility update failed:", result); + setShowToast({ + key: "error", + label: errorMessage, + transitionTime: 5000 + }); },
Line range hint
391-428
: Enhance accessibility for facility assignment actionsThe selection and assignment actions could be made more accessible to screen reader users and keyboard navigation:
<div className="selection-state-wrapper"> <div className="svg-state-wrapper"> - <SVG.DoneAll width={"1.5rem"} height={"1.5rem"} fill={"#C84C0E"}></SVG.DoneAll> + <SVG.DoneAll + width={"1.5rem"} + height={"1.5rem"} + fill={"#C84C0E"} + aria-hidden="true" + /> <div className={"selected-state"}>{`${selectedRows.length} ${ selectedRows?.length === 1 ? t("MICROPLAN_SELECTED") : t("MICROPLAN_SELECTED_PLURAL") }`}</div> </div> <div className={`table-actions-wrapper`}> <Button className={"campaign-type-alert-button"} variation="secondary" label={ facilityAssignedStatus ? `${t("MICROPLAN_UNASSIGN_FACILITY")} ${details?.additionalDetails?.facilityName}` : `${t("MICROPLAN_ASSIGN_FACILITY")} ${details?.additionalDetails?.facilityName}` } type="button" onClick={handleAssignUnassign} size={"large"} icon={facilityAssignedStatus ? "Close" : "AddIcon"} + aria-label={ + facilityAssignedStatus + ? t("ARIA_UNASSIGN_SELECTED_FACILITIES") + : t("ARIA_ASSIGN_SELECTED_FACILITIES") + } /> </div> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FacilityPopup.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js
(3 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MyMicroplanSearchConfig.js
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FacilityPopup.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MyMicroplanSearchConfig.js (1)
Pattern **/*.js
: check
🔇 Additional comments (3)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MicroplanSearchConfig.js (2)
Line range hint 39-45
: LGTM! Field configuration is properly structured
The search field configuration is well-structured with all necessary properties (label, key, type, and populators) properly defined.
39-39
: Verify the translation key exists
The label has been changed to use the translation key "MP_NAME_OF_MICROPLAN_OPEN_MICROPLANS". Let's ensure this key is properly defined in the translation files to prevent missing translations.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
44-44
: LGTM: Improved state management for row-specific popups
The introduction of chipPopUpRowId
state is a good improvement over the previous boolean state, as it enables better control over popup visibility for individual rows.
...-ui/web/micro-ui-internals/packages/modules/microplan/src/configs/MyMicroplanSearchConfig.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js
Outdated
Show resolved
Hide resolved
...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FacilityPopup.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
Line range hint
233-239
: Optimize search functionalityThe commented-out length check has been removed, which could lead to excessive API calls. Consider implementing debouncing:
+const [debouncedSearchQuery, setDebouncedSearchQuery] = useState(null); + +useEffect(() => { + const timer = setTimeout(() => { + setDebouncedSearchQuery(searchQuery); + }, 300); + return () => clearTimeout(timer); +}, [searchQuery]); + const handleSearch = (query) => { - setSearchQuery(query); + if (query?.length >= 2 || query === '') { + setSearchQuery(query); + } };
📜 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/UserAccess.js
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
Pattern **/*.js
: check
🔇 Additional comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserAccess.js (1)
44-44
: LGTM: Clean state management implementation
The new state variable follows React best practices and provides clear intent for managing row-specific popups.
Popup for userAccess boundary area and toast message for assign unassign
Summary by CodeRabbit
Release Notes
New Features
User Interface Changes
These enhancements aim to improve user experience through better feedback and clearer interface elements.