BUGFIX/HCMPRE-1776 : Added UI/UX Master changes #2110
Conversation
📝 WalkthroughWalkthroughThis pull request introduces several changes across the health micro-UI project, primarily focusing on two main aspects: updating CSS stylesheets and replacing the Changes
Sequence DiagramsequenceDiagram
participant UI as User Interface
participant TC as TagComponent
participant Tag as Original Tag Component
UI->>Tag: Previously used with multiple props
UI->>TC: Now uses simplified prop (campaignName)
TC->>Tag: Internally renders Tag with predefined properties
Note over UI,TC: Standardized tag rendering across components
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: 9
🔭 Outside diff range comments (18)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/SelectingBoundariesDuplicate.js (1)
Line range hint
1-152: Consider implementing error boundaries for TagComponent.Since TagComponent is a new addition, it would be good practice to wrap it with error boundaries to gracefully handle any potential rendering errors.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignSummary.js (3)
Line range hint
376-412: Improved code structure with conditional rendering.The refactoring from commented code to active conditional rendering improves code maintainability. However, the edit functionality needs keyboard accessibility.
Add keyboard support to the edit container:
<div className="campaign-preview-edit-container" onClick={() => handleRedirect(4)} + onKeyDown={(e) => e.key === 'Enter' && handleRedirect(4)} + role="button" + tabIndex={0} >
Line range hint
1-694: Consider breaking down the component for better maintainability.The component is quite large (694 lines) and handles multiple responsibilities including data fetching, transformation, and rendering.
Consider:
- Extract the data transformation logic (e.g.,
reverseDeliveryRemap,boundaryDataGrp) into separate utility files- Split the view composition logic into smaller components
- Move the data fetching logic into a custom hook
Would you like me to help create a detailed refactoring plan?
Line range hint
32-38: Consider usingreducefor better performance infetchResourceFile.The current implementation uses array methods that create intermediate arrays.
Consider using
reducefor better performance:const fetchResourceFile = async (tenantId, resourceIdArr) => { const res = await Digit.CustomService.getResponse({ url: `/project-factory/v1/data/_search`, body: { SearchCriteria: { tenantId: tenantId, - id: resourceIdArr, + id: resourceIdArr.reduce((acc, id) => [...acc, id], []), }, }, }); return res?.ResourceDetails; };health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateChecklist.js (4)
Line range hint
4-4: Remove unusedTagimportSince
Taghas been replaced withTagComponent, theTagimport from@egovernments/digit-ui-componentsis no longer needed.-import { ViewCardFieldPair, Toast, Card, TextBlock, Button, PopUp, CardText, TextInput, BreadCrumb, Loader, ActionBar, Tag } from "@egovernments/digit-ui-components"; +import { ViewCardFieldPair, Toast, Card, TextBlock, Button, PopUp, CardText, TextInput, BreadCrumb, Loader, ActionBar } from "@egovernments/digit-ui-components";Also applies to: 8-8
Line range hint
271-272: Add error handling for crypto.randomUUID()The
crypto.randomUUID()call could fail in unsupported browsers. Consider adding a fallback mechanism.+const generateUUID = () => { + try { + return crypto.randomUUID(); + } catch (error) { + // Fallback to a simple timestamp-based unique ID + return `${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; + } +}; - key: crypto.randomUUID(), // Using crypto.randomUUID() for a unique key + key: generateUUID(), // Using a browser-compatible UUID generatorAlso applies to: 273-274, 275-276
Line range hint
271-397: Consider breaking down the complexgenerateCodesfunctionThe
generateCodesfunction is quite complex with nested logic for handling questions, options, and subquestions. Consider splitting it into smaller, more focused functions for better maintainability.Suggested structure:
const generateQuestionCode = (question, index) => { // Handle single question code generation }; const generateOptionCodes = (options, questionCode) => { // Handle options code generation }; const generateSubQuestionCodes = (subQuestions, prefix, parentCounter) => { // Handle subquestions code generation }; const generateCodes = (questions) => { // Orchestrate the code generation using the helper functions };
Line range hint
10-58: Consider extracting form logic into a custom hookThe component has complex form state management with multiple
useStatehooks and effects. Consider extracting this logic into a custom hook for better reusability and maintainability.Example structure:
const useChecklistForm = (initialData) => { const [config, setConfig] = useState(null); const [checklistTypeCode, setChecklistTypeCode] = useState(null); // ... other form-related state // Form-related effects and handlers return { config, checklistTypeCode, // ... other form-related state and handlers }; };health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundaryDetailsSummary.js (1)
Line range hint
103-107: Consider consistent margin handling across views.The main view has
margin: "0rem"for the tag container while the PopUp view doesn't specify any margin. Consider maintaining consistent spacing or documenting why they differ.- <div className="digit-tag-container" style={{ display: "flex", maxWidth: "100%" }}> + <div className="digit-tag-container" style={{ display: "flex", maxWidth: "100%", margin: "0rem" }}>health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateDatesWithBoundaries.js (1)
Line range hint
89-99: Clean up commented codeThere are multiple blocks of commented-out code that should be removed to improve code maintainability.
- // if (endSecond < 59) { - // return { - // ...item, - // endDate: itemEndDate + 1000, - // }; - // } else { - // return { - // ...item, - // endDate: itemEndDate - 1000, - // }; - // }health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SearchChecklist.js (2)
Line range hint
21-23: Fix potential memory leak in useEffectThe useEffect dependency array is incorrect. It's using campaignName as both the dependency and the state update value, which could cause unnecessary re-renders.
- useEffect(() => { - setCampaignName(campaignName); - }, campaignName); + useEffect(() => { + const name = searchParams.get("name"); + if (name !== campaignName) { + setCampaignName(name); + } + }, [searchParams, campaignName]);
Line range hint
126-137: Remove commented-out stepper codeLarge blocks of commented-out code should be removed to improve code maintainability.
Remove the entire commented-out Stepper component block if it's no longer needed.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DataUploadSummary.js (1)
Line range hint
11-33: Clean up commented code.Remove the commented-out version of
boundaryDataGrpfunction as it's no longer needed and the new implementation is already in use.health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/CycleConfiguration.js (1)
Line range hint
221-258: Clean up commented out InfoCard component.Remove the commented-out
InfoCardcomponent as it's no longer needed.health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignUpdateSummary.js (1)
Line range hint
127-196: Clean up commented out useEffect code.Remove the large block of commented-out
useEffectcode as it's no longer needed.health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignDates.js (3)
Line range hint
9-10: Consider extracting date constants and utilities.The magic number calculation for ONE_DAY_IN_MS could be more maintainable if extracted to a constants file or utility function.
-const ONE_DAY_IN_MS = 24 * 60 * 60 * 1000; +const MILLISECONDS_PER_DAY = 24 * 60 * 60 * 1000; +const ONE_DAY_IN_MS = MILLISECONDS_PER_DAY;
Line range hint
89-94: Document the purpose of execution count limit.The execution count limit of 5 seems arbitrary. Consider documenting why this limit exists or refactor to remove the need for counting executions.
useEffect(() => { + // Limits the number of initial date selections to prevent infinite loops if (executionCount < 5) { onSelect("campaignDates", { startDate: startDate, endDate: endDate }); setExecutionCount((prevCount) => prevCount + 1); } });
Line range hint
96-112: Simplify stepper navigation logic.The nested if conditions in
onStepClickcould be simplified using a switch statement or lookup object for better maintainability.- const onStepClick = (currentStep) => { - if (!props?.props?.sessionData?.HCM_CAMPAIGN_NAME || !props?.props?.sessionData?.HCM_CAMPAIGN_TYPE) return; - if(currentStep === 0){ - setKey(1); - } - else if(currentStep === 1){ - setKey(2); - } - else if(currentStep === 3){ - if (!props?.props?.sessionData?.HCM_CAMPAIGN_DATE) return; - else setKey(4); - } - else setKey(3); - }; + const onStepClick = (currentStep) => { + if (!props?.props?.sessionData?.HCM_CAMPAIGN_NAME || !props?.props?.sessionData?.HCM_CAMPAIGN_TYPE) return; + + const stepToKeyMap = { + 0: 1, + 1: 2, + 2: 3, + 3: props?.props?.sessionData?.HCM_CAMPAIGN_DATE ? 4 : undefined + }; + + const nextKey = stepToKeyMap[currentStep]; + if (nextKey) setKey(nextKey); + };
📜 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 (22)
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/css/src/pages/employee/index.scss(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundaryDetailsSummary.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundarySummary.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignDates.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignDetailsSummary.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignSummary.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignUpdateSummary.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DataUploadSummary.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DeliveryDetailsSummary.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/SelectingBoundariesDuplicate.js(3 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/TagComponent.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UpdateBoundaryWrapper.js(3 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadData.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/CreateChecklist.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/CycleConfiguration.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SearchChecklist.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateChecklist.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateDatesWithBoundaries.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js(2 hunks)health/micro-ui/web/public/index.html(1 hunks)
👮 Files not reviewed due to content moderation or server errors (2)
- health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/CreateChecklist.js
- health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadData.js
🧰 Additional context used
📓 Path-based instructions (18)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateDatesWithBoundaries.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/SelectingBoundariesDuplicate.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UpdateBoundaryWrapper.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SearchChecklist.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/TagComponent.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignDetailsSummary.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundaryDetailsSummary.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignUpdateSummary.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateChecklist.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DeliveryDetailsSummary.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/CycleConfiguration.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundarySummary.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DataUploadSummary.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/CreateChecklist.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignDates.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignSummary.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UploadData.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignSummary.js
[error] 406-409: 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 (18)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/SelectingBoundariesDuplicate.js (2)
8-8: LGTM!Clean addition of the new TagComponent import.
116-116: LGTM!Appropriate replacement of the Tag component with TagComponent, correctly passing the campaignName prop.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/UpdateBoundaryWrapper.js (2)
80-80: LGTM! Component replacement looks good.The
Tagcomponent has been correctly replaced withTagComponent, maintaining the campaign name display functionality.
105-105: LGTM! Style improvements enhance layout.The added style properties provide better spacing and width control for the InfoCard component.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/UpdateChecklist.js (1)
469-469: LGTM! Component replacement is correctThe
Tagcomponent has been successfully replaced withTagComponent, and thecampaignNameprop is correctly passed.health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundaryDetailsSummary.js (1)
50-50: LGTM! Verify CSS class existence.The class name change from "parent-group" to "summary-parent-group" provides better semantic meaning for the summary view context.
Let's verify the CSS class exists in the SCSS files:
✅ Verification successful
CSS class implementation verified successfully
The new
summary-parent-groupclass is properly defined inpackages/css/src/pages/employee/index.scsswhile maintaining the originalparent-groupclass for other components. The change is consistent with the UI/UX improvements mentioned in the PR.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the new summary-parent-group class definition rg -l "summary-parent-group" --type scssLength of output: 69
Script:
#!/bin/bash # Search for both class names in style files echo "=== Searching for summary-parent-group ===" rg "summary-parent-group" --type css --type less --type sass echo -e "\n=== Searching for parent-group ===" rg "parent-group" --type css --type less --type sass # Backup: search without file type filters echo -e "\n=== Searching in all files with style extensions ===" fd "\.(css|scss|less|sass)$" | xargs rg "summary-parent-group|parent-group"Length of output: 1589
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DeliveryDetailsSummary.js (1)
Line range hint
354-362: LGTM! Integration looks good.The
TagComponentis well-integrated within the delivery details card container and follows the existing patterns for layout and translations.health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundarySummary.js (1)
154-154: Add fallback for session storage accessThe campaignName is retrieved from session storage without a proper fallback mechanism. Consider adding error handling for cases where the session data might be missing.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ViewChecklist.js (1)
8-8: LGTM! Component replacement is consistent.The replacement of
TagwithTagComponentis consistent with the PR objective and maintains the same functionality.Also applies to: 189-189
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/DataUploadSummary.js (1)
8-8: LGTM! Component replacement is consistent.The replacement of
TagwithTagComponentis consistent with the PR objective and maintains the same functionality.Also applies to: 308-308
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/CycleConfiguration.js (1)
7-7: LGTM! Component replacement is consistent.The replacement of
TagwithTagComponentis consistent with the PR objective and maintains the same functionality.Also applies to: 219-219
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignUpdateSummary.js (1)
9-9: LGTM! Component replacement is consistent.The replacement of
TagwithTagComponentis consistent with the PR objective and maintains the same functionality.Also applies to: 448-448
health/micro-ui/web/public/index.html (1)
13-13: LGTM! Consistent version update.The CSS version update matches the change in the other HTML file, maintaining consistency across the codebase.
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaign.scss (1)
138-140: LGTM! Minor style adjustments to the stepper card.The height reduction and padding adjustments look appropriate for better visual alignment.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignDetailsSummary.js (2)
162-162: LGTM! Component replacement looks good.The TagComponent implementation with campaignName prop is correctly placed.
6-6: Verify TagComponent import path.Ensure that the TagComponent is properly exported from the specified path.
✅ Verification successful
Import path for TagComponent is correctly specified
The component is properly exported from
TagComponent.jsin the same directory.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify TagComponent exists and is exported # Test: Search for TagComponent definition and export ast-grep --pattern 'export default TagComponent'Length of output: 186
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss (1)
795-806: LGTM! Well-structured CSS additions.The new summary-parent-group classes provide consistent styling and proper spacing between groups. The padding adjustment to parent-group maintains visual harmony.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignDates.js (1)
5-5: Verify TagComponent implementation and integration.The new
TagComponentis used with only thecampaignNameprop, which is a significant change from the typicalTagcomponent usage. Let's verify its implementation and integration.Also applies to: 135-135
✅ Verification successful
Implementation verified - TagComponent is correctly standardized
The TagComponent is intentionally implemented as a simplified wrapper around the Tag component, providing consistent styling and behavior across the campaign manager module. The single
campaignNameprop usage is by design and is consistently followed in all 15 occurrences throughout the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check TagComponent implementation and its usage across the codebase echo "Checking TagComponent implementation:" cat "./TagComponent.js" 2>/dev/null || echo "TagComponent.js not found in current directory" echo -e "\nChecking TagComponent usage patterns:" rg -A 2 "import.*TagComponent" --type jsLength of output: 6691
Script:
#!/bin/bash # Find and check TagComponent implementation echo "Checking TagComponent implementation:" fd TagComponent.js$ health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/ --exec cat {} echo -e "\nChecking TagComponent usage patterns:" rg "TagComponent.*campaign" -B 1 -A 3 --type jsLength of output: 14096
...i-internals/packages/modules/campaign-manager/src/components/SelectingBoundariesDuplicate.js
Show resolved
Hide resolved
...micro-ui-internals/packages/modules/campaign-manager/src/components/UpdateBoundaryWrapper.js
Show resolved
Hide resolved
...i/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignSummary.js
Show resolved
Hide resolved
...icro-ui-internals/packages/modules/campaign-manager/src/components/DeliveryDetailsSummary.js
Show resolved
Hide resolved
...icro-ui-internals/packages/modules/campaign-manager/src/components/DeliveryDetailsSummary.js
Show resolved
Hide resolved
...o-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/TagComponent.js
Show resolved
Hide resolved
...o-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/TagComponent.js
Show resolved
Hide resolved
...-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignDates.js
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
TagComponentfor consistent campaign name display across multiple componentsBug Fixes
Style