Go Back button and microplan name quotes fix #1844
Conversation
📝 WalkthroughWalkthroughThe pull request introduces several modifications across multiple components within the microplan module. Key changes include adjustments to string interpolation in the rendering of various labels, specifically removing unnecessary quotation marks. Additionally, 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range comments (5)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/ActivityCard.js (1)
Line range hint
23-35: Consider these improvements for better maintainability and accessibility.
- Move inline styles to CSS classes
- Extract click handler logic to a separate function
- Add ARIA attributes for better accessibility of disabled states
Example refactor:
-<Card - className={`activity-card ${i.disable ? "disabled-activity" : ""}`} - style={{ - minWidth: "100px", - cursor: i.disable ? "not-allowed" : "pointer", - opacity: i.disable ? 0.8 : 1, - justifyContent: "center", - alignItems: "center", - height: "200px", - }} - onClick={() => { - if (!i.disable) { - onClickCard(i) - i.link ? history.push(i.link) : location.assign(i.locate); - } - }} +<Card + className={`activity-card ${i.disable ? "disabled-activity" : ""}`} + aria-disabled={i.disable} + role="button" + onClick={handleCardClick(i)}Add corresponding CSS:
.activity-card { min-width: 100px; height: 200px; display: flex; justify-content: center; align-items: center; cursor: pointer; } .activity-card.disabled-activity { cursor: not-allowed; opacity: 0.8; }And the click handler:
const handleCardClick = (item) => () => { if (item.disable) return; onClickCard(item); item.link ? history.push(item.link) : location.assign(item.locate); };health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (3)
Line range hint
23-35: Consider consolidating related state variables.Multiple related state variables could be combined into a single state object for better state management:
-const [showAccessbilityPopup, setShowAccessbilityPopup] = useState(false); -const [showSecurityPopup, setShowSecurityPopup] = useState(false); -const [showToast, setShowToast] = useState(null); -const [showEditVillagePopulationPopup, setShowEditVillagePopulationPopup] = useState(false); -const [showCommentLogPopup, setShowCommentLogPopup] = useState(false); -const [showComment, setShowComment] = useState(false); +const [uiState, setUiState] = useState({ + showAccessbilityPopup: false, + showSecurityPopup: false, + showToast: null, + showEditVillagePopulationPopup: false, + showCommentLogPopup: false, + showComment: false +});
Line range hint
71-108: Consider improving error handling for API calls.The component makes multiple API calls but doesn't handle error states comprehensively. Consider adding error boundaries or error states for better user experience.
const { data: planEmployeeDetailsData, isLoading: isLoadingPlanEmployee } = Digit.Hooks.microplanv1.usePlanSearchEmployee({ tenantId: tenantId, body: { PlanEmployeeAssignmentSearchCriteria: { tenantId: tenantId, planConfigurationId: microplanId, employeeId: [userInfo.info.uuid], role: ["POPULATION_DATA_APPROVER", "ROOT_POPULATION_DATA_APPROVER"] }, }, config: { enabled: true, select: (data) => { return data?.PlanEmployeeAssignment?.[0]; }, + onError: (error) => { + setUiState(prev => ({ + ...prev, + showToast: { + key: "error", + label: t("HCM_MICROPLAN_ERROR_FETCHING_EMPLOYEE_DETAILS"), + transitionTime: 5000 + } + })); + } }, });
Line range hint
1-400: Consider extracting string literals as constants.There are several repeated string literals (especially translation keys) that could be extracted as constants at the top of the file for better maintainability.
+const TRANSLATION_KEYS = { + MICROPLAN_NAME: "HCM_MICROPLAN_MICROPLAN_NAME_LABEL", + NO_NAME: "NO_NAME_AVAILABLE", + SECURITY_HEADING: "HCM_MICROPLAN_SECURITY_AND_ACCESSIBILITY_HEADING", + // ... other translation keys +}; + +const API_ENDPOINTS = { + CENSUS_SEARCH: "/census-service/_search", + CENSUS_UPDATE: "/census-service/_update", + // ... other endpoints +};health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
Line range hint
1-594: Consider component refactoring for improved maintainability.The component handles multiple responsibilities and could benefit from the following improvements:
- Break down into smaller components for better separation of concerns:
- Workflow management
- Data fetching logic
- UI rendering
- Group related state variables using a reducer or custom hooks
- Extract complex conditions into helper functions for better readability
Example of extracting complex conditions:
+const shouldShowActionBar = (isRootApprover, totalStatusCount, planStatus) => { + return isRootApprover && + isStatusConditionMet(totalStatusCount) && + planStatus === "CENSUS_DATA_APPROVAL_IN_PROGRESS"; +}; {/* Replace complex condition */} -{isRootApprover && isStatusConditionMet(totalStatusCount) && planObject?.status === "CENSUS_DATA_APPROVAL_IN_PROGRESS" && +{shouldShowActionBar(isRootApprover, totalStatusCount, planObject?.status) && <ActionBar actionFields={[ <Button icon="CheckCircle" label={t(`HCM_MICROPLAN_FINALIZE_POPULATION_DATA`)} onClick={handleActionBarClick} type="button" variation="primary" />, ]} className="" maxActionFieldsAllowed={5} setactionFieldsToRight sortActionFields style={{}} /> }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (5)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/ActivityCard.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js(3 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/ActivityCard.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js (1)
Pattern **/*.js: check
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/pages/employee/PopInbox.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (1)
Pattern **/*.js: check
📓 Learnings (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (2)
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1832
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js:592-594
Timestamp: 2024-11-14T12:48:23.024Z
Learning: In `PopInbox.js`, when integrating the `ConfirmationPopUp` component, error handling for API failures is managed internally within the component, so explicitly passing an `onError` prop is not necessary.
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1837
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js:559-561
Timestamp: 2024-11-15T10:15:57.370Z
Learning: In `PopInbox.js`, error handling for the `onError` callback in `WorkflowCommentPopUp` is handled within the component, so explicit error handling in the parent component is unnecessary.
🪛 Biome
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
[error] 187-187: 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 (5)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/ActivityCard.js (1)
16-16: LGTM! String interpolation change looks good.
The template literal correctly displays the microplan name with proper fallback handling for undefined values. This change aligns with the PR objective of fixing microplan name quotes.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js (1)
159-160: LGTM: Clean string interpolation implementation
The template literal implementation correctly displays the microplan name with fallback.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js (1)
224-224: LGTM! String interpolation fix looks good.
The template literal syntax for displaying the campaign name is correct and includes proper fallback handling.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PopInbox.js (1)
513-513: LGTM! Improved string interpolation.
The removal of unnecessary quotes around the microplan name display improves readability without affecting functionality.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/PlanInbox.js (1)
532-532: LGTM! String interpolation improvement.
The simplified string interpolation enhances code clarity by removing unnecessary quotation marks.
Let's verify similar string interpolations across the microplan module for consistency:
✅ Verification successful
Based on the search results, I can see that the string interpolation pattern HCM_MICROPLAN_MICROPLAN_NAME_LABEL is consistently used across multiple files in the microplan module, including:
- ActivityCard.js:
${t("HCM_MICROPLAN_MICROPLAN_NAME_LABEL")}: ${props.microplanName || t("NO_NAME_AVAILABLE")} - viewVillage.js:
${t("HCM_MICROPLAN_MICROPLAN_NAME_LABEL")}: ${campaignObject?.campaignName || t("NO_NAME_AVAILABLE")} - PopInbox.js:
${t("HCM_MICROPLAN_MICROPLAN_NAME_LABEL")}: ${planObject?.name || t("NO_NAME_AVAILABLE")} - FacilityCatchmentMapping.js:
${t("HCM_MICROPLAN_MICROPLAN_NAME_LABEL")}: ${planObject?.name || t("NO_NAME_AVAILABLE")}
The string interpolation pattern in the review comment matches the consistent pattern used throughout the codebase. Therefore, I can conclude that this change aligns with the established conventions.
String interpolation follows codebase conventions
The template literal usage for displaying the microplan name is consistent with similar implementations across the microplan module.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar string interpolation patterns in microplan module
rg -U "{\`.*?MICROPLAN.*?LABEL.*?\`}" "health/micro-ui/web/micro-ui-internals/packages/modules/microplan/"
# Search for potential inconsistent string interpolations with quotes
rg -U "'{\`.*?MICROPLAN.*?LABEL.*?\`}'" "health/micro-ui/web/micro-ui-internals/packages/modules/microplan/"
Length of output: 434
Script:
#!/bin/bash
# Search for similar string interpolation patterns in microplan module with fixed regex
rg -U "\{`.*MICROPLAN.*LABEL.*`\}" "health/micro-ui/web/micro-ui-internals/packages/modules/microplan/"
# Search for potential inconsistent string interpolations with quotes (fixed regex)
rg -U "'\{`.*MICROPLAN.*LABEL.*`\}'" "health/micro-ui/web/micro-ui-internals/packages/modules/microplan/"
# Alternative search using plain text patterns
rg "HCM_MICROPLAN" "health/micro-ui/web/micro-ui-internals/packages/modules/microplan/" -A 1 -B 1
Length of output: 71050
...micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
Show resolved
Hide resolved
...micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
Show resolved
Hide resolved
...micro-ui-internals/packages/modules/microplan/src/pages/employee/FacilityCatchmentMapping.js
Show resolved
Hide resolved
Summary by CodeRabbit