-
Notifications
You must be signed in to change notification settings - Fork 46
Sandbox login page #2765
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
base: develop
Are you sure you want to change the base?
Sandbox login page #2765
Conversation
* sandbox-ui-update * ui-updates * ui-update and old ui route fix * ui update and old ui conifgs added to view
)" This reverts commit 51e8d80.
…IGIT-Frontend into sandbox-ui-update
* privacy component alignment fix * package changes
…IGIT-Frontend into sandbox-ui-update
Co-authored-by: syedkaif866 <[email protected]>
Co-authored-by: syedkaif866 <[email protected]>
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: 1
🔭 Outside diff range comments (1)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/SupaDemoPlayer.js (1)
44-52
: Remove unnecessary empty lines.Multiple empty lines at the end of the file are not needed and should be cleaned up for consistency.
export default SupaDemoPlayer; - - - - - - - -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/LandingComponent.js
(4 hunks)micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/SandboxConfig.js
(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/SupaDemoPlayer.js
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit Configuration File
check
Files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/SandboxConfig.js
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/LandingComponent.js
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/SupaDemoPlayer.js
🧠 Learnings (4)
📓 Common learnings
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in `micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js` is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:279-280
Timestamp: 2025-06-26T10:21:42.619Z
Learning: In the sandbox ProductDetailsComponentUpdated.js component, string concatenation is intentionally used for translation keys (e.g., `t(`${module}` + "_SECTION1_ROLE_1")`) instead of template literals for easier config identification and to make the key structure more explicit.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js:24-24
Timestamp: 2025-06-26T09:43:00.597Z
Learning: In LoginSignupSelector component, the URL checking using window.location.href.includes("sandbox-ui/user/sign-up") is intentional for URL validation purposes, not navigation. The team prefers this approach over React Router's useLocation hook for specific URL checking needs.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss:1642-1662
Timestamp: 2025-06-26T10:24:08.628Z
Learning: In the sandbox SCSS file, tab styling with margin-bottom and border changes that cause layout shifts are intentional design requirements per updated UI specifications, not bugs to be fixed.
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/SandboxConfig.js (4)
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:279-280
Timestamp: 2025-06-26T10:21:42.619Z
Learning: In the sandbox ProductDetailsComponentUpdated.js component, string concatenation is intentionally used for translation keys (e.g., t(
${module} + "_SECTION1_ROLE_1")
) instead of template literals for easier config identification and to make the key structure more explicit.
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js:24-24
Timestamp: 2025-06-26T09:43:00.597Z
Learning: In LoginSignupSelector component, the URL checking using window.location.href.includes("sandbox-ui/user/sign-up") is intentional for URL validation purposes, not navigation. The team prefers this approach over React Router's useLocation hook for specific URL checking needs.
Learnt from: rachana-egov
PR: #1770
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js:320-322
Timestamp: 2024-11-07T11:02:33.520Z
Learning: In health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js
, the data?.additionalFields
object is guaranteed to be defined, so checking for its existence before accessing its keys is unnecessary.
Learnt from: Tulika-eGov
PR: #2188
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js:72-78
Timestamp: 2025-02-05T10:01:29.668Z
Learning: In the employee login form, defaultValues are dynamically generated from config[0].body based on populators.defaultValue, so spreading the entire defaultValues object is required rather than cherry-picking specific fields.
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/LandingComponent.js (6)
Learnt from: siddhant-nawale-egov
PR: #876
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js:3-39
Timestamp: 2024-06-14T14:10:20.359Z
Learning: The CustomScaleControl
component in the micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js
file should not include a "loading map" message as there is another component handling this functionality to avoid redundancy.
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:279-280
Timestamp: 2025-06-26T10:21:42.619Z
Learning: In the sandbox ProductDetailsComponentUpdated.js component, string concatenation is intentionally used for translation keys (e.g., t(
${module} + "_SECTION1_ROLE_1")
) instead of template literals for easier config identification and to make the key structure more explicit.
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js
is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Learnt from: jagankumar-egov
PR: #2488
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js:152-152
Timestamp: 2025-06-04T04:08:48.234Z
Learning: In the employee login component (micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js), prefer using a defaultTenant
variable to store Digit.ULBService.getStateId()
instead of calling the function multiple times inline in the defaultValue
object. This improves code readability and avoids function call duplication.
Learnt from: rachana-egov
PR: #1770
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js:320-322
Timestamp: 2024-11-07T11:02:33.520Z
Learning: In health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js
, the data?.additionalFields
object is guaranteed to be defined, so checking for its existence before accessing its keys is unnecessary.
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js:24-24
Timestamp: 2025-06-26T09:43:00.597Z
Learning: In LoginSignupSelector component, the URL checking using window.location.href.includes("sandbox-ui/user/sign-up") is intentional for URL validation purposes, not navigation. The team prefers this approach over React Router's useLocation hook for specific URL checking needs.
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/SupaDemoPlayer.js (4)
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js
is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Learnt from: siddhant-nawale-egov
PR: #876
File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392
Timestamp: 2024-06-14T14:10:38.086Z
Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.
Learnt from: siddhant-nawale-egov
PR: #876
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js:3-39
Timestamp: 2024-06-14T14:10:20.359Z
Learning: The CustomScaleControl
component in the micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js
file should not include a "loading map" message as there is another component handling this functionality to avoid redundancy.
Learnt from: siddhant-nawale-egov
PR: #698
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanPreview.js:1-1
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The imports in MicroplanPreview.js
are from different libraries: @egovernments/digit-ui-components
and @egovernments/digit-ui-react-components
.
⏰ Context from checks skipped due to timeout of 10000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/SandboxConfig.js (1)
3-3
: LGTM! Configuration update for demo video source.The URL change from YouTube to Supademo is clean and the new URL includes proper embed parameters.
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/LandingComponent.js (3)
12-13
: LGTM! Clean imports for the new demo player.The imports for
SupaDemoPlayer
andsandboxConfig
are properly structured and align with the video player replacement.
29-29
: LGTM! Default URL updated consistently.The default URL matches the one in
sandboxConfig
, ensuring consistency across the configuration.
72-75
: LGTM! Video player replacement with proper styling.The replacement of
YoutubeVideo
withSupaDemoPlayer
is clean. The explicit styling on the container provides good layout constraints with fixed dimensions. The fallback logic usingconfig.url
first, thensandboxConfig.url
ensures proper configuration precedence.micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/SupaDemoPlayer.js (1)
4-19
: LGTM! Well-structured styling with responsive design.The wrapper and iframe styles provide good responsive behavior with aspect ratio control and absolute positioning. The max-height constraint prevents the iframe from becoming too large on larger screens.
const SupaDemoPlayer = ({ src, title = "Embedded Demo" }) => { | ||
|
||
if (!src) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<div style={wrapperStyle}> | ||
<iframe | ||
src={src} | ||
style={iframeStyle} | ||
loading="lazy" | ||
title={title} | ||
allow="clipboard-write" | ||
frameBorder="0" | ||
allowFullScreen | ||
></iframe> | ||
</div> | ||
); | ||
}; |
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.
🧹 Nitpick (assertive)
Component logic is solid, but consider security and HTML5 compliance.
The component structure is clean with proper guard clause and prop handling. However, there are a few minor improvements to consider:
frameBorder="0"
is deprecated in HTML5 - use CSSborder: none
instead- The
allow
attribute could be more restrictive for security
Consider this diff for HTML5 compliance and improved security:
<iframe
src={src}
- style={iframeStyle}
+ style={{...iframeStyle, border: 'none'}}
loading="lazy"
title={title}
- allow="clipboard-write"
- frameBorder="0"
+ allow="clipboard-write; fullscreen"
allowFullScreen
></iframe>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const SupaDemoPlayer = ({ src, title = "Embedded Demo" }) => { | |
if (!src) { | |
return null; | |
} | |
return ( | |
<div style={wrapperStyle}> | |
<iframe | |
src={src} | |
style={iframeStyle} | |
loading="lazy" | |
title={title} | |
allow="clipboard-write" | |
frameBorder="0" | |
allowFullScreen | |
></iframe> | |
</div> | |
); | |
}; | |
const SupaDemoPlayer = ({ src, title = "Embedded Demo" }) => { | |
if (!src) { | |
return null; | |
} | |
return ( | |
<div style={wrapperStyle}> | |
<iframe | |
src={src} | |
style={{ ...iframeStyle, border: 'none' }} | |
loading="lazy" | |
title={title} | |
allow="clipboard-write; fullscreen" | |
allowFullScreen | |
></iframe> | |
</div> | |
); | |
}; |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/Landing/SupaDemoPlayer.js
between lines 21 and 40, replace the deprecated iframe attribute frameBorder="0"
with CSS style border: none to comply with HTML5 standards. Additionally, refine
the allow attribute to be more restrictive by specifying only necessary
permissions to enhance security, for example limiting it to "clipboard-write" or
other minimal required features.
Co-authored-by: syedkaif866 <[email protected]>
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: 1
♻️ Duplicate comments (1)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js (1)
1-23
: Reorganize imports and remove excessive empty lines.Group all imports together at the top and remove the excessive empty lines for better code organization.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit Configuration File
check
Files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
🧠 Learnings (2)
📓 Common learnings
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in `micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js` is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:279-280
Timestamp: 2025-06-26T10:21:42.619Z
Learning: In the sandbox ProductDetailsComponentUpdated.js component, string concatenation is intentionally used for translation keys (e.g., `t(`${module}` + "_SECTION1_ROLE_1")`) instead of template literals for easier config identification and to make the key structure more explicit.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js:24-24
Timestamp: 2025-06-26T09:43:00.597Z
Learning: In LoginSignupSelector component, the URL checking using window.location.href.includes("sandbox-ui/user/sign-up") is intentional for URL validation purposes, not navigation. The team prefers this approach over React Router's useLocation hook for specific URL checking needs.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss:1642-1662
Timestamp: 2025-06-26T10:24:08.628Z
Learning: In the sandbox SCSS file, tab styling with margin-bottom and border changes that cause layout shifts are intentional design requirements per updated UI specifications, not bugs to be fixed.
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js (35)
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:279-280
Timestamp: 2025-06-26T10:21:42.619Z
Learning: In the sandbox ProductDetailsComponentUpdated.js component, string concatenation is intentionally used for translation keys (e.g., t(
${module} + "_SECTION1_ROLE_1")
) instead of template literals for easier config identification and to make the key structure more explicit.
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js
is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Learnt from: siddhant-nawale-egov
PR: #606
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js:276-276
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The addResourcesToFilteredDataToShow
function in micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js
now includes checks for data integrity, such as validation of previewData
, the common column, and resources
, along with error logging for invalid data formats.
Learnt from: siddhant-nawale-egov
PR: #606
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js:276-276
Timestamp: 2024-10-08T20:11:07.772Z
Learning: The addResourcesToFilteredDataToShow
function in micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js
now includes checks for data integrity, such as validation of previewData
, the common column, and resources
, along with error logging for invalid data formats.
Learnt from: siddhant-nawale-egov
PR: #698
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanPreview.js:1-1
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The imports in MicroplanPreview.js
are from different libraries: @egovernments/digit-ui-components
and @egovernments/digit-ui-react-components
.
Learnt from: siddhant-nawale-egov
PR: #876
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js:3-39
Timestamp: 2024-06-14T14:10:20.359Z
Learning: The CustomScaleControl
component in the micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js
file should not include a "loading map" message as there is another component handling this functionality to avoid redundancy.
Learnt from: rachana-egov
PR: #1770
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js:320-322
Timestamp: 2024-11-07T11:02:33.520Z
Learning: In health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js
, the data?.additionalFields
object is guaranteed to be defined, so checking for its existence before accessing its keys is unnecessary.
Learnt from: siddhant-nawale-egov
PR: #876
File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392
Timestamp: 2024-06-14T14:10:38.086Z
Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.
Learnt from: Ramkrishna-egov
PR: #1545
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/ActivityCard.js:1-1
Timestamp: 2024-10-17T11:59:00.327Z
Learning: In ActivityCard.js
, React.Fragment
is used in the code.
Learnt from: Hari-egov
PR: #2788
File: micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js:26-26
Timestamp: 2025-07-15T04:58:30.634Z
Learning: In the HRMS module (micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js), the team has moved away from using MDMS hooks for boundary data fetching and now uses direct API calls to the boundary service endpoint /boundary-service/boundary-relationships/_search
instead of the useHrmsMDMS
hook with "FetchBoundaries" parameter.
Learnt from: nabeelmd-eGov
PR: #1680
File: health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js:1032-1094
Timestamp: 2024-10-26T15:11:49.938Z
Learning: In the DIGIT-Frontend project, when reviewing the UICustomizations.js
file, avoid suggesting refactoring of repetitive rendering logic in switch cases into helper functions, unless explicitly requested.
Learnt from: jagankumar-egov
PR: #1763
File: health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/MyCampaign.js:65-75
Timestamp: 2024-11-07T07:17:27.636Z
Learning: In MyCampaign.js
, when suggesting code improvements for the onClickRow
function, keep suggestions simple and avoid unnecessary complexity.
Learnt from: jagankumar-egov
PR: #2089
File: micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js:111-111
Timestamp: 2025-01-02T04:41:15.957Z
Learning: In micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js, always use "mdms[0].uniqueIdentifier" for the jsonPath instead of differentiating between "SOR" and other masters.
Learnt from: jagankumar-egov
PR: #2488
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js:152-152
Timestamp: 2025-06-04T04:08:48.234Z
Learning: In the employee login component (micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js), prefer using a defaultTenant
variable to store Digit.ULBService.getStateId()
instead of calling the function multiple times inline in the defaultValue
object. This improves code readability and avoids function call duplication.
Learnt from: siddhant-nawale-egov
PR: #876
File: micro-ui/web/micro-ui-internals/packages/css/postcss.config.js:3-13
Timestamp: 2024-10-08T20:11:12.539Z
Learning: Retain commented-out code in postcss.config.js
as it might be needed for future configurations or debugging.
Learnt from: rachana-egov
PR: #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.
Learnt from: rachana-egov
PR: #1770
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/timelinePopUpWrapper.js:36-36
Timestamp: 2024-11-07T11:00:11.847Z
Learning: In the TimelinePopUpWrapper
component in timelinePopUpWrapper.js
, the action
property of instance
is a required field and will always be defined; thus, checks for undefined values for instance.action
are unnecessary.
Learnt from: jagankumar-egov
PR: #2342
File: health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CreateCampaignComponents.js/DateSelection.js:41-47
Timestamp: 2025-04-21T05:15:36.122Z
Learning: In React components, prefer using state setters directly in event handlers rather than creating wrapper functions that only call the setter without additional logic.
Learnt from: rachana-egov
PR: #1622
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js:355-363
Timestamp: 2024-10-22T12:51:42.266Z
Learning: In PopInboxTable.js
, the showEditVillagePopup
state variable is initialized as an empty object.
Learnt from: siddhant-nawale-egov
PR: #923
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js:458-459
Timestamp: 2024-06-17T18:54:45.789Z
Learning: onKeyPress
is deprecated and should be replaced with onKeyDown
or onKeyUp
for handling keyboard interactions.
Learnt from: siddhant-nawale-egov
PR: #698
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/resourceMapping.js:74-74
Timestamp: 2024-06-10T19:25:42.992Z
Learning: onKeyPress
is deprecated and should be replaced with onKeyDown
or onKeyUp
for handling keyboard interactions.
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#0
File: :0-0
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The onStepClick
logic in CreateMicroplan.js
is intentionally commented out to allow developers to navigate between screens during development, even though not all screens are made yet.
Learnt from: rachana-egov
PR: #1612
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/WorkflowCommentPopUp.js:37-41
Timestamp: 2024-10-22T09:14:46.876Z
Learning: In this project, the TextArea
component should submit the form when the 'Enter' key is pressed.
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#0
File: :0-0
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The Button component from @egovernments/digit-ui-components
has compatibility issues with the ActionBar, leading to the preference for using the Button from @egovernments/digit-ui-react-components
in certain scenarios.
Learnt from: Tulika-eGov
PR: #2188
File: micro-ui/web/micro-ui-internals/packages/libraries/src/hooks/pgr/useComplaintSubType.js:14-14
Timestamp: 2025-02-05T10:05:37.433Z
Learning: In PGR's useComplaintSubType hook, the translation function 't' must be included in the useEffect dependency array to ensure complaint types are updated when the language changes.
Learnt from: Tulika-eGov
PR: #2188
File: micro-ui/web/micro-ui-internals/packages/modules/pgr/src/pages/employee/new-inbox.js:34-47
Timestamp: 2025-02-05T10:18:29.947Z
Learning: In the DIGIT-Frontend codebase, translations within FormComposer are handled internally by the component itself, so the translation function 't' does not need to be added to useMemo dependency arrays even when used within the memoized function that generates form configurations.
Learnt from: ashish-egov
PR: #1441
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/useGenerateIdCampaign.js:1-1
Timestamp: 2024-09-30T05:45:14.549Z
Learning: In the useGenerateIdCampaign
function, auto assignment of default parameter values is needed in parameter destructuring.
Learnt from: rachana-egov
PR: #1925
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js:267-269
Timestamp: 2024-11-28T11:34:27.480Z
Learning: In the downloadTemplate
function of health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js
, the scenario where state?.hierarchyType
is undefined will not occur.
Learnt from: siddhant-nawale-egov
PR: #675
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanPreview.js:4-10
Timestamp: 2024-06-10T19:25:42.992Z
Learning: Imports in MicroplanPreview.js
are either already grouped appropriately or come from different directories, as clarified by the user.
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#0
File: :0-0
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The pagination SVG components (PaginationPrevious
, PaginationNext
, PaginationLast
, and PaginationFirst
) are correctly listed once in the export block of the index.js
file within the svg-components
package, without any duplications.
Learnt from: siddhant-nawale-egov
PR: #675
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js:675-675
Timestamp: 2024-10-08T20:11:12.539Z
Learning: The modal component from the external library used in Mapping.js
does not support the className
property, and thus styles cannot be refactored into a CSS module.
Learnt from: siddhant-nawale-egov
PR: #675
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js:675-675
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The modal component from the external library used in Mapping.js
does not support the className
property, and thus styles cannot be refactored into a CSS module.
Learnt from: siddhant-nawale-egov
PR: #675
File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:33-58
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The use of !important
in the .wbh-header-container
and .guideline-actionbar-content
CSS classes within microplanning.scss
is necessary to prevent these styles from being overridden.
Learnt from: Bhavya-egov
PR: #2013
File: health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/utils/jsonReader.js:24-27
Timestamp: 2024-12-12T05:38:22.901Z
Learning: In the jsonReader
function within health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/utils/jsonReader.js
, generic error handling is sufficient, and enhancing error handling with specific error types is not necessary.
Learnt from: Hari-egov
PR: #2788
File: micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js:157-162
Timestamp: 2025-07-15T06:50:54.535Z
Learning: In the HRMS module (micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js), localization is already working correctly when using ele.hierarchyType
directly in the gethierarchylistdata()
function. The suggested locale conversion using Digit.Utils.locale.convertToLocale(ele.hierarchyType, 'EGOV_LOCATION_BOUNDARYTYPE')
is not needed as the current implementation already provides proper localization.
🧬 Code Graph Analysis (1)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js (2)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/index.js (1)
config
(10-25)micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (1)
getLinkByType
(73-79)
🪛 Biome (2.1.2)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
[error] 247-252: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 247-252: 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)
[error] 111-111: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 80-80: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 104-104: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 150-165: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form
element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
⏰ Context from checks skipped due to timeout of 10000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
// const WalkthroughSection = ({ activeTab, setActiveTab, t, employeeWTLink, citizenWTLink, stakeholderWTLink, module }) => { | ||
// const iframeSrc = activeTab === "citizen" | ||
// ? citizenWTLink | ||
// : activeTab === "employee" | ||
// ? employeeWTLink | ||
// : stakeholderWTLink; | ||
|
||
// return ( | ||
// <div className="walkthrough-container" style={{ backgroundColor: '#efefefef', padding: '3rem 6rem' }}> | ||
// <div className="wt-c1"> | ||
// <h2 className="wt-title">{t("SB_WALK_THROUHG_HEADER")}</h2> | ||
// <p className="wt-subtitle">{t("SB_WALK_THROUHG_DESCRIPTION")}</p> | ||
// </div> | ||
// <div className="wt-tabs-center wt-tabs-and-iframe"> | ||
// <div className="wt-tab-wrapper"> | ||
// {module === "Finance" ? ( | ||
// <div className="wt-tab active">{t("SB_WALK_THROUHG_EMPLOYEE")}</div> | ||
// ) : ( | ||
// <div className="wt-tab-wrapper"> | ||
// <div className={`wt-tab ${activeTab === "citizen" ? "active" : ""}`} onClick={() => setActiveTab("citizen")}>{t("SB_WALK_THROUHG_CITIZEN")}</div> | ||
// <div className={`wt-tab ${activeTab === "employee" ? "active" : ""}`} onClick={() => setActiveTab("employee")}>{t("SB_WALK_THROUHG_EMPLOYEE")}</div> | ||
// {(module === "FSM" || module === "OBPS") && ( | ||
// <div className={`wt-tab ${activeTab === "stakeholder" ? "active" : ""}`} onClick={() => setActiveTab("stakeholder")}> | ||
// {module === "FSM" ? t("FSM_STAKEHOLDER_FEATURE_HEADER") : t("OBPS_STAKEHOLDER_FEATURE_HEADER")} | ||
// </div> | ||
// )} | ||
// </div> | ||
// )} | ||
// </div> | ||
// <div className="wt-iframe-wrapper"> | ||
// <iframe src={iframeSrc} title="Digit Sandbox" className="wt-iframe"></iframe> | ||
// </div> | ||
// </div> | ||
// </div> | ||
// ); | ||
// }; |
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.
🧹 Nitpick (assertive)
Remove commented out code.
This large block of commented code should be removed if it's no longer needed. If it's kept for reference, consider moving it to documentation or version control history.
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
between lines 169 and 204, there is a large block of commented-out code defining
the WalkthroughSection component. Since this code is no longer needed, remove
the entire commented block to clean up the file and improve readability. If the
code needs to be preserved for reference, ensure it is stored in version control
or documentation instead of inline comments.
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: 4
♻️ Duplicate comments (3)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/DummyLoader.js (3)
1-1
: Remove unused import.The
useRef
import is not used in the visible code and should be removed to keep imports clean.-import React, {useRef, useEffect, useState } from "react"; +import React, { useEffect, useState } from "react";
33-37
: Consider extracting navigation logic to reduce duplication.The navigation logic is duplicated between the
useEffect
andonButtonClick
handlers. Consider extracting this into a separate function.+ const handleNavigation = () => { + if (roleForLandingPage(getUserRoles, MdmsRes)) { + window.location.href = `/${window?.globalPath}/${tenant}${RoleLandingUrl}`; + } else { + window.location.href = `/${window?.globalPath}/${tenant}/employee`; + } + }; const navigateTimeout = setTimeout(() => { - if (roleForLandingPage(getUserRoles, MdmsRes)) { - window.location.href = `/${window?.globalPath}/${tenant}${RoleLandingUrl}`; - } else { - window.location.href = `/${window?.globalPath}/${tenant}/employee`; - } + handleNavigation(); }, 1000); const onButtonClick = () => { - if (roleForLandingPage(getUserRoles, MdmsRes)) { - window.location.href = `/${window?.globalPath}/${tenant}${RoleLandingUrl}`; - } else { - window.location.href = `/${window?.globalPath}/${tenant}/employee`; - } + handleNavigation(); };
48-48
: Remove unused variable.The
ref
variable is declared but never used.- const ref = useRef(null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
micro-ui/web/micro-ui-internals/packages/libraries/package.json
is excluded by!**/*.json
📒 Files selected for processing (3)
micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/ApiCacheService.js
(3 hunks)micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/Utils/Storage.js
(2 hunks)micro-ui/web/micro-ui-internals/packages/modules/core/src/components/DummyLoader.js
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit Configuration File
check
Files:
micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/ApiCacheService.js
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/DummyLoader.js
micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/Utils/Storage.js
🧠 Learnings (2)
📓 Common learnings
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in `micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js` is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:279-280
Timestamp: 2025-06-26T10:21:42.619Z
Learning: In the sandbox ProductDetailsComponentUpdated.js component, string concatenation is intentionally used for translation keys (e.g., `t(`${module}` + "_SECTION1_ROLE_1")`) instead of template literals for easier config identification and to make the key structure more explicit.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js:24-24
Timestamp: 2025-06-26T09:43:00.597Z
Learning: In LoginSignupSelector component, the URL checking using window.location.href.includes("sandbox-ui/user/sign-up") is intentional for URL validation purposes, not navigation. The team prefers this approach over React Router's useLocation hook for specific URL checking needs.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss:1642-1662
Timestamp: 2025-06-26T10:24:08.628Z
Learning: In the sandbox SCSS file, tab styling with margin-bottom and border changes that cause layout shifts are intentional design requirements per updated UI specifications, not bugs to be fixed.
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/DummyLoader.js (23)
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js:24-24
Timestamp: 2025-06-26T09:43:00.597Z
Learning: In LoginSignupSelector component, the URL checking using window.location.href.includes("sandbox-ui/user/sign-up") is intentional for URL validation purposes, not navigation. The team prefers this approach over React Router's useLocation hook for specific URL checking needs.
Learnt from: siddhant-nawale-egov
PR: #876
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js:3-39
Timestamp: 2024-06-14T14:10:20.359Z
Learning: The CustomScaleControl
component in the micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js
file should not include a "loading map" message as there is another component handling this functionality to avoid redundancy.
Learnt from: Hari-egov
PR: #2788
File: micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js:26-26
Timestamp: 2025-07-15T04:58:30.634Z
Learning: In the HRMS module (micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js), the team has moved away from using MDMS hooks for boundary data fetching and now uses direct API calls to the boundary service endpoint /boundary-service/boundary-relationships/_search
instead of the useHrmsMDMS
hook with "FetchBoundaries" parameter.
Learnt from: jagankumar-egov
PR: #2488
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js:152-152
Timestamp: 2025-06-04T04:08:48.234Z
Learning: In the employee login component (micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js), prefer using a defaultTenant
variable to store Digit.ULBService.getStateId()
instead of calling the function multiple times inline in the defaultValue
object. This improves code readability and avoids function call duplication.
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js
is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Learnt from: jagankumar-egov
PR: #2089
File: micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js:111-111
Timestamp: 2025-01-02T04:41:15.957Z
Learning: In micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js, always use "mdms[0].uniqueIdentifier" for the jsonPath instead of differentiating between "SOR" and other masters.
Learnt from: siddhant-nawale-egov
PR: #606
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js:276-276
Timestamp: 2024-10-08T20:11:07.772Z
Learning: The addResourcesToFilteredDataToShow
function in micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js
now includes checks for data integrity, such as validation of previewData
, the common column, and resources
, along with error logging for invalid data formats.
Learnt from: siddhant-nawale-egov
PR: #606
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js:276-276
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The addResourcesToFilteredDataToShow
function in micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js
now includes checks for data integrity, such as validation of previewData
, the common column, and resources
, along with error logging for invalid data formats.
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#0
File: :0-0
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The onStepClick
logic in CreateMicroplan.js
is intentionally commented out to allow developers to navigate between screens during development, even though not all screens are made yet.
Learnt from: siddhant-nawale-egov
PR: #691
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js:16-17
Timestamp: 2024-10-08T20:11:12.539Z
Learning: The components Loader
, Toast
, and Modal
are from different libraries in the DIGIT-Frontend project, as clarified by the user.
Learnt from: siddhant-nawale-egov
PR: #698
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanPreview.js:1-1
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The imports in MicroplanPreview.js
are from different libraries: @egovernments/digit-ui-components
and @egovernments/digit-ui-react-components
.
Learnt from: Ramkrishna-egov
PR: #1545
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/ActivityCard.js:1-1
Timestamp: 2024-10-17T11:59:00.327Z
Learning: In ActivityCard.js
, React.Fragment
is used in the code.
Learnt from: siddhant-nawale-egov
PR: #720
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Nagivator.js:46-56
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The user prefers the useEffect
hook to run only when checkDataCompletion
changes in the Navigator
component.
Learnt from: nabeelmd-eGov
PR: #1680
File: health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js:1032-1094
Timestamp: 2024-10-26T15:11:49.938Z
Learning: In the DIGIT-Frontend project, when reviewing the UICustomizations.js
file, avoid suggesting refactoring of repetitive rendering logic in switch cases into helper functions, unless explicitly requested.
Learnt from: siddhant-nawale-egov
PR: #675
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanPreview.js:4-10
Timestamp: 2024-06-10T19:25:42.992Z
Learning: Imports in MicroplanPreview.js
are either already grouped appropriately or come from different directories, as clarified by the user.
Learnt from: rachana-egov
PR: #1770
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js:320-322
Timestamp: 2024-11-07T11:02:33.520Z
Learning: In health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js
, the data?.additionalFields
object is guaranteed to be defined, so checking for its existence before accessing its keys is unnecessary.
Learnt from: jagankumar-egov
PR: #1763
File: health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/MyCampaign.js:65-75
Timestamp: 2024-11-07T07:17:27.636Z
Learning: In MyCampaign.js
, when suggesting code improvements for the onClickRow
function, keep suggestions simple and avoid unnecessary complexity.
Learnt from: rachana-egov
PR: #1770
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/timelinePopUpWrapper.js:36-36
Timestamp: 2024-11-07T11:00:11.847Z
Learning: In the TimelinePopUpWrapper
component in timelinePopUpWrapper.js
, the action
property of instance
is a required field and will always be defined; thus, checks for undefined values for instance.action
are unnecessary.
Learnt from: Tulika-eGov
PR: #2188
File: micro-ui/web/micro-ui-internals/packages/libraries/src/hooks/pgr/useComplaintSubType.js:14-14
Timestamp: 2025-02-05T10:05:37.433Z
Learning: In PGR's useComplaintSubType hook, the translation function 't' must be included in the useEffect dependency array to ensure complaint types are updated when the language changes.
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#0
File: :0-0
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The Button component from @egovernments/digit-ui-components
has compatibility issues with the ActionBar, leading to the preference for using the Button from @egovernments/digit-ui-react-components
in certain scenarios.
Learnt from: jagankumar-egov
PR: #2342
File: health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CreateCampaignComponents.js/DateSelection.js:41-47
Timestamp: 2025-04-21T05:15:36.122Z
Learning: In React components, prefer using state setters directly in event handlers rather than creating wrapper functions that only call the setter without additional logic.
Learnt from: Tulika-eGov
PR: #2188
File: micro-ui/web/micro-ui-internals/packages/modules/pgr/src/pages/employee/new-inbox.js:34-47
Timestamp: 2025-02-05T10:18:29.947Z
Learning: In the DIGIT-Frontend codebase, translations within FormComposer are handled internally by the component itself, so the translation function 't' does not need to be added to useMemo dependency arrays even when used within the memoized function that generates form configurations.
Learnt from: Tulika-eGov
PR: #2188
File: micro-ui/web/micro-ui-internals/packages/libraries/src/hooks/useLocalities.js:5-6
Timestamp: 2025-02-05T10:06:57.846Z
Learning: In useLocalities hook, the language parameter is used in the query key to trigger React Query cache invalidation when language changes, while actual translation is handled by the 't' function parameter.
🧬 Code Graph Analysis (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/DummyLoader.js (3)
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Otp/index.js (3)
MdmsRes
(37-42)tenant
(35-35)RoleLandingUrl
(44-44)micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (2)
MdmsRes
(18-31)Digit
(10-10)micro-ui/web/micro-ui-internals/packages/libraries/src/hooks/pgr/useComplaintDetails.js (1)
window
(31-31)
⏰ Context from checks skipped due to timeout of 10000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/Utils/Storage.js (3)
1-1
: LGTM: Compression library importThe LZString import is appropriate for the compression functionality implemented in the PersistantStorage methods.
58-69
: Verify removal of TTL functionalityThe new
get
method implementation lacks the TTL (time-to-live) expiry checking that was present in the originalgetStorage
implementation. This means stored data will persist indefinitely without automatic cleanup.Is this intentional, or should TTL functionality be preserved for consistency with the original Storage implementation?
89-96
: LGTM: Clean remove method implementationThe
remove
method implementation is well-structured with appropriate error handling and logging.micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/ApiCacheService.js (1)
6-7
: Verify the performance implications of disabling API cachingThe cache durations for critical services (localization, egov-mdms-service, mdms-v2) have been set to 0, effectively disabling caching. This will result in:
- Increased API calls: Every request will hit the server instead of using cached data
- Higher network usage: More frequent data fetching
- Potential performance degradation: Especially for frequently accessed data like localization
Given that the original values are preserved as comments, is this change:
- Temporary for testing purposes?
- Part of a new caching strategy using the enhanced PersistantStorage?
- Intentional for real-time data requirements?
Please confirm this is intentional and consider the impact on application performance and server load.
Also applies to: 19-20, 31-32
set: (key, value) => { | ||
try { | ||
let dataToStore = value; | ||
// Deduplicate if key is localization | ||
if (key === "localization") { | ||
const existing = PersistantStorage.get(key) || []; | ||
const combined = [...existing, ...value]; | ||
// Remove duplicates based on 'code' | ||
dataToStore = Array.from( | ||
new Map(combined.map(item => [item.code, item])).values() | ||
); | ||
} | ||
// Compress before storing | ||
const compressed = LZString.compress(JSON.stringify(dataToStore)); | ||
localStorage.setItem(key, compressed); | ||
} catch (e) { | ||
console.error("Error writing to localStorage:", e); | ||
} | ||
}, |
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.
🧹 Nitpick (assertive)
Review localization deduplication logic
The special handling for localization data has some considerations:
-
Assumption about data structure: The code assumes all localization items have a
code
property for deduplication. Consider adding a safety check. -
Recursive dependency: The method calls
PersistantStorage.get(key)
withinset
, which could be problematic if there are circular dependencies or issues with theget
method. -
Data merging behavior: The logic always merges new data with existing data. Consider if this is always the desired behavior or if there should be an option to replace existing data.
Consider this improvement for safer deduplication:
if (key === "localization") {
const existing = PersistantStorage.get(key) || [];
const combined = [...existing, ...value];
// Remove duplicates based on 'code'
- dataToStore = Array.from(
- new Map(combined.map(item => [item.code, item])).values()
- );
+ dataToStore = Array.from(
+ new Map(combined.map(item => [item?.code || Math.random(), item])).values()
+ );
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
set: (key, value) => { | |
try { | |
let dataToStore = value; | |
// Deduplicate if key is localization | |
if (key === "localization") { | |
const existing = PersistantStorage.get(key) || []; | |
const combined = [...existing, ...value]; | |
// Remove duplicates based on 'code' | |
dataToStore = Array.from( | |
new Map(combined.map(item => [item.code, item])).values() | |
); | |
} | |
// Compress before storing | |
const compressed = LZString.compress(JSON.stringify(dataToStore)); | |
localStorage.setItem(key, compressed); | |
} catch (e) { | |
console.error("Error writing to localStorage:", e); | |
} | |
}, | |
set: (key, value) => { | |
try { | |
let dataToStore = value; | |
// Deduplicate if key is localization | |
if (key === "localization") { | |
const existing = PersistantStorage.get(key) || []; | |
const combined = [...existing, ...value]; | |
// Remove duplicates based on 'code' | |
dataToStore = Array.from( | |
new Map(combined.map(item => [item?.code || Math.random(), item])).values() | |
); | |
} | |
// Compress before storing | |
const compressed = LZString.compress(JSON.stringify(dataToStore)); | |
localStorage.setItem(key, compressed); | |
} catch (e) { | |
console.error("Error writing to localStorage:", e); | |
} | |
}, |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/Utils/Storage.js
between lines 70 and 88, improve the localization deduplication by first adding
a check to ensure each item has a valid 'code' property before using it as a key
for deduplication. Avoid directly calling PersistantStorage.get(key) inside set
to prevent potential recursive dependency issues; instead, consider passing
existing data as a parameter or restructuring the logic. Also, add an option to
control whether to merge with existing data or replace it entirely, allowing
more flexible data handling.
useEffect(() => { | ||
if (MdmsRes?.[0].url) { | ||
setButtonDisabled(false); | ||
} | ||
}, [MdmsRes]); |
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.
🛠️ Refactor suggestion
Add null safety check for MDMS data.
The current implementation may throw an error if MdmsRes
is undefined or empty.
useEffect(() => {
- if (MdmsRes?.[0].url) {
+ if (MdmsRes?.[0]?.url) {
setButtonDisabled(false);
}
}, [MdmsRes]);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
useEffect(() => { | |
if (MdmsRes?.[0].url) { | |
setButtonDisabled(false); | |
} | |
}, [MdmsRes]); | |
useEffect(() => { | |
if (MdmsRes?.[0]?.url) { | |
setButtonDisabled(false); | |
} | |
}, [MdmsRes]); |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/DummyLoader.js
around lines 69 to 73, the useEffect hook accesses MdmsRes[0].url without
checking if MdmsRes is defined or has elements, which can cause runtime errors.
Add a null safety check to ensure MdmsRes is defined and has at least one
element before accessing MdmsRes[0].url to prevent errors when MdmsRes is
undefined or empty.
} | ||
}, [MdmsRes]); | ||
|
||
const RoleLandingUrl = MdmsRes?.[0].url; |
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.
🛠️ Refactor suggestion
Add null safety for MDMS data access.
Consistent with the fix above, add optional chaining for safer property access.
-const RoleLandingUrl = MdmsRes?.[0].url;
+const RoleLandingUrl = MdmsRes?.[0]?.url;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const RoleLandingUrl = MdmsRes?.[0].url; | |
const RoleLandingUrl = MdmsRes?.[0]?.url; |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/DummyLoader.js
at line 75, the code accesses MdmsRes[0].url without null safety, which can
cause runtime errors if MdmsRes or its first element is undefined. Add optional
chaining to safely access the url property by changing the expression to
MdmsRes?.[0]?.url.
const roleForLandingPage = (getUserRoles, MdmsRes) => { | ||
MdmsRes | ||
const userRole = getUserRoles?.[0]?.code; | ||
return ( | ||
userRole === "SUPERUSER" && | ||
Array.isArray(MdmsRes) && | ||
MdmsRes.some((page) => page.rolesForLandingPage.includes("SUPERUSER")) | ||
); | ||
}; |
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.
Fix syntax error and improve function structure.
The function has a syntax error and logical issues that need to be addressed.
const roleForLandingPage = (getUserRoles, MdmsRes) => {
- MdmsRes
const userRole = getUserRoles?.[0]?.code;
return (
userRole === "SUPERUSER" &&
Array.isArray(MdmsRes) &&
MdmsRes.some((page) => page.rolesForLandingPage.includes("SUPERUSER"))
);
};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const roleForLandingPage = (getUserRoles, MdmsRes) => { | |
MdmsRes | |
const userRole = getUserRoles?.[0]?.code; | |
return ( | |
userRole === "SUPERUSER" && | |
Array.isArray(MdmsRes) && | |
MdmsRes.some((page) => page.rolesForLandingPage.includes("SUPERUSER")) | |
); | |
}; | |
const roleForLandingPage = (getUserRoles, MdmsRes) => { | |
const userRole = getUserRoles?.[0]?.code; | |
return ( | |
userRole === "SUPERUSER" && | |
Array.isArray(MdmsRes) && | |
MdmsRes.some((page) => page.rolesForLandingPage.includes("SUPERUSER")) | |
); | |
}; |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/DummyLoader.js
around lines 77 to 85, the function roleForLandingPage contains a syntax error
with an unused expression "MdmsRes" and has structural issues. Remove the stray
"MdmsRes" line, ensure the function parameters are used correctly, and improve
the logic to clearly check if the user's role is "SUPERUSER" and if MdmsRes is
an array containing any page with "SUPERUSER" in rolesForLandingPage. Refactor
the function to be syntactically correct and logically clear.
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
micro-ui/web/micro-ui-internals/packages/css/src/components/bannercomponents.scss
(0 hunks)micro-ui/web/micro-ui-internals/packages/css/src/digitv2/components/bannerV2.scss
(0 hunks)micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/LanguageSelection/index.js
(1 hunks)
💤 Files with no reviewable changes (2)
- micro-ui/web/micro-ui-internals/packages/css/src/components/bannercomponents.scss
- micro-ui/web/micro-ui-internals/packages/css/src/digitv2/components/bannerV2.scss
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit Configuration File
check
Files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/LanguageSelection/index.js
🧠 Learnings (2)
📓 Common learnings
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in `micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js` is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:279-280
Timestamp: 2025-06-26T10:21:42.619Z
Learning: In the sandbox ProductDetailsComponentUpdated.js component, string concatenation is intentionally used for translation keys (e.g., `t(`${module}` + "_SECTION1_ROLE_1")`) instead of template literals for easier config identification and to make the key structure more explicit.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js:24-24
Timestamp: 2025-06-26T09:43:00.597Z
Learning: In LoginSignupSelector component, the URL checking using window.location.href.includes("sandbox-ui/user/sign-up") is intentional for URL validation purposes, not navigation. The team prefers this approach over React Router's useLocation hook for specific URL checking needs.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss:1642-1662
Timestamp: 2025-06-26T10:24:08.628Z
Learning: In the sandbox SCSS file, tab styling with margin-bottom and border changes that cause layout shifts are intentional design requirements per updated UI specifications, not bugs to be fixed.
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/LanguageSelection/index.js (11)
Learnt from: jagankumar-egov
PR: #2488
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js:152-152
Timestamp: 2025-06-04T04:08:48.234Z
Learning: In the employee login component (micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js), prefer using a defaultTenant
variable to store Digit.ULBService.getStateId()
instead of calling the function multiple times inline in the defaultValue
object. This improves code readability and avoids function call duplication.
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js:24-24
Timestamp: 2025-06-26T09:43:00.597Z
Learning: In LoginSignupSelector component, the URL checking using window.location.href.includes("sandbox-ui/user/sign-up") is intentional for URL validation purposes, not navigation. The team prefers this approach over React Router's useLocation hook for specific URL checking needs.
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:279-280
Timestamp: 2025-06-26T10:21:42.619Z
Learning: In the sandbox ProductDetailsComponentUpdated.js component, string concatenation is intentionally used for translation keys (e.g., t(
${module} + "_SECTION1_ROLE_1")
) instead of template literals for easier config identification and to make the key structure more explicit.
Learnt from: jagankumar-egov
PR: #2089
File: micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js:111-111
Timestamp: 2025-01-02T04:41:15.957Z
Learning: In micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js, always use "mdms[0].uniqueIdentifier" for the jsonPath instead of differentiating between "SOR" and other masters.
Learnt from: rachana-egov
PR: #1770
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js:320-322
Timestamp: 2024-11-07T11:02:33.520Z
Learning: In health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js
, the data?.additionalFields
object is guaranteed to be defined, so checking for its existence before accessing its keys is unnecessary.
Learnt from: siddhant-nawale-egov
PR: #204
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/Upload.js:468-481
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The error message in the checkForErrorInUploadedFile
function within Upload.js
is being localized and improved for better user experience, as clarified by the user.
Learnt from: Hari-egov
PR: #2788
File: micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js:157-162
Timestamp: 2025-07-15T06:50:54.535Z
Learning: In the HRMS module (micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js), localization is already working correctly when using ele.hierarchyType
directly in the gethierarchylistdata()
function. The suggested locale conversion using Digit.Utils.locale.convertToLocale(ele.hierarchyType, 'EGOV_LOCATION_BOUNDARYTYPE')
is not needed as the current implementation already provides proper localization.
Learnt from: Hari-egov
PR: #2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js
is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Learnt from: Hari-egov
PR: #2788
File: micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js:43-74
Timestamp: 2025-07-15T06:49:28.898Z
Learning: In the HRMS module (micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js), the current error handling approach in the fetchDetailsForSelectedOption
function is considered sufficient by the team. Enhanced error handling with user notifications and fallback behavior is not required for the boundary service API call.
Learnt from: Tulika-eGov
PR: #2188
File: micro-ui/web/micro-ui-internals/packages/modules/pgr/src/pages/employee/new-inbox.js:34-47
Timestamp: 2025-02-05T10:18:29.947Z
Learning: In the DIGIT-Frontend codebase, translations within FormComposer are handled internally by the component itself, so the translation function 't' does not need to be added to useMemo dependency arrays even when used within the memoized function that generates form configurations.
Learnt from: Ramkrishna-egov
PR: #1545
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/ActivityCard.js:1-1
Timestamp: 2024-10-17T11:59:00.327Z
Learning: In ActivityCard.js
, React.Fragment
is used in the code.
🔇 Additional comments (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/LanguageSelection/index.js (1)
30-32
: Navigation implementation looks good with cache busting.The dynamic path construction using the
getContextPath
helper and timestamp parameter for cache busting aligns well with the broader pattern mentioned in the AI summary. This approach prevents caching issues during navigation.
function getContextPath(contextPath) { | ||
if (!contextPath || typeof contextPath !== "string") return ""; | ||
return contextPath.split("/")[0]; | ||
} |
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.
🛠️ Refactor suggestion
Improve error handling and add input validation.
The getContextPath
function has potential issues with edge cases:
- It splits on "/" but doesn't handle cases where the first element might be empty (e.g., "/path/to/something" would return "")
- No validation that the returned path is meaningful
function getContextPath(contextPath) {
- if (!contextPath || typeof contextPath !== "string") return "";
- return contextPath.split("/")[0];
+ if (!contextPath || typeof contextPath !== "string") return "";
+ const segments = contextPath.split("/").filter(segment => segment.length > 0);
+ return segments.length > 0 ? segments[0] : "";
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
function getContextPath(contextPath) { | |
if (!contextPath || typeof contextPath !== "string") return ""; | |
return contextPath.split("/")[0]; | |
} | |
function getContextPath(contextPath) { | |
if (!contextPath || typeof contextPath !== "string") return ""; | |
const segments = contextPath.split("/").filter(segment => segment.length > 0); | |
return segments.length > 0 ? segments[0] : ""; | |
} |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/LanguageSelection/index.js
around lines 25 to 28, the getContextPath function does not handle cases where
the input string starts with a slash, causing it to return an empty string, and
it lacks validation for meaningful output. Update the function to trim leading
slashes before splitting and add validation to ensure the returned path segment
is non-empty and meaningful; if not, return an empty string or a default value.
* Update package.json * Update index.html
Co-authored-by: syedkaif866 <[email protected]>
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: 12
♻️ Duplicate comments (5)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js (5)
363-363
: Remove console.logDebug log in production code.
- console.log(`*** LOG ***`,config);
378-381
: Remove unused stateThese local states are not used; links are derived via getLinkByType.
- const [employeeWTLink, setEmployeeWTLink] = useState(''); - const [citizenWTLink, setCitizenWTLink] = useState(''); - const [stakeholderWTLink, setStakeholderWTLink] = useState('');
336-358
: DRY: unify getImageByType/getLinkByTypeBoth duplicate traversal logic. Introduce a generic helper.
-function getImageByType(config, type) { - const contentArray = config[0]?.subsections?.[1]?.content; - - if (!Array.isArray(contentArray)) { - return null; - } - - const imageObject = contentArray.find(item => item.type === type); - - return imageObject?.text || null; -} - -function getLinkByType(config, type) { - const contentArray = config[0]?.subsections?.[1]?.content; - - if (!Array.isArray(contentArray)) { - return null; - } - - const imageObject = contentArray.find(item => item.type === type); - - return imageObject?.link || null; -} +function getContentByType(config, type, key) { + const contentArray = config[0]?.subsections?.[1]?.content; + if (!Array.isArray(contentArray)) return null; + const entry = contentArray.find(item => item.type === type); + return entry?.[key] || null; +} +const getImageByType = (config, type) => getContentByType(config, type, "text"); +const getLinkByType = (config, type) => getContentByType(config, type, "link");
150-150
: Add explicit type to buttonAvoid defaulting to submit.
- <button + <button type="button"
111-111
: Fix invalid default parameter for t
t = { t }
is a syntax error and triggers TDZ issues. Acceptt
as a plain prop.-const ExperienceSection = ({ experience, t = { t } }) => ( +const ExperienceSection = ({ experience, t }) => (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
micro-ui/web/micro-ui-internals/example/public/index.html
(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
(2 hunks)micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
(1 hunks)micro-ui/web/public/index.html
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit Configuration File
check
Files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
🧠 Learnings (43)
📓 Common learnings
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in `micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js` is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:279-280
Timestamp: 2025-06-26T10:21:42.619Z
Learning: In the sandbox ProductDetailsComponentUpdated.js component, string concatenation is intentionally used for translation keys (e.g., `t(`${module}` + "_SECTION1_ROLE_1")`) instead of template literals for easier config identification and to make the key structure more explicit.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js:24-24
Timestamp: 2025-06-26T09:43:00.597Z
Learning: In LoginSignupSelector component, the URL checking using window.location.href.includes("sandbox-ui/user/sign-up") is intentional for URL validation purposes, not navigation. The team prefers this approach over React Router's useLocation hook for specific URL checking needs.
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss:1642-1662
Timestamp: 2025-06-26T10:24:08.628Z
Learning: In the sandbox SCSS file, tab styling with margin-bottom and border changes that cause layout shifts are intentional design requirements per updated UI specifications, not bugs to be fixed.
📚 Learning: 2024-10-26T15:11:49.938Z
Learnt from: nabeelmd-eGov
PR: egovernments/DIGIT-Frontend#1680
File: health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js:1032-1094
Timestamp: 2024-10-26T15:11:49.938Z
Learning: In the DIGIT-Frontend project, when reviewing the `UICustomizations.js` file, avoid suggesting refactoring of repetitive rendering logic in switch cases into helper functions, unless explicitly requested.
Applied to files:
micro-ui/web/public/index.html
micro-ui/web/micro-ui-internals/example/public/index.html
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-02-05T10:26:52.452Z
Learnt from: Tulika-eGov
PR: egovernments/DIGIT-Frontend#2188
File: micro-ui/web/micro-ui-internals/packages/modules/pgr/src/configs/UICustomizations.js:22-130
Timestamp: 2025-02-05T10:26:52.452Z
Learning: UI customizations in DIGIT-Frontend should not be restructured at this time as a new methodology for formcomposer will be implemented in the future.
Applied to files:
micro-ui/web/public/index.html
micro-ui/web/micro-ui-internals/example/public/index.html
📚 Learning: 2024-06-10T19:25:42.992Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#698
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanPreview.js:1-1
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The imports in `MicroplanPreview.js` are from different libraries: `egovernments/digit-ui-components` and `egovernments/digit-ui-react-components`.
Applied to files:
micro-ui/web/public/index.html
micro-ui/web/micro-ui-internals/example/public/index.html
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-07-15T06:50:54.535Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2788
File: micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js:157-162
Timestamp: 2025-07-15T06:50:54.535Z
Learning: In the HRMS module (micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js), localization is already working correctly when using `ele.hierarchyType` directly in the `gethierarchylistdata()` function. The suggested locale conversion using `Digit.Utils.locale.convertToLocale(ele.hierarchyType, 'EGOV_LOCATION_BOUNDARYTYPE')` is not needed as the current implementation already provides proper localization.
Applied to files:
micro-ui/web/public/index.html
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-06-26T10:17:02.717Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Otp/index.js:0-0
Timestamp: 2025-06-26T10:17:02.717Z
Learning: In the DIGIT Frontend project, banner images and similar configuration data should be managed through MDMS (Master Data Management Service) rather than hardcoded arrays or separate config files, as indicated by the user Hari-egov.
Applied to files:
micro-ui/web/public/index.html
📚 Learning: 2025-06-26T10:16:36.004Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/css/src/components/Carousel.scss:0-0
Timestamp: 2025-06-26T10:16:36.004Z
Learning: In the DIGIT-Frontend codebase, signup-container CSS classes cannot be namespaced under parent classes due to existing style constraints and requirements.
Applied to files:
micro-ui/web/public/index.html
micro-ui/web/micro-ui-internals/example/public/index.html
📚 Learning: 2025-06-26T10:24:08.628Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss:1642-1662
Timestamp: 2025-06-26T10:24:08.628Z
Learning: In the sandbox SCSS file, tab styling with margin-bottom and border changes that cause layout shifts are intentional design requirements per updated UI specifications, not bugs to be fixed.
Applied to files:
micro-ui/web/public/index.html
micro-ui/web/micro-ui-internals/example/public/index.html
📚 Learning: 2024-06-10T19:25:42.992Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#0
File: :0-0
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The Button component from `egovernments/digit-ui-components` has compatibility issues with the ActionBar, leading to the preference for using the Button from `egovernments/digit-ui-react-components` in certain scenarios.
Applied to files:
micro-ui/web/public/index.html
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-10-08T20:11:12.539Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#691
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js:16-17
Timestamp: 2024-10-08T20:11:12.539Z
Learning: The components `Loader`, `Toast`, and `Modal` are from different libraries in the DIGIT-Frontend project, as clarified by the user.
Applied to files:
micro-ui/web/public/index.html
micro-ui/web/micro-ui-internals/example/public/index.html
📚 Learning: 2024-06-14T14:10:20.359Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#876
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js:3-39
Timestamp: 2024-06-14T14:10:20.359Z
Learning: The `CustomScaleControl` component in the `micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js` file should not include a "loading map" message as there is another component handling this functionality to avoid redundancy.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
micro-ui/web/micro-ui-internals/example/public/index.html
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-06-27T05:52:33.156Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in `micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js` is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
micro-ui/web/micro-ui-internals/example/public/index.html
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-06-26T10:21:42.619Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:279-280
Timestamp: 2025-06-26T10:21:42.619Z
Learning: In the sandbox ProductDetailsComponentUpdated.js component, string concatenation is intentionally used for translation keys (e.g., `t(`${module}` + "_SECTION1_ROLE_1")`) instead of template literals for easier config identification and to make the key structure more explicit.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-06-10T19:25:42.992Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#204
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/Upload.js:468-481
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The error message in the `checkForErrorInUploadedFile` function within `Upload.js` is being localized and improved for better user experience, as clarified by the user.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
📚 Learning: 2024-06-10T19:25:42.992Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#606
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js:276-276
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The `addResourcesToFilteredDataToShow` function in `micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js` now includes checks for data integrity, such as validation of `previewData`, the common column, and `resources`, along with error logging for invalid data formats.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-10-17T12:00:51.749Z
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1545
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/ActivityCard.js:30-35
Timestamp: 2024-10-17T12:00:51.749Z
Learning: In our codebase, the 'Card' component is commonly used by passing children via the 'children' prop.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
📚 Learning: 2024-10-17T11:59:00.327Z
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1545
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/ActivityCard.js:1-1
Timestamp: 2024-10-17T11:59:00.327Z
Learning: In `ActivityCard.js`, `React.Fragment` is used in the code.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-11-14T12:48:23.023Z
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.023Z
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.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
📚 Learning: 2024-06-14T14:10:38.086Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#876
File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:1940-2392
Timestamp: 2024-06-14T14:10:38.086Z
Learning: Classes related to interactive elements in the microplan preview section are mostly passed to Higher Order Components (HOCs), and ARIA attributes for non-HOC elements will be managed directly by adding them where necessary.
Applied to files:
micro-ui/web/micro-ui-internals/example/public/index.html
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-10-08T20:11:07.772Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#876
File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:86-97
Timestamp: 2024-10-08T20:11:07.772Z
Learning: The use of `!important` in the `.modal-header` CSS class within `microplanning.scss` is necessary to ensure the styles are applied as intended and are not overridden by other styles.
Applied to files:
micro-ui/web/micro-ui-internals/example/public/index.html
📚 Learning: 2024-06-10T19:25:42.992Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#675
File: micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss:33-58
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The use of `!important` in the `.wbh-header-container` and `.guideline-actionbar-content` CSS classes within `microplanning.scss` is necessary to prevent these styles from being overridden.
Applied to files:
micro-ui/web/micro-ui-internals/example/public/index.html
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-11-07T11:02:33.520Z
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1770
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js:320-322
Timestamp: 2024-11-07T11:02:33.520Z
Learning: In `health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js`, the `data?.additionalFields` object is guaranteed to be defined, so checking for its existence before accessing its keys is unnecessary.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-07-15T04:58:30.634Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2788
File: micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js:26-26
Timestamp: 2025-07-15T04:58:30.634Z
Learning: In the HRMS module (micro-ui/web/micro-ui-internals/packages/modules/hrms/src/components/pageComponents/jurisdiction.js), the team has moved away from using MDMS hooks for boundary data fetching and now uses direct API calls to the boundary service endpoint `/boundary-service/boundary-relationships/_search` instead of the `useHrmsMDMS` hook with "FetchBoundaries" parameter.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-11-07T07:17:27.636Z
Learnt from: jagankumar-egov
PR: egovernments/DIGIT-Frontend#1763
File: health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/MyCampaign.js:65-75
Timestamp: 2024-11-07T07:17:27.636Z
Learning: In `MyCampaign.js`, when suggesting code improvements for the `onClickRow` function, keep suggestions simple and avoid unnecessary complexity.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-01-02T04:41:15.957Z
Learnt from: jagankumar-egov
PR: egovernments/DIGIT-Frontend#2089
File: micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js:111-111
Timestamp: 2025-01-02T04:41:15.957Z
Learning: In micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js, always use "mdms[0].uniqueIdentifier" for the jsonPath instead of differentiating between "SOR" and other masters.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-06-04T04:08:48.234Z
Learnt from: jagankumar-egov
PR: egovernments/DIGIT-Frontend#2488
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js:152-152
Timestamp: 2025-06-04T04:08:48.234Z
Learning: In the employee login component (micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js), prefer using a `defaultTenant` variable to store `Digit.ULBService.getStateId()` instead of calling the function multiple times inline in the `defaultValue` object. This improves code readability and avoids function call duplication.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-10-08T20:11:12.539Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#876
File: micro-ui/web/micro-ui-internals/packages/css/postcss.config.js:3-13
Timestamp: 2024-10-08T20:11:12.539Z
Learning: Retain commented-out code in `postcss.config.js` as it might be needed for future configurations or debugging.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-11-18T04:35:51.535Z
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.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-11-07T11:00:11.847Z
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1770
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/timelinePopUpWrapper.js:36-36
Timestamp: 2024-11-07T11:00:11.847Z
Learning: In the `TimelinePopUpWrapper` component in `timelinePopUpWrapper.js`, the `action` property of `instance` is a required field and will always be defined; thus, checks for undefined values for `instance.action` are unnecessary.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-04-21T05:15:36.122Z
Learnt from: jagankumar-egov
PR: egovernments/DIGIT-Frontend#2342
File: health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CreateCampaignComponents.js/DateSelection.js:41-47
Timestamp: 2025-04-21T05:15:36.122Z
Learning: In React components, prefer using state setters directly in event handlers rather than creating wrapper functions that only call the setter without additional logic.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-10-22T12:51:42.266Z
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1622
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/PopInboxTable.js:355-363
Timestamp: 2024-10-22T12:51:42.266Z
Learning: In `PopInboxTable.js`, the `showEditVillagePopup` state variable is initialized as an empty object.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-06-10T19:25:42.992Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#698
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/resourceMapping.js:74-74
Timestamp: 2024-06-10T19:25:42.992Z
Learning: `onKeyPress` is deprecated and should be replaced with `onKeyDown` or `onKeyUp` for handling keyboard interactions.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-06-10T19:25:42.992Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#0
File: :0-0
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The `onStepClick` logic in `CreateMicroplan.js` is intentionally commented out to allow developers to navigate between screens during development, even though not all screens are made yet.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-10-22T09:14:46.876Z
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1612
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/WorkflowCommentPopUp.js:37-41
Timestamp: 2024-10-22T09:14:46.876Z
Learning: In this project, the `TextArea` component should submit the form when the 'Enter' key is pressed.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-02-05T10:05:37.433Z
Learnt from: Tulika-eGov
PR: egovernments/DIGIT-Frontend#2188
File: micro-ui/web/micro-ui-internals/packages/libraries/src/hooks/pgr/useComplaintSubType.js:14-14
Timestamp: 2025-02-05T10:05:37.433Z
Learning: In PGR's useComplaintSubType hook, the translation function 't' must be included in the useEffect dependency array to ensure complaint types are updated when the language changes.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-02-05T10:18:29.947Z
Learnt from: Tulika-eGov
PR: egovernments/DIGIT-Frontend#2188
File: micro-ui/web/micro-ui-internals/packages/modules/pgr/src/pages/employee/new-inbox.js:34-47
Timestamp: 2025-02-05T10:18:29.947Z
Learning: In the DIGIT-Frontend codebase, translations within FormComposer are handled internally by the component itself, so the translation function 't' does not need to be added to useMemo dependency arrays even when used within the memoized function that generates form configurations.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-09-30T05:45:14.549Z
Learnt from: ashish-egov
PR: egovernments/DIGIT-Frontend#1441
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/hooks/useGenerateIdCampaign.js:1-1
Timestamp: 2024-09-30T05:45:14.549Z
Learning: In the `useGenerateIdCampaign` function, auto assignment of default parameter values is needed in parameter destructuring.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-11-28T11:34:27.480Z
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1925
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js:267-269
Timestamp: 2024-11-28T11:34:27.480Z
Learning: In the `downloadTemplate` function of `health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/UserUpload.js`, the scenario where `state?.hierarchyType` is undefined will not occur.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-06-10T19:25:42.992Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#675
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanPreview.js:4-10
Timestamp: 2024-06-10T19:25:42.992Z
Learning: Imports in `MicroplanPreview.js` are either already grouped appropriately or come from different directories, as clarified by the user.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-06-10T19:25:42.992Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#0
File: :0-0
Timestamp: 2024-06-10T19:25:42.992Z
Learning: The pagination SVG components (`PaginationPrevious`, `PaginationNext`, `PaginationLast`, and `PaginationFirst`) are correctly listed once in the export block of the `index.js` file within the `svg-components` package, without any duplications.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-10-08T20:11:12.539Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#675
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js:675-675
Timestamp: 2024-10-08T20:11:12.539Z
Learning: The modal component from the external library used in `Mapping.js` does not support the `className` property, and thus styles cannot be refactored into a CSS module.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2024-12-12T05:38:22.901Z
Learnt from: Bhavya-egov
PR: egovernments/DIGIT-Frontend#2013
File: health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/utils/jsonReader.js:24-27
Timestamp: 2024-12-12T05:38:22.901Z
Learning: In the `jsonReader` function within `health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/utils/jsonReader.js`, generic error handling is sufficient, and enhancing error handling with specific error types is not necessary.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
📚 Learning: 2025-03-25T10:47:42.433Z
Learnt from: Tulika-eGov
PR: egovernments/DIGIT-Frontend#2288
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Landing/LandingComponent.js:18-24
Timestamp: 2025-03-25T10:47:42.433Z
Learning: In the DIGIT-Frontend project, commented-out code like `//config = defaultconfig;` is sometimes intentionally kept when it serves a development purpose, such as providing a way to quickly switch between configurations during local testing.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
🪛 Biome (2.1.2)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
[error] 247-252: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 247-252: 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)
[error] 111-111: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 80-80: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 104-104: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
[error] 150-165: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form
element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
⏰ Context from checks skipped due to timeout of 10000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
micro-ui/web/public/index.html (2)
56-56
: No actionable changeClosing tag change appears to be formatting/no-op. Nothing to do.
10-10
: Bumped base CSS to 1.8.36 — CDN availability confirmed; validate UI
- CDN check passed (HTTP/2 200), so
@egovernments/[email protected]
is available.- Next step: run visual-regression tests (or manual QA) on the login, signup, and sandbox pages to catch any styling regressions.
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js (1)
76-82
: Sanitize HTML before using dangerouslySetInnerHTMLDecoding + injecting HTML can enable XSS if translations/config are compromised. Sanitize the decoded HTML.
- {about.paragraphs.map((para, i) => { - const decodedpara = he.decode(para); // decode <strong> - - return ( - <p key={i} className="about-description" dangerouslySetInnerHTML={{ __html: decodedpara }}></p> - ) - })} + {about.paragraphs.map((para, i) => { + const decoded = he.decode(para); + const sanitized = DOMPurify.sanitize(decoded, { ALLOWED_TAGS: ["strong","em","b","i","u","br"], ALLOWED_ATTR: [] }); + return <p key={i} className="about-description" dangerouslySetInnerHTML={{ __html: sanitized }} />; + })}- const decodedDescription = he.decode(description); // decode <strong> + const decodedDescription = he.decode(description); + const sanitizedDescription = DOMPurify.sanitize(decodedDescription, { ALLOWED_TAGS: ["strong","em","b","i","u","br"], ALLOWED_ATTR: [] }); @@ - <p dangerouslySetInnerHTML={{ __html: decodedDescription }} /> + <p dangerouslySetInnerHTML={{ __html: sanitizedDescription }} />Additional import to add near other imports:
+import DOMPurify from "dompurify";
Also applies to: 100-106
⛔ Skipped due to learnings
Learnt from: Hari-egov PR: egovernments/DIGIT-Frontend#2644 File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:78-78 Timestamp: 2025-06-26T10:20:33.997Z Learning: The team prefers to create custom library solutions for HTML sanitization rather than using third-party libraries like DOMPurify for dangerouslySetInnerHTML security concerns in the DIGIT-Frontend codebase.
Learnt from: Hari-egov PR: egovernments/DIGIT-Frontend#2644 File: micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js:279-280 Timestamp: 2025-06-26T10:21:42.619Z Learning: In the sandbox ProductDetailsComponentUpdated.js component, string concatenation is intentionally used for translation keys (e.g., `t(`${module}` + "_SECTION1_ROLE_1")`) instead of template literals for easier config identification and to make the key structure more explicit.
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js (1)
62-62
: LGTM on exportDefault export is fine and matches module usage elsewhere.
<style> | ||
.digit-header-content.label .info-icon .infotext { | ||
position: absolute; | ||
text-align: center; | ||
font-family: Roboto; | ||
font-style: normal; | ||
font-weight: 400; | ||
line-height: 1.37rem; | ||
visibility: hidden; | ||
width: 7.5rem; | ||
background-color: #555; | ||
color: #fff; | ||
padding: .5rem 0; | ||
z-index: 1; | ||
bottom: 100%; | ||
left: 50%; | ||
margin-left: -6.5rem; | ||
border-radius: .375rem; | ||
} | ||
|
||
</style> |
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.
🧹 Nitpick (assertive)
🛠️ Refactor suggestion
Style tag is outside the (invalid HTML) + brittle centering
Same issues as the main public index:
- Move the <style> into .
- Replace margin-based centering with transform for correctness.
- Add font fallback.
Apply this diff to remove the misplaced style:
-<style>
- .digit-header-content.label .info-icon .infotext {
- position: absolute;
- text-align: center;
- font-family: Roboto;
- font-style: normal;
- font-weight: 400;
- line-height: 1.37rem;
- visibility: hidden;
- width: 7.5rem;
- background-color: #555;
- color: #fff;
- padding: .5rem 0;
- z-index: 1;
- bottom: 100%;
- left: 50%;
- margin-left: -6.5rem;
- border-radius: .375rem;
-}
-
-</style>
Insert inside :
<style>
.digit-header-content.label .info-icon .infotext {
position: absolute;
text-align: center;
font-family: "Roboto", Arial, sans-serif;
font-style: normal;
font-weight: 400;
line-height: 1.37rem;
visibility: hidden;
width: 7.5rem;
background-color: #555;
color: #fff;
padding: .5rem 0;
z-index: 1;
bottom: 100%;
left: 50%;
transform: translateX(-50%);
border-radius: .375rem;
}
</style>
Also consider centralizing this rule in a common stylesheet to prevent drift between entry points.
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/example/public/index.html between lines 21
and 41, the <style> tag is incorrectly placed outside the <head> section, which
is invalid HTML, and the centering uses a brittle margin-left approach. Move the
entire <style> block inside the <head> tag, replace the margin-left centering
with transform: translateX(-50%) for proper horizontal centering, and update the
font-family to include fallbacks like Arial and sans-serif after Roboto.
Optionally, consider moving this CSS rule to a shared stylesheet to maintain
consistency across entry points.
const children = [ | ||
<div> | ||
<CardText> | ||
{t("CORE_LOGOUT_WEB_CONFIRMATION_MESSAGE") + " "} | ||
<strong>{t("CORE_LOGOUT_MESSAGE")}</strong> | ||
{t("CORE_LOGOUTPOPUP_CONFIRM") + " "} | ||
</CardText> | ||
</div>, | ||
]; |
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.
🧹 Nitpick (assertive)
Avoid array wrapper for a single child (or add a key)
Cleaner as a ReactNode; if you keep the array style, add a key to the element to prevent potential React warnings.
Option A (preferred):
- const children = [
- <div>
- <CardText>
- {t("CORE_LOGOUTPOPUP_CONFIRM")}
- </CardText>
- </div>,
- ];
+ const children = (
+ <div>
+ <CardText>{t("CORE_LOGOUTPOPUP_CONFIRM")}</CardText>
+ </div>
+ );
Option B (keep array style, add key):
- const children = [
- <div>
+ const children = [
+ <div key="logout-confirmation">
<CardText>{t("CORE_LOGOUTPOPUP_CONFIRM")}</CardText>
</div>,
];
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
around lines 8 to 14, the children variable is assigned an array with a single
React element without a key, which can cause React warnings. To fix this, either
remove the array wrapper and assign the single React element directly to
children, or if you want to keep the array, add a unique key prop to the div
element inside the array.
@@ -8,8 +8,7 @@ const LogoutDialog = ({ onSelect, onCancel, onDismiss, PopupStyles, isDisabled, | |||
const children = [ | |||
<div> | |||
<CardText> | |||
{t("CORE_LOGOUT_WEB_CONFIRMATION_MESSAGE") + " "} | |||
<strong>{t("CORE_LOGOUT_MESSAGE")}</strong> | |||
{t("CORE_LOGOUTPOPUP_CONFIRM") + " "} |
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.
💡 Verification agent
🧩 Analysis chain
Drop the hardcoded trailing space; rely on the i18n string
Avoid concatenating " ". Keep spacing/punctuation inside the translation to prevent brittle UI.
Apply:
- {t("CORE_LOGOUTPOPUP_CONFIRM") + " "}
+ {t("CORE_LOGOUTPOPUP_CONFIRM")}
Run to verify i18n keys exist and old ones aren’t lingering:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking required translation keys:"
keys=(
'CORE_LOGOUTPOPUP_CONFIRM'
'CORE_LOGOUT_WEB_HEADER'
'CORE_LOGOUT_CANCEL'
'CORE_LOGOUT_WEB_YES'
)
for k in "${keys[@]}"; do
hits=$(rg -n --hidden -S "\"$k\"" | wc -l | tr -d ' ')
if [ "$hits" -eq 0 ]; then
echo "MISSING: $k"
else
echo "FOUND ($hits): $k"
fi
done
echo
echo "Searching for deprecated keys (if any remain):"
deprecated=(
'CORE_LOGOUT_WEB_CONFIRMATION_MESSAGE'
'CORE_LOGOUT_MESSAGE'
)
for k in "${deprecated[@]}"; do
rg -n --hidden -S "$k" || true
done
Length of output: 1339
I’m verifying the children prop structure in LogoutDialog.js before finalizing the comment.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file
file="micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js"
# Show lines around where children are defined/exported
echo "Showing 'children' usage in $file:"
rg -n "children" "$file" -A3 || echo "No explicit 'children' found"
Length of output: 773
Remove concatenation and simplify children prop
All translation keys are present and no deprecated ones remain. To prevent React warnings and improve clarity,
- Drop the hard-coded
" "
—keep spacing inside the i18n string. - Refactor the single-element
children
array into a plain ReactNode (or add a key if you must keep the array).
Locations to update:
- micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
Suggested changes:
@@ lines 8–11
- const children = [
- <div>
- <CardText>
- {t("CORE_LOGOUTPOPUP_CONFIRM") + " "}
- </CardText>
- </div>
- ];
+ const children = (
+ <div>
+ <CardText>
+ {t("CORE_LOGOUTPOPUP_CONFIRM")}
+ </CardText>
+ </div>
+ );
@@ around line 11
- {t("CORE_LOGOUTPOPUP_CONFIRM") + " "}
+ {t("CORE_LOGOUTPOPUP_CONFIRM")}
(You can also inline `children={…}` directly in the component props if preferred.)
> Committable suggestion skipped: line range outside the PR's diff.
<details>
<summary>🤖 Prompt for AI Agents</summary>
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/Dialog/LogoutDialog.js
at line 11, remove the string concatenation of a space after the translation key
t("CORE_LOGOUTPOPUP_CONFIRM") and instead keep spacing inside the i18n string
itself. Also, refactor the children prop from a single-element array to a plain
ReactNode by replacing the array brackets with parentheses, or alternatively
inline the children directly in the component props for clarity and to avoid
React warnings.
</details>
<!-- fingerprinting:phantom:triton:capybara -->
<!-- This is an auto-generated comment by CodeRabbit -->
{cards.map(({ icon, text }, idx) => { | ||
const IconComponent = iconMap[icon]; | ||
return ( | ||
<div key={idx} className="cs-card"> | ||
<IconComponent className="cs-icon" /> | ||
<span>{text}</span> | ||
</div> |
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.
🛠️ Refactor suggestion
Provide a safe fallback for unknown icon names
Avoid runtime errors if an unexpected icon key appears.
- {cards.map(({ icon, text }, idx) => {
- const IconComponent = iconMap[icon];
+ {cards.map(({ icon, text }, idx) => {
+ const IconComponent = iconMap[icon] || Graph;
return (
<div key={idx} className="cs-card">
<IconComponent className="cs-icon" />
<span>{text}</span>
</div>
);
})}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
{cards.map(({ icon, text }, idx) => { | |
const IconComponent = iconMap[icon]; | |
return ( | |
<div key={idx} className="cs-card"> | |
<IconComponent className="cs-icon" /> | |
<span>{text}</span> | |
</div> | |
{cards.map(({ icon, text }, idx) => { | |
const IconComponent = iconMap[icon] || Graph; | |
return ( | |
<div key={idx} className="cs-card"> | |
<IconComponent className="cs-icon" /> | |
<span>{text}</span> | |
</div> | |
); | |
})} |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
around lines 141 to 147, the code directly uses iconMap[icon] without checking
if the icon exists, which can cause runtime errors for unknown icon keys. Fix
this by providing a safe fallback component or null when the icon key is not
found in iconMap before rendering IconComponent.
if (config.isExternal) { | ||
window.open(config?.action, "_blank"); | ||
} else { | ||
handleButtonClick(config?.action); | ||
} |
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.
Harden window.open to prevent reverse tabnabbing
Use noopener/noreferrer and clear opener for both external and internal opens.
- if (config.isExternal) {
- window.open(config?.action, "_blank");
- } else {
+ if (config.isExternal) {
+ const w = window.open(config?.action, "_blank", "noopener,noreferrer");
+ if (w) w.opener = null;
+ } else {
handleButtonClick(config?.action);
}
-const handleButtonClick = (action) => {
- const url = '/' + window.contextPath + action + "?from=sandbox";
- window.open(url, "_blank");
-};
+const handleButtonClick = (action) => {
+ const url = '/' + window.contextPath + action + "?from=sandbox";
+ const w = window.open(url, "_blank", "noopener,noreferrer");
+ if (w) w.opener = null;
+};
Also applies to: 330-333
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
around lines 156 to 160 and also lines 330 to 333, the window.open call is
vulnerable to reverse tabnabbing. To fix this, update window.open to include
"noopener noreferrer" in the window features string and explicitly set the
opened window's opener property to null. Apply this hardening for both external
and internal opens to prevent security risks.
const content = { | ||
heroTitle: "Local Business License Issuing System", | ||
heroHeadline: [ | ||
{ text: "Get your", color: "#215B73" }, | ||
{ text: "business license issued", color: "#C84C0E" }, | ||
{ text: "easily", color: "#215B73" }, | ||
], | ||
about: { | ||
title: "About Local Business License Issuing System", | ||
paragraphs: [ | ||
"The Local Business License Issuing system is an end-to-end product that enables <strong>businesses to apply for licenses</strong> and <strong>governments to ensure compliance and issue license certificates</strong> based on local regulations.", | ||
"Each product on DIGIT allows specific roles to be assigned to each user. The default workflow has two types of users: Citizen and Government Employee." | ||
], | ||
roles: [ | ||
{ description: "Citizens can apply online or with help of counter employee at a service counter and make payment." }, | ||
{ description: "Government employees can verify documents, log field inspections and take action—such as approving, rejecting, or sending applications back—based on predefined rules." }, | ||
] | ||
}, | ||
experience: { | ||
title: "Experience Local Business Issuing System", | ||
description: "On Sandbox, the government employee has been set up as a super-user, with all employee roles being assigned to it. For each user, a new browser tab will open with the appropriate interface. Please do not share any sensitive or private details as this instance of Local Business License Issuing System is shared across users.", | ||
roles: [ | ||
{ | ||
role: "Citizens", | ||
// imageUrl: "https://digit-sandbox-prod-s3.s3.ap-south-1.amazonaws.com/assets/b49b43c60c88ed87c0a54cf6dc06b26ce83c1bcf.png", | ||
reverse: false, | ||
cards: [ | ||
{ icon: "Graph", text: "Fill necessary details and upload relevant documents" }, | ||
{ icon: "Calculate", text: "Track application(s) until issuance" }, | ||
{ icon: "FeatureSearch", text: "Make dummy online payments" }, | ||
{ icon: "Chat", text: "Download sample license and payment receipts" }, | ||
{ icon: "BarChart", text: "Get notifications through emails and SMS" } | ||
] | ||
}, | ||
{ | ||
role: "Employee", | ||
// imageUrl: "https://digit-sandbox-prod-s3.s3.ap-south-1.amazonaws.com/assets/8ae2d85d61e5ca4df1c3e12602f2027e7e3b56bd.png", | ||
reverse: true, | ||
cards: [ | ||
{ icon: "Graph", text: "Track applications based on SLAs" }, | ||
{ icon: "Calculate", text: "Verify documents and conduct field verification" }, | ||
{ icon: "FeatureSearch", text: "Take action at each step and assign forward" }, | ||
{ icon: "Chat", text: "Leave comments on document verification and field verification" }, | ||
{ icon: "BarChart", text: "Review performance through the Dashboard" } | ||
] | ||
} | ||
] | ||
} | ||
}; |
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.
🧹 Nitpick (assertive)
🛠️ Refactor suggestion
Avoid mutating a module-scoped object based on props
content
is defined at module scope and then mutated using per-render props. This can leak state across renders/instances and cause hard-to-debug bugs. Derive content inside the component (useMemo) instead.
High-level refactor (example):
// Remove the module-level `content` constant.
function buildContentFromConfig(config, module, t) {
const base = {
heroTitle: t(config[0].heading),
heroHeadline: [
{ text: t(`${module}` + "_SECTION2_HL1"), color: "#215B73" },
{ text: t(`${module}` + "_SECTION2_HL2"), color: "#C84C0E" },
{ text: t(`${module}` + "_SECTION2_HL3"), color: "#215B73" },
],
about: { title: t(config[0].subsections[0].title), paragraphs: [], roles: [] },
experience: { title: t(config[0].subsections[2].title), description: "", roles: [] },
};
base.about.paragraphs = config[0].subsections[0].content
.filter(c => c.type === "paragraph")
.map(c => t(c.text));
const roleItems = config[0].subsections[0].content.filter(c => c.type === "role");
base.about.roles = roleItems.length
? roleItems.map(c => ({ description: t(c.text || "Data missing") }))
: [{ description: t(`${module}` + "_SECTION1_ROLE_1") }, { description: t(`${module}` + "_SECTION1_ROLE_2") }];
const heroHeadlineItems = config[0].subsections[1].content.filter(c => c.type === "heroHeadline");
if (heroHeadlineItems.length) {
base.heroHeadline = base.heroHeadline.map((item, idx) => ({
text: t(heroHeadlineItems[idx]?.text || "Data missing"),
color: item.color,
}));
}
const descParagraphs = config[0].subsections[2].content
.filter(c => c.type === "paragraph")
.map(c => t(c.text));
base.experience.description = descParagraphs.join(" ");
// roles are still computed as you do below (after applying the robustness fixes)
return base;
}
// Inside ProductDetailsComponentUpdated:
const content = React.useMemo(() => buildContentFromConfig(config, module, t), [config, module, t]);
// ...then compute roles and set `content.experience.roles = roles;`
I can push a concrete patch if you want this change applied now.
Also applies to: 384-419, 421-465
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
around lines 280 to 328, the `content` object is defined at module scope and
mutated based on props, which risks leaking state across renders. To fix this,
remove the module-level `content` constant and instead derive the content inside
the component using React's useMemo hook. Implement a function that builds the
content object from props like config, module, and t, and call this function
inside useMemo with these dependencies. This ensures content is recomputed per
render without shared mutable state.
content.experience.title = `${t(config[0].subsections[2].title)}` | ||
content.experience.description = config[0].subsections[2].content.filter(c => c.type === "paragraph").map(c => `${t(c.text)}`) | ||
|
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.
Fix experience.description type (array -> string)
Currently it becomes an array and renders oddly. Join or pick the first.
- content.experience.title = `${t(config[0].subsections[2].title)}`
- content.experience.description = config[0].subsections[2].content.filter(c => c.type === "paragraph").map(c => `${t(c.text)}`)
+ content.experience.title = `${t(config[0].subsections[2].title)}`
+ const descParagraphs = config[0].subsections[2].content
+ .filter(c => c.type === "paragraph")
+ .map(c => t(c.text));
+ content.experience.description = descParagraphs.join(" ")
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
content.experience.title = `${t(config[0].subsections[2].title)}` | |
content.experience.description = config[0].subsections[2].content.filter(c => c.type === "paragraph").map(c => `${t(c.text)}`) | |
content.experience.title = `${t(config[0].subsections[2].title)}` | |
const descParagraphs = config[0].subsections[2].content | |
.filter(c => c.type === "paragraph") | |
.map(c => t(c.text)); | |
content.experience.description = descParagraphs.join(" ") |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
around lines 403 to 405, the experience.description is assigned an array due to
mapping over filtered content, which causes rendering issues. To fix this,
convert the array to a string by either joining the elements with a separator
like a space or selecting only the first element from the mapped array before
assigning it to experience.description.
{ text: t(`${module}` + "_SECTION2_HL3"), color: "#215B73" }, | ||
]; | ||
|
||
content.experience.roles = config[0].subsections[2].content.map(c => `${t(c.text)}`) |
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.
🧹 Nitpick (assertive)
Remove redundant assignment to content.experience.roles
You overwrite it later with the computed roles array. Drop this earlier assignment.
- content.experience.roles = config[0].subsections[2].content.map(c => `${t(c.text)}`)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
content.experience.roles = config[0].subsections[2].content.map(c => `${t(c.text)}`) |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
at line 419, remove the assignment to content.experience.roles because it is
overwritten later with the computed roles array. Simply delete this line to
avoid redundant code.
section.forEach(item => { | ||
const roleType = item.text?.split("_")[1].toLowerCase(); | ||
if (item.type === "step-heading") { | ||
currentRoleIndex++; | ||
roles[currentRoleIndex] = { | ||
role: t(item.text), | ||
imageUrl: roleConfigs[roleType].imageUrl, | ||
reverse: currentRoleIndex % 2 === 1, | ||
cards: [], | ||
config: { | ||
action: config[0].cards[currentRoleIndex].action, | ||
isExternal: config[0].cards[currentRoleIndex].isExternal, | ||
title: config[0].cards[currentRoleIndex].title, | ||
} | ||
}; | ||
} else if (item.type === "step" && currentRoleIndex > -1) { | ||
const icon = roleConfigs[roleType].defaultIcons[roles[currentRoleIndex].cards.length] || "Graph"; | ||
roles[currentRoleIndex].cards.push({ icon, text: t(item.text) }); | ||
} | ||
}); |
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.
Guard role type parsing and indexing into roleConfigs
Prevent crashes when text format or mapping changes; provide defaults.
- section.forEach(item => {
- const roleType = item.text?.split("_")[1].toLowerCase();
+ section.forEach(item => {
+ const roleKey = (typeof item.text === "string" && item.text.includes("_"))
+ ? item.text.split("_")[1].toLowerCase()
+ : "citizen";
+ const roleConf = roleConfigs[roleKey] || roleConfigs.citizen;
if (item.type === "step-heading") {
currentRoleIndex++;
roles[currentRoleIndex] = {
role: t(item.text),
- imageUrl: roleConfigs[roleType].imageUrl,
+ imageUrl: roleConf.imageUrl,
reverse: currentRoleIndex % 2 === 1,
cards: [],
config: {
action: config[0].cards[currentRoleIndex].action,
isExternal: config[0].cards[currentRoleIndex].isExternal,
title: config[0].cards[currentRoleIndex].title,
}
};
} else if (item.type === "step" && currentRoleIndex > -1) {
- const icon = roleConfigs[roleType].defaultIcons[roles[currentRoleIndex].cards.length] || "Graph";
+ const icon = (roleConf.defaultIcons[roles[currentRoleIndex].cards.length]) || "Graph";
roles[currentRoleIndex].cards.push({ icon, text: t(item.text) });
}
});
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
section.forEach(item => { | |
const roleType = item.text?.split("_")[1].toLowerCase(); | |
if (item.type === "step-heading") { | |
currentRoleIndex++; | |
roles[currentRoleIndex] = { | |
role: t(item.text), | |
imageUrl: roleConfigs[roleType].imageUrl, | |
reverse: currentRoleIndex % 2 === 1, | |
cards: [], | |
config: { | |
action: config[0].cards[currentRoleIndex].action, | |
isExternal: config[0].cards[currentRoleIndex].isExternal, | |
title: config[0].cards[currentRoleIndex].title, | |
} | |
}; | |
} else if (item.type === "step" && currentRoleIndex > -1) { | |
const icon = roleConfigs[roleType].defaultIcons[roles[currentRoleIndex].cards.length] || "Graph"; | |
roles[currentRoleIndex].cards.push({ icon, text: t(item.text) }); | |
} | |
}); | |
section.forEach(item => { | |
const roleKey = (typeof item.text === "string" && item.text.includes("_")) | |
? item.text.split("_")[1].toLowerCase() | |
: "citizen"; | |
const roleConf = roleConfigs[roleKey] || roleConfigs.citizen; | |
if (item.type === "step-heading") { | |
currentRoleIndex++; | |
roles[currentRoleIndex] = { | |
role: t(item.text), | |
imageUrl: roleConf.imageUrl, | |
reverse: currentRoleIndex % 2 === 1, | |
cards: [], | |
config: { | |
action: config[0].cards[currentRoleIndex].action, | |
isExternal: config[0].cards[currentRoleIndex].isExternal, | |
title: config[0].cards[currentRoleIndex].title, | |
} | |
}; | |
} else if (item.type === "step" && currentRoleIndex > -1) { | |
const icon = roleConf.defaultIcons[roles[currentRoleIndex].cards.length] || "Graph"; | |
roles[currentRoleIndex].cards.push({ icon, text: t(item.text) }); | |
} | |
}); |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/pages/employee/ProductDetails/ProductDetailsComponentUpdated.js
between lines 444 and 463, the code assumes item.text always contains an
underscore and roleConfigs always has a matching key, which can cause crashes if
the format changes or the key is missing. To fix this, add guards to safely
parse roleType by checking if item.text contains an underscore before splitting,
and verify roleConfigs[roleType] exists before accessing its properties. Provide
default values for imageUrl and defaultIcons to prevent runtime errors when the
expected data is missing.
micro-ui/web/public/index.html
Outdated
<style> | ||
.digit-header-content.label .info-icon .infotext { | ||
position: absolute; | ||
text-align: center; | ||
font-family: Roboto; | ||
font-style: normal; | ||
font-weight: 400; | ||
line-height: 1.37rem; | ||
visibility: hidden; | ||
width: 7.5rem; | ||
background-color: #555; | ||
color: #fff; | ||
padding: .5rem 0; | ||
z-index: 1; | ||
bottom: 100%; | ||
left: 50%; | ||
margin-left: -6.5rem; | ||
border-radius: .375rem; | ||
} | ||
|
||
</style> |
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.
🧹 Nitpick (assertive)
🛠️ Refactor suggestion
Style tag is outside the (invalid HTML) + brittle centering
- The <style> block appears after . Move it inside to avoid invalid markup and inconsistent browser behavior.
- Centering via left: 50% + margin-left: -6.5rem is incorrect for width: 7.5rem (should be -3.75rem). Prefer transform: translateX(-50%) for robustness.
- Add a font fallback for Roboto.
Apply this diff to remove the misplaced style block:
-<style>
- .digit-header-content.label .info-icon .infotext {
- position: absolute;
- text-align: center;
- font-family: Roboto;
- font-style: normal;
- font-weight: 400;
- line-height: 1.37rem;
- visibility: hidden;
- width: 7.5rem;
- background-color: #555;
- color: #fff;
- padding: .5rem 0;
- z-index: 1;
- bottom: 100%;
- left: 50%;
- margin-left: -6.5rem;
- border-radius: .375rem;
-}
-
-</style>
Add the corrected style inside the (just before ):
<style>
.digit-header-content.label .info-icon .infotext {
position: absolute;
text-align: center;
font-family: "Roboto", Arial, sans-serif;
font-style: normal;
font-weight: 400;
line-height: 1.37rem;
visibility: hidden;
width: 7.5rem;
background-color: #555;
color: #fff;
padding: .5rem 0;
z-index: 1;
bottom: 100%;
left: 50%;
transform: translateX(-50%);
border-radius: .375rem;
}
</style>
Optional: If this tooltip is used across multiple entry points, consider moving it to a shared stylesheet to avoid duplication.
🤖 Prompt for AI Agents
In micro-ui/web/public/index.html around lines 19 to 39, move the entire <style>
block inside the <head> section before the closing </head> tag to fix invalid
HTML structure. Update the CSS by replacing margin-left: -6.5rem with transform:
translateX(-50%) for correct horizontal centering of the 7.5rem wide element.
Also, add font fallbacks by changing font-family to "Roboto", Arial, sans-serif
for better font loading resilience.
Co-authored-by: syedkaif866 <[email protected]>
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: 11
🔭 Outside diff range comments (1)
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (1)
1129-1133
: Invalid CSS: quoted hex colorcolor: "#c84c0e" is a string and will be ignored. Remove quotes.
.highlight-sandbox { - color: "#c84c0e"; + color: #c84c0e; /* or your specific orange color code */ }
♻️ Duplicate comments (4)
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (4)
116-119
: Add mobile-safe viewport fallbacks (svh/dvh) for dynamic viewportsUsing 80vh/30vw can be jumpy on mobile with collapsing browser UI. Add svh/dvh and a min-size fallback as previously suggested.
.sandbox-onboarding-wrapper { - height: 80vh !important; + height: 80vh !important; + height: 80svh !important; /* mobile-safe */ - width: 30vw !important; + width: 30vw !important; + min-width: 400px; /* small-viewport fallback */
341-344
: Use CSS variables for color valuesReplace hardcoded #0B4B66 with a CSS variable for theming consistency.
- color: #0B4B66; + color: var(--digit-primary-color, #0B4B66); - font-size: 2rem; + font-size: clamp(1.5rem, 2vw + 1rem, 2rem);
268-273
: vh on mobile: add svh/dvh fallback for stable full-height sections100vh can be taller than the visible viewport on mobile. Add 100svh/100dvh for better UX.
@media (max-width: 768px) { .sandbox-signup-form { - height: 100vh !important; + height: 100vh !important; + height: 100svh !important; /* or 100dvh depending on your target */ align-items: center !important; display: flex !important; } }
1447-1462
: Externalize hardcoded colors into CSS variablesFor theming consistency, move #215B73 and #505A5F into variables and reference with var().
-.about-title { color: #215B73; } +.about-title { color: var(--digit-secondary-color, #215B73); } -.about-description { color: #505A5F; } +.about-description { color: var(--digit-text-secondary, #505A5F); }Consider defining these at :root if not already present.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
(10 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-26T10:24:08.628Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss:1642-1662
Timestamp: 2025-06-26T10:24:08.628Z
Learning: In the sandbox SCSS file, tab styling with margin-bottom and border changes that cause layout shifts are intentional design requirements per updated UI specifications, not bugs to be fixed.
Applied to files:
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
📚 Learning: 2025-06-26T10:16:36.004Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/css/src/components/Carousel.scss:0-0
Timestamp: 2025-06-26T10:16:36.004Z
Learning: In the DIGIT-Frontend codebase, signup-container CSS classes cannot be namespaced under parent classes due to existing style constraints and requirements.
Applied to files:
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
🔇 Additional comments (3)
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (3)
89-93
: Utility centering helper looks goodConcise, reusable flex-centering helper. No issues.
349-354
: Selector container looks fineSimple, clear flex alignment for login/signup selector; no issues.
1644-1701
: Tab styles acknowledged as intentional per UI specMargin/border changes that cause slight layout shifts are intentional for selected tab emphasis per prior spec decisions. No changes requested.
.employeeCard { | ||
height: 500px !important; | ||
width: 408px !important; | ||
height: 650px !important; | ||
width: 500px !important; | ||
|
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.
🛠️ Refactor suggestion
Avoid rigid fixed pixel dimensions; prefer responsive constraints
Fixed 500×650 with !important can overflow smaller screens. Use min()/max()/clamp() to preserve intent while staying responsive.
.employeeCard {
- height: 650px !important;
- width: 500px !important;
+ height: min(650px, 100svh) !important;
+ width: min(500px, 100%) !important;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
.employeeCard { | |
height: 500px !important; | |
width: 408px !important; | |
height: 650px !important; | |
width: 500px !important; | |
.employeeCard { | |
height: min(650px, 100svh) !important; | |
width: min(500px, 100%) !important; | |
} |
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
around lines 148 to 151, the .employeeCard rule currently forces height: 650px
!important and width: 500px !important which is too rigid for smaller screens;
replace the fixed pixel values with responsive constraints (for example use
clamp(), max-width, min-width and/or max-height/min-height) and remove the
!important flags so the component can scale — pick sensible min and max sizes
(e.g. width: clamp(280px, 40vw, 500px); height: clamp(320px, 60vh, 650px)) or
equivalent max-width/min-width rules and ensure parent/container styles handle
centering and overflow instead of using !important.
@media (max-width: 768px) { | ||
.sandbox-onboarding-wrapper { | ||
height: 80vh !important; | ||
width: 80vw !important; | ||
max-width: 500px !important; | ||
max-height: 650px !important; | ||
display: flex; | ||
-ms-flex-pack: justify; | ||
justify-content: space-between; | ||
} | ||
} |
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.
🧹 Nitpick (assertive)
Mirror viewport fallbacks on mobile onboarding wrapper
Same vh/vw concerns apply here. Add svh/dvh and sensible min/max.
@media (max-width: 768px) {
.sandbox-onboarding-wrapper {
- height: 80vh !important;
+ height: 80vh !important;
+ height: 80svh !important;
- width: 80vw !important;
+ width: 80vw !important;
+ max-width: 500px !important;
+ max-height: 650px !important;
}
}
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
around lines 181 to 191, the mobile onboarding wrapper uses vh/vw with
!important and lacks safe-area/viewport fallbacks and sensible min/max; replace
the raw vh/vw with a stack of viewport units (svh, lvh/dvh as appropriate) and
add sensible min-width/min-height and max-width/max-height limits to prevent
extreme sizes on mobile, remove or avoid unnecessary !important flags, and
ensure display and justify-content rules remain; implement the unit fallbacks in
order (svh, dvh, vh and similarly for width) and set explicit min/max dimensions
(e.g., min-height/min-width to avoid too-small containers and cap
max-width/max-height) so the wrapper behaves reliably across mobile browsers.
.digit-card-label.sandbox-custom-otp-subheader { | ||
font-family: "Roboto"; | ||
font-weight: 700; | ||
font-size: 1.5rem; | ||
text-align: center; | ||
margin-top: 2.5rem; | ||
margin-bottom: 3rem; | ||
font-family: Roboto; | ||
margin-top: 8rem !important; | ||
color: #0b4b66 !important; | ||
text-align: center !important; | ||
font-size: 2rem !important; | ||
font-weight: 700 !important; | ||
} |
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.
🛠️ Refactor suggestion
Clean up duplicate font-family, use theme vars for color, and reconsider large margins
- font-family declared twice; remove duplicate.
- Replace hardcoded color with CSS variable for theming.
- 8rem top margin can push content off-screen on small devices.
.digit-card-label.sandbox-custom-otp-subheader {
- font-family: "Roboto";
- margin-bottom: 3rem;
- font-family: Roboto;
- margin-top: 8rem !important;
- color: #0b4b66 !important;
+ font-family: "Roboto";
+ margin-bottom: 3rem;
+ margin-top: 2rem !important; /* more reasonable default; consider responsive adjustments */
+ color: var(--digit-primary-color, #0B4B66) !important;
text-align: center !important;
font-size: 2rem !important;
font-weight: 700 !important;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
.digit-card-label.sandbox-custom-otp-subheader { | |
font-family: "Roboto"; | |
font-weight: 700; | |
font-size: 1.5rem; | |
text-align: center; | |
margin-top: 2.5rem; | |
margin-bottom: 3rem; | |
font-family: Roboto; | |
margin-top: 8rem !important; | |
color: #0b4b66 !important; | |
text-align: center !important; | |
font-size: 2rem !important; | |
font-weight: 700 !important; | |
} | |
.digit-card-label.sandbox-custom-otp-subheader { | |
font-family: "Roboto"; | |
margin-bottom: 3rem; | |
margin-top: 2rem !important; /* more reasonable default; consider responsive adjustments */ | |
color: var(--digit-primary-color, #0B4B66) !important; | |
text-align: center !important; | |
font-size: 2rem !important; | |
font-weight: 700 !important; | |
} |
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
around lines 230 to 239, remove the duplicate font-family declaration, replace
the hardcoded color #0b4b66 with the appropriate CSS theme variable (e.g.
var(--theme-primary) or the project’s defined token), and reduce or make the top
margin responsive instead of using margin-top: 8rem !important (for example use
margin-top: 2rem and/or a media query to increase it on larger screens) while
removing unnecessary !important flags where possible.
.sandbox-signup-form { | ||
.label-container { | ||
|
||
.info-icon { | ||
position: relative; | ||
display: inline-flex; | ||
&:hover .infotext { | ||
visibility: visible; | ||
opacity: 1; | ||
} | ||
|
||
.infotext { | ||
visibility: hidden; | ||
opacity: 0; | ||
transition: opacity 0.3s; | ||
|
||
position: absolute; | ||
width: 15rem; | ||
background-color: #555; | ||
color: #fff; | ||
font-family: Roboto, sans-serif; | ||
font-style: normal; | ||
font-weight: 400; | ||
text-align: center; | ||
line-height: 1.25rem; | ||
padding: 0.5rem; | ||
border-radius: 0.375rem; | ||
z-index: 10; | ||
|
||
|
||
bottom: 125%; | ||
left: 50%; | ||
margin-left: -10.3rem; | ||
} | ||
} | ||
} | ||
} |
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.
🛠️ Refactor suggestion
Improve tooltip accessibility and interaction beyond hover
Currently visibility toggles only on :hover; add :focus-within for keyboard users, and consider reduced motion.
.sandbox-signup-form {
.label-container {
.info-icon {
position: relative;
display: inline-flex;
- &:hover .infotext {
+ &:hover .infotext,
+ &:focus-within .infotext {
visibility: visible;
opacity: 1;
}
.infotext {
visibility: hidden;
opacity: 0;
- transition: opacity 0.3s;
+ transition: opacity 0.3s;
+ @media (prefers-reduced-motion: reduce) {
+ transition: none;
+ }
Optionally add pointer-events: none to the tooltip if it overlaps the trigger and causes flicker.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
.sandbox-signup-form { | |
.label-container { | |
.info-icon { | |
position: relative; | |
display: inline-flex; | |
&:hover .infotext { | |
visibility: visible; | |
opacity: 1; | |
} | |
.infotext { | |
visibility: hidden; | |
opacity: 0; | |
transition: opacity 0.3s; | |
position: absolute; | |
width: 15rem; | |
background-color: #555; | |
color: #fff; | |
font-family: Roboto, sans-serif; | |
font-style: normal; | |
font-weight: 400; | |
text-align: center; | |
line-height: 1.25rem; | |
padding: 0.5rem; | |
border-radius: 0.375rem; | |
z-index: 10; | |
bottom: 125%; | |
left: 50%; | |
margin-left: -10.3rem; | |
} | |
} | |
} | |
} | |
.sandbox-signup-form { | |
.label-container { | |
.info-icon { | |
position: relative; | |
display: inline-flex; | |
&:hover .infotext, | |
&:focus-within .infotext { | |
visibility: visible; | |
opacity: 1; | |
} | |
.infotext { | |
visibility: hidden; | |
opacity: 0; | |
transition: opacity 0.3s; | |
@media (prefers-reduced-motion: reduce) { | |
transition: none; | |
} | |
position: absolute; | |
width: 15rem; | |
background-color: #555; | |
color: #fff; | |
font-family: Roboto, sans-serif; | |
font-style: normal; | |
font-weight: 400; | |
text-align: center; | |
line-height: 1.25rem; | |
padding: 0.5rem; | |
border-radius: 0.375rem; | |
z-index: 10; | |
bottom: 125%; | |
left: 50%; | |
margin-left: -10.3rem; | |
} | |
} | |
} | |
} |
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
around lines 278 to 314, the tooltip only toggles on :hover which excludes
keyboard users and ignores reduced-motion preferences; update the CSS so the
tooltip also becomes visible when the trigger receives keyboard focus (use
:focus-within on the label container or a :focus-visible rule on the trigger),
add pointer-events: none to the .infotext to prevent overlap flicker, and wrap
the opacity transition in a prefers-reduced-motion: reduce media query (or
disable transitions) so users who prefer reduced motion do not get animations.
.sandbox-signup-form .digit-field .label-field-wrapper { | ||
all: unset !important; | ||
grid-gap: .25rem !important; | ||
.digit-field{ | ||
margin-bottom: unset !important; | ||
} | ||
.digit-header-content .label-container .label-styles{ | ||
margin-bottom: 5px !important; | ||
} | ||
} |
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.
🛠️ Refactor suggestion
Avoid ‘all: unset’ nuking; use targeted resets and modern gap
Using all: unset will strip essential layout/typography, often causing brittle overrides. Also grid-gap is deprecated in favor of gap.
-.sandbox-signup-form .digit-field .label-field-wrapper {
- all: unset !important;
- grid-gap: .25rem !important;
+.sandbox-signup-form .digit-field .label-field-wrapper {
+ display: grid; /* or original intended display */
+ gap: .25rem !important;
+ padding: 0;
+ margin: 0;
+ background: transparent;
+ border: 0;
}
If the intention was only spacing reset, constrain overrides to margin/padding/border/background and keep display/flow intact.
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
around lines 320 to 329, the rule uses "all: unset !important" and "grid-gap",
which over-nukes layout and uses deprecated property; replace "all: unset" with
targeted resets (e.g., reset margin, padding, border, background only) and
remove or avoid using !important where possible, preserve display/positioning
properties, and replace "grid-gap: .25rem" with the modern "gap: .25rem"; ensure
nested selectors (.digit-field, .digit-header-content .label-container
.label-styles) only override spacing properties (margin/padding) so flow and
typography are not unintentionally removed.
border: solid 1px #efefef00 !important; | ||
border-radius: 5px !important; | ||
margin-top: auto; | ||
font-weight: bold; | ||
width: 100%; | ||
} | ||
|
||
.no-hover:hover { | ||
/* Prevent any changes on hover */ | ||
background-color: inherit !important; | ||
/* Revert background color */ | ||
color: inherit !important; | ||
/* Revert text color */ | ||
opacity: 1 !important; | ||
/* Ensure full opacity */ | ||
transform: none !important; | ||
/* Reset any transformations */ | ||
box-shadow: none !important; | ||
/* Remove any shadows */ | ||
text-decoration: none !important; | ||
/* Ensure no underline */ | ||
font-size: inherit !important; | ||
/* Revert font size */ | ||
font-weight: inherit !important; | ||
/* Revert font weight */ | ||
/* Add any other properties that might be changing on hover */ | ||
} |
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.
🛠️ Refactor suggestion
Don’t suppress all hover states; preserve focus-visible for accessibility
- .no-hover removes all hover effects, but ensure keyboard focus is still clearly visible.
- Consider a more targeted approach to avoid nuking useful affordances.
.no-hover:hover {
background-color: inherit !important;
color: inherit !important;
opacity: 1 !important;
transform: none !important;
box-shadow: none !important;
text-decoration: none !important;
font-size: inherit !important;
font-weight: inherit !important;
}
+/* Preserve focus outline/visibility */
+.no-hover:focus-visible {
+ outline: 2px solid var(--digit-focus-color, #0b4b66);
+ outline-offset: 2px;
+}
Also review whether !important is necessary on every property to avoid specificity wars.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
.explore-button-updated { | |
border: solid 1px #efefef00 !important; | |
border-radius: 5px !important; | |
margin-top: auto; | |
font-weight: bold; | |
width: 100%; | |
} | |
.no-hover:hover { | |
/* Prevent any changes on hover */ | |
background-color: inherit !important; | |
/* Revert background color */ | |
color: inherit !important; | |
/* Revert text color */ | |
opacity: 1 !important; | |
/* Ensure full opacity */ | |
transform: none !important; | |
/* Reset any transformations */ | |
box-shadow: none !important; | |
/* Remove any shadows */ | |
text-decoration: none !important; | |
/* Ensure no underline */ | |
font-size: inherit !important; | |
/* Revert font size */ | |
font-weight: inherit !important; | |
/* Revert font weight */ | |
/* Add any other properties that might be changing on hover */ | |
} | |
.explore-button-updated { | |
border: solid 1px #efefef00 !important; | |
border-radius: 5px !important; | |
margin-top: auto; | |
font-weight: bold; | |
width: 100%; | |
} | |
.no-hover:hover { | |
/* Prevent any changes on hover */ | |
background-color: inherit !important; | |
/* Revert background color */ | |
color: inherit !important; | |
/* Revert text color */ | |
opacity: 1 !important; | |
/* Ensure full opacity */ | |
transform: none !important; | |
/* Reset any transformations */ | |
box-shadow: none !important; | |
/* Remove any shadows */ | |
text-decoration: none !important; | |
/* Ensure no underline */ | |
font-size: inherit !important; | |
/* Revert font size */ | |
font-weight: inherit !important; | |
/* Revert font weight */ | |
/* Add any other properties that might be changing on hover */ | |
} | |
/* Preserve focus outline/visibility */ | |
.no-hover:focus-visible { | |
outline: 2px solid var(--digit-focus-color, #0b4b66); | |
outline-offset: 2px; | |
} |
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
around lines 1392 to 1419, the .no-hover rule blanket-suppresses all hover
effects (and uses !important everywhere) which kills keyboard focus visibility
and causes specificity problems; instead, narrow the rule to only override the
specific hover properties you must change, remove gratuitous !important
declarations, and explicitly preserve or add a :focus-visible style so keyboard
users retain a visible focus ring; update selectors to target pointer-hover
(e.g., combine .no-hover with a pointer media query or :hover only) rather than
removing all interactive affordances, and reduce specificity so the rule doesn’t
require !important.
display: flex; | ||
align-items: center !important; | ||
justify-content: flex-start !important; | ||
/* Vertical alignment */ | ||
gap: 1rem; | ||
width: 100%; | ||
padding: 0rem 8rem; | ||
} |
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.
🛠️ Refactor suggestion
Large horizontal padding may overflow on small screens
Padding: 0 8rem on role blocks can cause cramped content/overflow on narrow viewports. Add a responsive reduction.
.role-block {
gap: 1rem;
width: 100%;
- padding: 0rem 8rem;
+ padding: 0 8rem;
}
+@media (max-width: 768px) {
+ .role-block {
+ padding: 0 1rem;
+ }
+}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
.role-block { | |
display: flex; | |
align-items: center !important; | |
justify-content: flex-start !important; | |
/* Vertical alignment */ | |
gap: 1rem; | |
width: 100%; | |
padding: 0rem 8rem; | |
} | |
.role-block { | |
display: flex; | |
align-items: center !important; | |
justify-content: flex-start !important; | |
/* Vertical alignment */ | |
gap: 1rem; | |
width: 100%; | |
padding: 0 8rem; | |
} | |
@media (max-width: 768px) { | |
.role-block { | |
padding: 0 1rem; | |
} | |
} |
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
around lines 1484 to 1492, the .role-block rule uses fixed horizontal padding of
0rem 8rem which can cause overflow or cramped layout on small viewports; change
it to a responsive padding strategy such as using padding-inline with a clamp()
(e.g. padding-inline: clamp(1rem, 5vw, 8rem)) or add a max-width media query
(e.g. @media (max-width: 600px) { .role-block { padding: 0 1rem; } }) so
horizontal padding reduces on narrow screens while preserving the large padding
on wide screens.
font-weight: 700; | ||
color: #215B73; | ||
margin-bottom: 24px; | ||
position: relative; | ||
display: inline-block; | ||
/* allow ::after to size with text */ | ||
text-align: center; | ||
line-height: 2.65rem; | ||
} | ||
|
||
.cs-title::after { | ||
content: ""; | ||
display: block; | ||
width: 8rem; | ||
/* match text width */ | ||
height: 4px; | ||
background-color: #215B73; | ||
margin: 6px auto 0 auto; | ||
/* center under text */ | ||
} |
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.
🧹 Nitpick (assertive)
Title underline color should use variables; comment contradicts behavior
Comment says “match text width” but width is fixed 8rem. Either change comment or implement text-width matching.
.cs-title::after {
- width: 8rem; /* match text width */
- background-color: #215B73;
+ width: 100%; /* match text width */
+ max-width: 8rem;
+ background-color: var(--digit-secondary-color, #215B73);
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
font-size: 24px; | |
font-weight: 700; | |
color: #215B73; | |
margin-bottom: 24px; | |
position: relative; | |
display: inline-block; | |
/* allow ::after to size with text */ | |
text-align: center; | |
line-height: 2.65rem; | |
} | |
.cs-title::after { | |
content: ""; | |
display: block; | |
width: 8rem; | |
/* match text width */ | |
height: 4px; | |
background-color: #215B73; | |
margin: 6px auto 0 auto; | |
/* center under text */ | |
} | |
.cs-title::after { | |
content: ""; | |
display: block; | |
width: 100%; /* match text width */ | |
max-width: 8rem; | |
height: 4px; | |
background-color: var(--digit-secondary-color, #215B73); | |
margin: 6px auto 0 auto; | |
/* center under text */ | |
} |
width: 100%; | ||
height: 500px; | ||
border: none; | ||
min-height: 40rem; | ||
} |
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.
🛠️ Refactor suggestion
Avoid conflicting height and min-height on iframe wrapper
height: 500px conflicts with min-height: 40rem (min-height wins). Keep only the intended constraint.
.wt-iframe {
- height: 500px;
border: none;
min-height: 40rem;
}
If a hard cap is required, use max-height instead of a smaller fixed height.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
.wt-iframe { | |
width: 100%; | |
height: 500px; | |
border: none; | |
min-height: 40rem; | |
} | |
.wt-iframe { | |
width: 100%; | |
border: none; | |
min-height: 40rem; | |
} |
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
around lines 1711 to 1716, the .wt-iframe rule sets both height: 500px and
min-height: 40rem which conflict (min-height wins); remove the fixed height if
you intend a minimum-only constraint, or replace height: 500px with max-height:
500px if you need a hard cap while preserving the min-height; update the rule to
contain only the intended height constraint and keep width and border as-is.
display: flex; | ||
align-items: center; | ||
font-size: 14px; | ||
font-weight: 500; | ||
padding: 12px 16px; | ||
border-radius: 4px; | ||
margin: 20px 0; | ||
} | ||
|
||
.nav-breadcrumb a { | ||
color: #e67e22 !important; | ||
text-decoration: none; | ||
transition: color 0.2s ease; | ||
} | ||
|
||
.nav-breadcrumb a:hover { | ||
color: #d35400; | ||
} | ||
|
||
.nav-breadcrumb .separator { | ||
margin: 0 4px; | ||
color: #666; | ||
font-weight: 500; | ||
} | ||
|
||
.nav-breadcrumb .current { | ||
color: #333; | ||
font-weight: 500; | ||
} |
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.
🛠️ Refactor suggestion
Add keyboard focus/active styles to breadcrumbs
Ensure visible focus and clear active/current state for a11y.
.nav-breadcrumb a {
color: #e67e22 !important;
text-decoration: none;
transition: color 0.2s ease;
}
.nav-breadcrumb a:hover {
color: #d35400;
}
+.nav-breadcrumb a:focus-visible {
+ outline: 2px solid var(--digit-focus-color, #0B4B66);
+ outline-offset: 2px;
+ text-decoration: underline;
+}
+.nav-breadcrumb a[aria-current="page"] {
+ color: #333;
+ font-weight: 600;
+ text-decoration: none;
+}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
.nav-breadcrumb { | |
display: flex; | |
align-items: center; | |
font-size: 14px; | |
font-weight: 500; | |
padding: 12px 16px; | |
border-radius: 4px; | |
margin: 20px 0; | |
} | |
.nav-breadcrumb a { | |
color: #e67e22 !important; | |
text-decoration: none; | |
transition: color 0.2s ease; | |
} | |
.nav-breadcrumb a:hover { | |
color: #d35400; | |
} | |
.nav-breadcrumb .separator { | |
margin: 0 4px; | |
color: #666; | |
font-weight: 500; | |
} | |
.nav-breadcrumb .current { | |
color: #333; | |
font-weight: 500; | |
} | |
.nav-breadcrumb { | |
display: flex; | |
align-items: center; | |
font-size: 14px; | |
font-weight: 500; | |
padding: 12px 16px; | |
border-radius: 4px; | |
margin: 20px 0; | |
} | |
.nav-breadcrumb a { | |
color: #e67e22 !important; | |
text-decoration: none; | |
transition: color 0.2s ease; | |
} | |
.nav-breadcrumb a:hover { | |
color: #d35400; | |
} | |
.nav-breadcrumb a:focus-visible { | |
outline: 2px solid var(--digit-focus-color, #0B4B66); | |
outline-offset: 2px; | |
text-decoration: underline; | |
} | |
.nav-breadcrumb a[aria-current="page"] { | |
color: #333; | |
font-weight: 600; | |
text-decoration: none; | |
} | |
.nav-breadcrumb .separator { | |
margin: 0 4px; | |
color: #666; | |
font-weight: 500; | |
} | |
.nav-breadcrumb .current { | |
color: #333; | |
font-weight: 500; | |
} |
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
around lines 1724 to 1753, the breadcrumb styles lack visible keyboard focus and
clear active/current state for accessibility; add styles for .nav-breadcrumb
a:focus, .nav-breadcrumb a:focus-visible and .nav-breadcrumb a:active to provide
a high-contrast outline or box-shadow and maintain the color transition (use
non-invasive outline-offset or ring-like box-shadow, e.g. 2px solid or 0 0 0 3px
with an accessible color), ensure .nav-breadcrumb .current is visually distinct
(stronger color, weight or underline) and rely on aria-current="page" in markup
so you can style .nav-breadcrumb a[aria-current="page"] as the active state;
keep hover behavior but ensure focus styles are at least as prominent as hover
for keyboard users.
Co-authored-by: syedkaif866 <[email protected]>
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: 6
♻️ Duplicate comments (6)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/ForgotOrganizationTooltip.js (3)
5-5
: Remove unused prop from component signature.
onSelect
isn’t used in this component.-const ForgotOrganizationTooltip = ({ onSelect }) => { +const ForgotOrganizationTooltip = () => {
43-51
: Make Button self-closing and drop commented prop.No children are rendered; prefer self-closing JSX and remove commented code.
- <Button - label={t(`SB_FORGOTORGANIZATION_TOOLTIP`)} - variation={"link"} - size={"small"} - onClick={() => setShowTip((prev) => !prev)} - // isSuffix={true} - style={{ marginBottom: "0.5rem", paddingLeft: "0.2rem" }} - ></Button> + <Button + label={t(`SB_FORGOTORGANIZATION_TOOLTIP`)} + variation={"link"} + size={"small"} + onClick={() => setShowTip((prev) => !prev)} + style={{ marginBottom: "0.5rem", paddingLeft: "0.2rem" }} + />
53-67
: Move extensive inline styles to CSS and center-position the tooltip.Inline styles reduce maintainability. Also, with
left: 50%
you likely wanttranslateX(-50%)
for centering.Option A (preferred): extract to CSS
- <div - style={{ - position: "absolute", - bottom: "100%", - left: "50%", - transform: "translateY(-4px)", - backgroundColor: "#0b4b66", - color: "white", - padding: "6px 10px", - borderRadius: "4px", - whiteSpace: "normal", - wordBreak: "break-word", - maxWidth: "20rem", - zIndex: 1000, - fontSize: "0.875rem" - }} - > + <div className="forgot-organization-tooltip">CSS (new):
/* forgot-organization-tooltip.css */ .forgot-organization-tooltip { position: absolute; bottom: 100%; left: 50%; transform: translate(-50%, -4px); /* center horizontally */ background-color: #0b4b66; color: #fff; padding: 6px 10px; border-radius: 4px; white-space: normal; word-break: break-word; max-width: 20rem; z-index: 1000; font-size: 0.875rem; }Option B (if keeping inline styles): minimally fix centering
- transform: "translateY(-4px)", + transform: "translate(-50%, -4px)",micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js (3)
1-1
: Remove unused Fragment import and tighten imports.
Fragment
is unused (you useReact.Fragment
). Also consider removinguseRef
ifwrapperRef
is not used for DOM operations.-import React, { useState, useEffect, Fragment,useRef } from "react"; +import React, { useState, useEffect, useRef } from "react";If you remove
wrapperRef
, then:-import React, { useState, useEffect, useRef } from "react"; +import React, { useState, useEffect } from "react";
7-7
: Drop unused ref if not leveraging DOM APIs.
wrapperRef
is assigned but not used. Remove it and theref
prop to reduce noise.- const wrapperRef = useRef(null); + // (removed) wrapperRef - <div ref={wrapperRef} className="loginSignUpSelector" style={{ marginTop: topMargin }}> + <div className="loginSignUpSelector" style={{ marginTop: topMargin }}>Also applies to: 45-45
48-55
: Use self-closing syntax for Button elements.No children are provided; prefer self-closing JSX.
- ></Button> + />- ></Button> + />Also applies to: 56-63
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/ForgotOrganizationTooltip.js
(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit Configuration File
check
Files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/ForgotOrganizationTooltip.js
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js
🧠 Learnings (3)
📚 Learning: 2025-06-27T05:52:33.156Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in `micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js` is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js
📚 Learning: 2025-06-26T09:43:00.597Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js:24-24
Timestamp: 2025-06-26T09:43:00.597Z
Learning: In LoginSignupSelector component, the URL checking using window.location.href.includes("sandbox-ui/user/sign-up") is intentional for URL validation purposes, not navigation. The team prefers this approach over React Router's useLocation hook for specific URL checking needs.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js
📚 Learning: 2024-10-17T11:59:00.327Z
Learnt from: Ramkrishna-egov
PR: egovernments/DIGIT-Frontend#1545
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/ActivityCard.js:1-1
Timestamp: 2024-10-17T11:59:00.327Z
Learning: In `ActivityCard.js`, `React.Fragment` is used in the code.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js
🧬 Code Graph Analysis (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/ForgotOrganizationTooltip.js (1)
micro-ui/web/micro-ui-internals/packages/react-components/src/atoms/Dropdown.js (1)
t
(98-98)
⏰ Context from checks skipped due to timeout of 10000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js (1)
15-16
: No SSR guard needed at this time
This component only runs in the browser (and our tests use a JSDOM-like environment), so all direct window calls are safe. If you later introduce true server‐side rendering or another non-browser context, you can wrap each usage in:const ctx = typeof window !== "undefined" ? window : undefined; ctx?.location.replace(…); ctx?.screen… ctx?.addEventListener(…)
useEffect(() => { | ||
const computeTopMargin = () => { | ||
if ( | ||
window.screen.availWidth <= 1366 || | ||
(window.screen.availHeight <= 768 && | ||
window.devicePixelRatio > 1.0) | ||
) { | ||
return "-0.1rem"; | ||
} | ||
return "-2rem"; | ||
}; | ||
|
||
setTopMargin(computeTopMargin()); | ||
|
||
const handleResize = () => setTopMargin(computeTopMargin()); | ||
window.addEventListener("resize", handleResize); | ||
return () => window.removeEventListener("resize", handleResize); | ||
}, []); |
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.
🧹 Nitpick (assertive)
Use viewport dimensions (innerWidth/innerHeight) and consider DRYing this logic.
screen.availWidth/availHeight
are OS-level; UI often needs window.innerWidth/innerHeight
. Also, the same logic exists in LoginSignupSelector; extract to a shared util/hook.
- window.screen.availWidth <= 1366 ||
- (window.screen.availHeight <= 768 &&
+ window.innerWidth <= 1366 ||
+ (window.innerHeight <= 768 &&
window.devicePixelRatio > 1.0)
Suggestion: create a small shared hook:
// useResponsiveTopMargin.js
import { useEffect, useState } from "react";
export const useResponsiveTopMargin = (small = "-0.1rem", large = "-2rem") => {
const [topMargin, setTopMargin] = useState(large);
useEffect(() => {
const compute = () =>
window.innerWidth <= 1366 || (window.innerHeight <= 768 && window.devicePixelRatio > 1)
? small
: large;
setTopMargin(compute());
const onResize = () => setTopMargin(compute());
window.addEventListener("resize", onResize);
return () => window.removeEventListener("resize", onResize);
}, [small, large]);
return topMargin;
};
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/ForgotOrganizationTooltip.js
around lines 11 to 28, the component uses window.screen.availWidth/availHeight
(OS-level) and duplicates responsive logic found in LoginSignupSelector; replace
screen.* with window.innerWidth/innerHeight, extract the computeTopMargin logic
into a shared hook or util (e.g., useResponsiveTopMargin) that accepts
small/large margin values, update this component to call that hook, and update
LoginSignupSelector to reuse the same hook so the logic is DRY and uses viewport
dimensions; ensure the hook sets initial state, adds/removes resize listener,
and depends on the margin args.
}; | ||
}, []); | ||
return ( | ||
<div ref={wrapperRef} className="loginSignUpSelector" style={{ position: "relative", marginTop: topMargin}}> |
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.
🧹 Nitpick (assertive)
Rename container class to reflect the component.
className="loginSignUpSelector"
looks copy-pasted; use a tooltip-specific class for clarity.
- <div ref={wrapperRef} className="loginSignUpSelector" style={{ position: "relative", marginTop: topMargin}}>
+ <div ref={wrapperRef} className="forgotOrganizationTooltip" style={{ position: "relative", marginTop: topMargin}}>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<div ref={wrapperRef} className="loginSignUpSelector" style={{ position: "relative", marginTop: topMargin}}> | |
<div ref={wrapperRef} className="forgotOrganizationTooltip" style={{ position: "relative", marginTop: topMargin}}> |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/ForgotOrganizationTooltip.js
around line 42, the container uses a generic className "loginSignUpSelector"
which appears copy-pasted; change it to a tooltip-specific class like
"forgotOrganizationTooltip" or "forgot-org-tooltip" to reflect the component,
update any related CSS/SCSS selectors to match the new class, and run a
repo-wide search to replace/remove other accidental uses of
"loginSignUpSelector" so styles and tests remain correct.
<Button | ||
label={t(`SB_FORGOTORGANIZATION_TOOLTIP`)} | ||
variation={"link"} | ||
size={"small"} | ||
onClick={() => setShowTip((prev) => !prev)} | ||
// isSuffix={true} | ||
style={{ marginBottom: "0.5rem", paddingLeft: "0.2rem" }} | ||
></Button> |
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.
🛠️ Refactor suggestion
Add basic accessibility attributes for the tooltip.
Improve screen-reader and keyboard support with ARIA and Escape-to-close handling.
- <Button
+ <Button
label={t(`SB_FORGOTORGANIZATION_TOOLTIP`)}
variation={"link"}
size={"small"}
- onClick={() => setShowTip((prev) => !prev)}
+ aria-expanded={showTip}
+ aria-controls="forgot-org-tooltip"
+ onClick={() => setShowTip((prev) => !prev)}
style={{ marginBottom: "0.5rem", paddingLeft: "0.2rem" }}
/>
- <div
+ <div
+ id="forgot-org-tooltip"
+ role="tooltip"
Optional: add key handler
- useEffect(() => {
+ useEffect(() => {
+ const handleKeyDown = (e) => { if (e.key === "Escape") setShowTip(false); };
+ document.addEventListener("keydown", handleKeyDown);
+ return () => document.removeEventListener("keydown", handleKeyDown);
- }, []);
+ }, []);
Also applies to: 53-69
import React, { useState, useEffect, Fragment,useRef } from "react"; | ||
import { useTranslation } from "react-i18next"; | ||
import { CheckBox, PopUp, Button, HeaderComponent } from "@egovernments/digit-ui-components"; | ||
const LoginSignupSelector = ({ onSelect, formData, control, formState, ...props }) => { |
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.
🛠️ Refactor suggestion
Make onSelect optional to avoid runtime errors.
If parent doesn’t pass onSelect
, calling it will throw. Default to a no-op.
-const LoginSignupSelector = ({ onSelect, formData, control, formState, ...props }) => {
+const LoginSignupSelector = ({ onSelect = () => {}, formData, control, formState, ...props }) => {
No other changes required; your useEffect
will keep working:
useEffect(() => {
onSelect("check", isChecked);
}, [isChecked, onSelect]);
Also applies to: 11-13
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js
around lines 4 and also affecting lines 11-13, the prop onSelect is assumed to
exist and calling it can throw if the parent doesn't pass it; make onSelect
optional by providing a no-op default (e.g., default parameter in the function
signature or a fallback when destructuring) so the useEffect and other callers
can safely invoke onSelect without runtime errors.
useEffect(() => { | ||
const computeTopMargin = () => { | ||
if ( | ||
window.screen.availWidth <= 1366 || | ||
(window.screen.availHeight <= 768 && | ||
window.devicePixelRatio > 1.0) | ||
) { | ||
return "-0.1rem"; | ||
} | ||
return "-2rem"; | ||
}; | ||
|
||
setTopMargin(computeTopMargin()); | ||
|
||
const handleResize = () => setTopMargin(computeTopMargin()); | ||
window.addEventListener("resize", handleResize); | ||
return () => window.removeEventListener("resize", handleResize); | ||
}, []); |
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.
🧹 Nitpick (assertive)
Deduplicate computeTopMargin logic (shared with ForgotOrganizationTooltip).
Extract to a small shared hook/util to keep behavior consistent and testable.
See suggested useResponsiveTopMargin
hook in the other file’s comment; you can use it here to replace local state/effect.
Also applies to: 11-13
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js
around lines 18 to 35, the computeTopMargin logic is duplicated (also used in
ForgotOrganizationTooltip); extract that logic into a shared hook or util (e.g.,
useResponsiveTopMargin) in a common utilities/hooks folder, export it, then
replace the local useEffect/useState in this file with a call to the shared hook
(import it and use its returned topMargin), remove the duplicated
computeTopMargin function and event listener code here, and update tests/usage
to rely on the new hook to keep behavior consistent and testable.
window.location.replace(`/${window?.contextPath}/user/sign-up`); | ||
}; | ||
|
||
const isSignupPage = window.location.href.includes("sandbox-ui/user/sign-up"); |
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.
🧹 Nitpick (assertive)
Avoid hardcoding context path in URL check.
Use the same contextPath
you employ for navigation to make this resilient across environments.
- const isSignupPage = window.location.href.includes("sandbox-ui/user/sign-up");
+ const isSignupPage = window.location.href.includes(`/${window?.contextPath}/user/sign-up`);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const isSignupPage = window.location.href.includes("sandbox-ui/user/sign-up"); | |
const isSignupPage = window.location.href.includes(`/${window?.contextPath}/user/sign-up`); |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js
around line 42, avoid hardcoding the context path in the URL check; replace the
literal "sandbox-ui/user/sign-up" with the same contextPath used for navigation
(e.g. check window.location.href.includes(`${contextPath}/user/sign-up`)),
importing or referencing the existing contextPath value the module uses for
navigation and providing a safe fallback (empty string) if it’s undefined.
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: 1
🔭 Outside diff range comments (3)
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (3)
10-15
: Conflicting margin declarations: margin-bottom is overridden by later margin: unset
margin: unset !important;
wipes out the earliermargin-bottom: 1.5rem;
. Reorder or remove the generic margin reset.Apply this diff to preserve bottom spacing while still clearing others:
.sandbox-module-card { min-height: 263px !important; width: 263px !important; - margin-bottom: 1.5rem; background-color: #fff; - padding: unset !important; - margin: unset !important; + padding: 0 !important; + margin: 0 !important; + margin-bottom: 1.5rem; position: relative; }
18-25
: Redundant/contradictory width declarations on absolutely positioned buttonYou set
width: 100%
and later override withwidth: auto;
. Also with bothleft
andright
set, the width will stretch automatically. Keep one approach..sandbox-module-button { position: absolute; - width: 100%; bottom: 1.5rem; left: 1rem; right: 1rem; margin: auto; - width: auto; }
1129-1133
: Invalid CSS: color value is quoted string
color: "#c84c0e";
is invalid; the quotes cause the declaration to be ignored..highlight-sandbox { - color: "#c84c0e"; + color: #c84c0e; /* or your specific orange color code */ }
♻️ Duplicate comments (12)
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (12)
116-119
: Add modern viewport fallbacks (svh) and sensible min constraintsSame concern previously noted: vh/vw alone can be unreliable on mobile dynamic viewports.
height: 80vh !important; + height: 80svh !important; width: 30vw !important; + min-width: 320px !important; max-width: 500px !important; max-height: 650px !important;
149-151
: Fixed 650×500 with !important is too rigid; use responsive constraintsThis was raised earlier. Prefer min/max/clamp to avoid overflow on small screens and remove reliance on !important.
- height: 650px !important; - width: 500px !important; + height: min(650px, 100svh); + width: min(500px, 100%);
231-239
: Duplicate font-family, excessive top margin, and hardcoded colorYou set font-family twice, use an 8rem top margin, and hardcode color.
.digit-card-label.sandbox-custom-otp-subheader { - font-family: "Roboto"; - margin-bottom: 3rem; - font-family: Roboto; - margin-top: 8rem !important; - color: #0b4b66 !important; + font-family: "Roboto"; + margin-bottom: 3rem; + margin-top: 2rem !important; /* adjust via media queries as needed */ + color: var(--digit-primary-color, #0B4B66) !important; text-align: center !important; font-size: 2rem !important; font-weight: 700 !important; }
268-314
: Tooltip is hover-only and ignores reduced-motion; add keyboard focus and motion prefsEnsure accessibility with focus-visible or focus-within, and respect prefers-reduced-motion.
- &:hover .infotext { + &:hover .infotext, + &:focus-within .infotext { visibility: visible; opacity: 1; } .infotext { visibility: hidden; opacity: 0; transition: opacity 0.3s; + pointer-events: none; + @media (prefers-reduced-motion: reduce) { + transition: none; + }
320-329
: Avoid all: unset; use targeted resets and replace deprecated grid-gap
all: unset
removes layout-critical properties. Also usegap
instead ofgrid-gap
.-.sandbox-signup-form .digit-field .label-field-wrapper { - all: unset !important; - grid-gap: .25rem !important; +.sandbox-signup-form .digit-field .label-field-wrapper { + display: grid; /* keep intended layout */ + gap: .25rem !important; + margin: 0; + padding: 0; + border: 0; + background: transparent;
341-344
: Use CSS variables/tokens instead of hardcoded colorsReplace
#0B4B66
with a theme variable for maintainability and theming.- color: #0B4B66; + color: var(--digit-primary-color, #0B4B66); - font-size: 2rem; + font-size: 2rem; /* keep */
616-620
: Typo in comment: duplicate word “components”Fix the comment for clarity.
- /* Ensure spacing between the two components components */ + /* Ensure spacing between the two components */
1392-1419
: Don’t blanket-suppress hover and remove all affordances; preserve focus-visibleThe .no-hover rule nukes interaction feedback and relies on many !important flags. Keep keyboard focus visible.
.no-hover:hover { background-color: inherit !important; color: inherit !important; opacity: 1 !important; transform: none !important; box-shadow: none !important; text-decoration: none !important; font-size: inherit !important; font-weight: inherit !important; } +/* Preserve keyboard focus visibility */ +.no-hover:focus-visible { + outline: 2px solid var(--digit-focus-color, #0B4B66); + outline-offset: 2px; +}
1484-1492
: Large horizontal padding may overflow small screensMake padding responsive to avoid cramped layouts on narrow devices.
.role-block { gap: 1rem; width: 100%; - padding: 0rem 8rem; + padding: 0 8rem; +} +@media (max-width: 768px) { + .role-block { + padding: 0 1rem; + } }
1553-1573
: Underline comment mismatches behavior; also use theme vars for colorWidth is fixed at 8rem but comment claims “match text width”. Either match text or correct the comment; also swap hardcoded color.
.cs-title::after { content: ""; display: block; - width: 8rem; /* match text width */ + width: 100%; /* match text width */ + max-width: 8rem; height: 4px; - background-color: #215B73; + background-color: var(--digit-secondary-color, #215B73); margin: 6px auto 0 auto; /* center under text */ }
1711-1716
: Conflicting height and min-height on iframe
min-height: 40rem
overridesheight: 500px
. Keep the intended constraint..wt-iframe { width: 100%; - height: 500px; border: none; min-height: 40rem; }
If you need an upper cap, add
max-height
instead.
1724-1753
: Breadcrumbs: add focus-visible and current-page state for a11yAdd a visible focus style and style the current page (
aria-current="page"
)..nav-breadcrumb a { color: #e67e22 !important; text-decoration: none; transition: color 0.2s ease; } .nav-breadcrumb a:hover { color: #d35400; } +.nav-breadcrumb a:focus-visible { + outline: 2px solid var(--digit-focus-color, #0B4B66); + outline-offset: 2px; + text-decoration: underline; +} +.nav-breadcrumb a[aria-current="page"] { + color: #333; + font-weight: 600; + text-decoration: none; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
(10 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-26T10:24:08.628Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss:1642-1662
Timestamp: 2025-06-26T10:24:08.628Z
Learning: In the sandbox SCSS file, tab styling with margin-bottom and border changes that cause layout shifts are intentional design requirements per updated UI specifications, not bugs to be fixed.
Applied to files:
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
📚 Learning: 2024-10-26T15:11:49.938Z
Learnt from: nabeelmd-eGov
PR: egovernments/DIGIT-Frontend#1680
File: health/micro-ui/web/micro-ui-internals/example/src/UICustomizations.js:1032-1094
Timestamp: 2024-10-26T15:11:49.938Z
Learning: In the DIGIT-Frontend project, when reviewing the `UICustomizations.js` file, avoid suggesting refactoring of repetitive rendering logic in switch cases into helper functions, unless explicitly requested.
Applied to files:
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
📚 Learning: 2025-06-26T10:16:36.004Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/css/src/components/Carousel.scss:0-0
Timestamp: 2025-06-26T10:16:36.004Z
Learning: In the DIGIT-Frontend codebase, signup-container CSS classes cannot be namespaced under parent classes due to existing style constraints and requirements.
Applied to files:
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
⏰ Context from checks skipped due to timeout of 10000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (4)
349-354
: LGTM: login/signup selector layout utilityConcise and clear flex alignment utility; no issues.
406-408
: LGTM: submit bar uses theme tokenGood use of theme() for color. No changes needed.
1369-1390
: LGTM: new icon/image helpersClear and neutral layout utilities for headers and images. No functional issues spotted.
121-123
: Drop obsolete IE flex fallback
After checking the repo, there’s no browserslist config targeting IE10/11, so the-ms-flex-pack
rule can be safely removed:- -ms-flex-pack: justify; justify-content: space-evenly;
.SandboxLogoAlignCenter { | ||
align-items: center; | ||
justify-content: center; | ||
display: flex; | ||
} |
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.
💡 Verification agent
🧩 Analysis chain
Class naming is inconsistent with project conventions
SandboxLogoAlignCenter
is PascalCase, while most classes here use kebab/camel. Unless there’s a precedent, prefer a consistent convention (e.g., .sandbox-logo-align-center
).
If renaming is acceptable across usages, I can generate a repo-wide rename patch. Confirm the preferred convention for this module.
🏁 Script executed:
#!/bin/bash
echo "PascalCase class definitions in css/src:"
rg -n "^\s*\.[A-Z][A-Za-z0-9]*" -tscss micro-ui/web/micro-ui-internals/packages/css/src
echo
echo "kebab-case class definitions in css/src:"
rg -n "^\s*\.[a-z0-9][a-z0-9-]*" -tscss micro-ui/web/micro-ui-internals/packages/css/src
Length of output: 428
🏁 Script executed:
#!/bin/bash
# Search for PascalCase class selectors in SCSS files under css/src
rg -n "^\s*\.[A-Z][A-Za-z0-9]*" micro-ui/web/micro-ui-internals/packages/css/src --glob "*.scss"
echo
# Search for kebab-case class selectors in SCSS files under css/src
rg -n "^\s*\.[a-z0-9][a-z0-9-]*" micro-ui/web/micro-ui-internals/packages/css/src --glob "*.scss"
Length of output: 226990
Align class naming to kebab-case in sandbox.scss
The sandbox.scss
file predominantly uses kebab-case selectors (e.g. .sandbox-module-container
, .sandbox-success-signup
, .sandbox-url-footer
), but at line 89 we introduce a PascalCase selector:
File: micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
Lines: 89-93
-.SandboxLogoAlignCenter {
+.sandbox-logo-align-center {
align-items: center;
justify-content: center;
display: flex;
}
Please rename .SandboxLogoAlignCenter
(and its usages in markup/JSX) to .sandbox-logo-align-center
to match the existing pattern.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
.SandboxLogoAlignCenter { | |
align-items: center; | |
justify-content: center; | |
display: flex; | |
} | |
.sandbox-logo-align-center { | |
align-items: center; | |
justify-content: center; | |
display: flex; | |
} |
🤖 Prompt for AI Agents
In micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss
around lines 89 to 93, the selector `.SandboxLogoAlignCenter` uses PascalCase
while the file uses kebab-case; rename the class to `.sandbox-logo-align-center`
and update every usage across markup/JSX/components to match the new kebab-case
name (search/replace occurrences, update tests or snapshots, and rebuild to
confirm no missing references).
* Update package.json * Update index.html
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
♻️ Duplicate comments (1)
micro-ui/web/public/index.html (1)
19-39
: Fix invalid HTML (style tag outside head) + correct brittle centering + add font fallbacksThis repeats a previously raised issue:
- The style block lives outside , which is invalid HTML and can yield inconsistent behavior.
- The tooltip is centered using left: 50% with margin-left: -10.3rem against a width of 7.5rem, which is incorrect and extremely brittle. Use transform: translateX(-50%) instead.
- Add font fallbacks for Roboto.
Apply this diff to remove the misplaced style block:
-<style> - .digit-header-content.label .info-icon .infotext { - position: absolute; - text-align: center; - font-family: Roboto; - font-style: normal; - font-weight: 400; - line-height: 1.37rem; - visibility: hidden; - width: 7.5rem; - background-color: #555; - color: #fff; - padding: .5rem 0; - z-index: 1; - bottom: 100%; - left: 50%; - margin-left: -10.3rem; - border-radius: .375rem; -} - -</style>Insert the corrected style inside the head (just before ):
<style> .digit-header-content.label .info-icon .infotext { position: absolute; text-align: center; font-family: "Roboto", Arial, sans-serif; font-style: normal; font-weight: 400; line-height: 1.37rem; visibility: hidden; width: 7.5rem; background-color: #555; color: #fff; padding: .5rem 0; z-index: 1; bottom: 100%; left: 50%; transform: translateX(-50%); border-radius: .375rem; } </style>Optional: If this tooltip is used across multiple entry points, move it into a shared stylesheet to avoid duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
micro-ui/web/micro-ui-internals/packages/css/package.json
is excluded by!**/*.json
📒 Files selected for processing (1)
micro-ui/web/public/index.html
(3 hunks)
⏰ Context from checks skipped due to timeout of 10000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
micro-ui/web/public/index.html (1)
56-56
: LGTM: newline at EOFNo issues; keeping a trailing newline is standard.
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].38/dist/index.css" /> | ||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> |
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.
💡 Verification agent
🧩 Analysis chain
Upgrade to [email protected]: verify downstream styling and consider SRI
- Good to see exact version pinning. Please verify no breaking visual regressions across Login/Signup V2 and header/tooltip components due to CSS changes between 1.8.26 → 1.8.38.
- Consider adding Subresource Integrity (integrity, crossorigin) for external CSS to mitigate supply-chain risks, and decide if a self-hosted fallback is needed for environments with restricted CDN access.
Would you like me to enumerate notable changes between these versions and generate a quick visual regression checklist?
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="micro-ui/web/public/index.html"
echo "=== File check: $file ==="
if [ -f "$file" ]; then
sed -n '1,200p' "$file"
else
echo "File not found: $file"
fi
echo
echo "=== Search for digit-ui CSS usage ==="
rg -n "@egovernments/digit-ui-css|digit-ui-components-css" || true
echo
echo "=== Search for Login/Signup/Header/Tooltip occurrences (case-insensitive) in micro-ui ==="
rg -n "(login|signup|header|tooltip)" micro-ui || true
echo
echo "=== Compute SRI (sha384) for the referenced CSS URLs ==="
urls=(
"https://unpkg.com/@egovernments/[email protected]/dist/index.css"
"https://unpkg.com/@egovernments/[email protected]/dist/index.css"
)
for url in "${urls[@]}"; do
echo
echo "URL: $url"
if command -v curl >/dev/null 2>&1; then
status=$(curl -s -o /dev/null -w "%{http_code}" -I -L "$url" || echo "000")
echo "HTTP status: $status"
if [ "$status" != "200" ]; then
echo "Failed to fetch (non-200). Skipping hash."
continue
fi
if command -v openssl >/dev/null 2>&1; then
hash=$(curl -sSL "$url" | openssl dgst -sha384 -binary | openssl base64 -A || true)
else
hash=$(curl -sSL "$url" | python -c 'import sys,hashlib,base64; data=sys.stdin.buffer.read(); print(base64.b64encode(hashlib.sha384(data).digest()).decode())' || true)
fi
elif command -v wget >/dev/null 2>&1; then
status=$(wget --server-response --spider -q "$url" 2>&1 | sed -n '1p' | awk '{print $2}' || echo "000")
echo "HTTP status (wget): $status"
if [ "$status" != "200" ]; then
echo "Failed to fetch (non-200). Skipping hash."
continue
fi
if command -v openssl >/dev/null 2>&1; then
hash=$(wget -q -O - "$url" | openssl dgst -sha384 -binary | openssl base64 -A || true)
else
hash=$(wget -q -O - "$url" | python -c 'import sys,hashlib,base64; data=sys.stdin.buffer.read(); print(base64.b64encode(hashlib.sha384(data).digest()).decode())' || true)
fi
else
echo "No curl/wget available; cannot fetch"
continue
fi
if [ -z "${hash:-}" ]; then
echo "Failed to compute hash"
else
echo "Suggested integrity attribute: sha384-$hash"
echo "Suggested crossorigin attribute: crossorigin=\"anonymous\""
fi
done
Length of output: 158972
Upgrade to [email protected] — SRI computed; please add SRI and verify UI
Short: I verified micro-ui/web/public/index.html and fetched the two CSS files (HTTP 200). I computed SHA-384 SRI hashes — add integrity + crossorigin and run visual regression checks (Login/SignUp V2, header/tooltip). Also reconcile other files that reference older digit-ui-css versions.
Files to action
- micro-ui/web/public/index.html — add integrity & crossorigin to the two external links (suggested diff below).
- Visual-check (priority):
- micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/login.scss
- micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/tooltip.scss
- micro-ui/web/micro-ui-internals/packages/css/src/index.scss (imports tooltip)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/** and Login-v2/**
- Check/align other references (may be docs/examples or intentional):
- micro-ui/web/micro-ui-internals/packages/modules/sandbox/README.md (references @egovernments/[email protected])
- micro-ui/web/micro-ui-internals/packages/modules/workbench/README.md (1.2.114)
- micro-ui/web/micro-ui-internals/packages/css/README.md (mentions 1.8.14)
- micro-ui/web/micro-ui-internals/example/public/index.html (example commented versions)
Suggested replacement (update these two tags in micro-ui/web/public/index.html)
Notes
- Both URLs returned 200 and the above integrity hashes were computed from the current CDN payloads — if you re-publish assets the hash will change.
- Decide whether to add a self-hosted fallback (e.g., local copy served when CDN fails) for restricted environments.
Would you like me to enumerate notable changes between 1.8.26 → 1.8.38 and generate a short visual-regression checklist for Login/Signup V2 and header/tooltip?
🤖 Prompt for AI Agents
In micro-ui/web/public/index.html around lines 10-11, add SRI integrity and
crossorigin attributes to the two external CSS link tags using the provided
SHA-384 hashes (add
integrity="sha384-ujgPEZxME0vI6gml3R9fLnJBJJtuX7crZuyXk7dSmmGevF1r5ZO0O7ty3PBJPFkC"
crossorigin="anonymous" for [email protected] and
integrity="sha384-2EfdlCC1EigMYJ59qOhrK2CfvNH6nzQpPONIej1kQQqdSp4/kpLXsw9Zm3/y7XnC"
crossorigin="anonymous" for [email protected]), then run visual
regression checks focused on Login/SignUp V2 and header/tooltip by verifying
styles in
micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/login.scss,
tooltip.scss, index.scss and the SignUp-v2/Login-v2 pages under modules/core;
finally search and update or document other references to older digit-ui-css
versions in the listed README and example files (sandbox README, workbench
README, css README, example public/index.html) and decide whether to add a local
fallback for restricted environments.
micro-ui/web/public/index.html
Outdated
<style> | ||
.digit-header-content.label .info-icon .infotext { | ||
position: absolute; | ||
text-align: center; | ||
font-family: Roboto; | ||
font-style: normal; | ||
font-weight: 400; | ||
line-height: 1.37rem; | ||
visibility: hidden; | ||
width: 7.5rem; | ||
background-color: #555; | ||
color: #fff; | ||
padding: .5rem 0; | ||
z-index: 1; | ||
bottom: 100%; | ||
left: 50%; | ||
margin-left: -10.3rem; | ||
border-radius: .375rem; | ||
} | ||
|
||
</style> |
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.
🧹 Nitpick (assertive)
Accessibility nit: ensure the tooltip is operable and announced
If this tooltip is triggered via hover/focus, ensure the HTML for the trigger and tooltip include appropriate roles/ARIA (e.g., role="tooltip", aria-describedby on trigger) and that it’s reachable via keyboard. Consider motion-safe positioning and a larger z-index context if inside complex headers.
Happy to propose an a11y-compliant pattern aligned with your component library.
🤖 Prompt for AI Agents
In micro-ui/web/public/index.html around lines 19 to 39, the tooltip CSS alone
does not make the tooltip accessible or keyboard-operable; update the markup and
behavior so the trigger element is focusable and references the tooltip with
aria-describedby, give the tooltip role="tooltip", and ensure visibility is
controlled for both :hover and :focus (and via JS for keyboard/assistive tech)
so it becomes visible and announced when focused; also ensure aria-hidden is
toggled appropriately when showing/hiding, use prefers-reduced-motion-safe
positioning adjustments, and increase z-index so the tooltip is above complex
header stacking contexts.
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/ApiCacheService.js (3)
74-76
: getSettingByServiceUrl fails for “access/v1/actions” entryThe default config contains a key "access/v1/actions", but getSettingByServiceUrl only uses the first path segment (e.g., "access"), which won’t match and will likely cause a TypeError downstream when serviceSetting is undefined.
Harden the URL parsing and try a multi-segment match before falling back:
- getSettingByServiceUrl: (serviceUrl, moduleName) => { - return getSetting(serviceUrl.split("/")[1], moduleName); - }, + getSettingByServiceUrl: (serviceUrl, moduleName) => { + const parts = (serviceUrl || "").split("/").filter(Boolean); // e.g., ['access','v1','actions'] + // Try 3-part (e.g., 'access/v1/actions'), then 2-part, then 1-part + const candidates = [ + parts.slice(0, 3).join("/"), + parts.slice(0, 2).join("/"), + parts[0], + ].filter(Boolean); + for (const name of candidates) { + const s = getSetting(name, moduleName); + if (typeof s?.cacheTimeInSecs !== "undefined") return s; + } + // Fallback to first segment if nothing matched + return getSetting(parts[0], moduleName); + },
51-69
: Null-deref risk: serviceSetting can be undefinedIf a serviceName isn’t found, serviceSetting is undefined and the next line will throw when accessing serviceSetting.cacheTimeInSecs.
Defensive fix:
- const serviceSetting = setting.find((item) => item.serviceName === serviceName); - const responseSetting = { - cacheTimeInSecs: serviceSetting.cacheTimeInSecs, - debounceTimeInMS: serviceSetting.debounceTimeInMS || 100, - }; + const serviceSetting = setting.find((item) => item.serviceName === serviceName); + const responseSetting = { + cacheTimeInSecs: serviceSetting?.cacheTimeInSecs ?? 0, + debounceTimeInMS: serviceSetting?.debounceTimeInMS ?? 100, + };
70-77
: Expose clearAllCache to align with component callsLogin-v2 and SignUp-v2 call window.Digit.ApiCacheService.clearAllCache(), but this API isn’t exported here. Add a no-op-safe method so those calls don’t throw, and optionally let it reset persisted settings.
export const ApiCacheService = { saveSetting: (setting) => { PersistantStorage.set(storageKey, setting || defaultApiCachingSettings); }, + clearAllCache: () => { + // No centralized cache here; reset to defaults so future reads start from defaults. + PersistantStorage.set(storageKey, defaultApiCachingSettings); + }, getSettingByServiceUrl: (serviceUrl, moduleName) => { return getSetting(serviceUrl.split("/")[1], moduleName); }, };
♻️ Duplicate comments (13)
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js (6)
81-83
: Fix tenantId fallback: current code always uses "DEFAULT"
This was already flagged in earlier reviews; reverse the OR.- "tenantId":"DEFAULT"|| stateCode, + "tenantId": stateCode || "DEFAULT",
3-7
: Remove duplicate router imports; import hooks in a single statement
This matches the pattern already flagged in SignUp-v2.-import { Route, Switch, useRouteMatch } from "react-router-dom"; +import { Route, Switch, useRouteMatch, useHistory, useLocation } from "react-router-dom"; import { SignUpConfig as defaultSignUpConfig } from "./config-v2"; import Login from "./signUp"; -import { useHistory, useLocation } from "react-router-dom";
96-114
: Tighten select handler and logging
Nit: spacing/variable names and clearer log context improve readability. Also, validate response shape.- select: (response) => { + select: (response) => { try { - const fetchedBanners =response?.MdmsRes?.sandbox?.BannerImages?.[0]?.bannerImages; + const fetchedBanners = response?.MdmsRes?.sandbox?.BannerImages?.[0]?.bannerImages; if (Array.isArray(fetchedBanners) && fetchedBanners.length > 0) { return fetchedBanners; } return null; } catch (error) { - console.error("Error processing MDMS data from useCustomAPIHook:", error); + console.error("Error processing banner images from MDMS:", error); return null; } },
138-146
: Use strict equality and simplify the loading check
Align with prior feedback; also keep spacing consistent.- useEffect(() => { - if(isLoading == false && mdmsData?.config) - { - setSignUpConfig(mdmsData?.config) - }else{ - setSignUpConfig(defaultSignUpConfig) - } - },[mdmsData, isLoading]) + useEffect(() => { + if (!isLoading && mdmsData?.config) { + setSignUpConfig(mdmsData.config); + } else { + setSignUpConfig(defaultSignUpConfig); + } + }, [mdmsData, isLoading]);Also remove the commented line at Line 138 to keep the code clean.
150-165
: finalConfig ignores SignUpConfig state; use state to reflect MDMS overrides
As noted before, using defaultSignUpConfig makes the MDMS-based update ineffective.- const finalConfig = useMemo(() => { - if (loadingimages) { - return defaultSignUpConfig; - } - if (mdmsBannerImages) { - return [ - { - ...defaultSignUpConfig[0], - bannerImages: mdmsBannerImages, - }, - ]; - } - return defaultSignUpConfig; - }, [loadingimages, mdmsBannerImages]); + const finalConfig = useMemo(() => { + const base = SignUpConfig || defaultSignUpConfig; + if (loadingimages) return base; + if (mdmsBannerImages) { + return [{ ...base[0], bannerImages: mdmsBannerImages }]; + } + return base; + }, [loadingimages, mdmsBannerImages, SignUpConfig]);
182-184
: Include MDMS config loading in Loader condition
Show loader until both banners and config are ready.- if (loadingimages) { + if (loadingimages || isLoading) { return <Loader />; }micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js (7)
79-81
: Fix tenantId fallback: current code always uses "DEFAULT"
Reverse the OR to prefer stateCode.- "tenantId": "DEFAULT" || stateCode, + "tenantId": stateCode || "DEFAULT",
3-7
: Remove duplicate router imports; import hooks in a single statement
Already flagged earlier; same fix applies here.-import { Route, Switch, useRouteMatch } from "react-router-dom"; +import { Route, Switch, useRouteMatch, useHistory, useLocation } from "react-router-dom"; import { SignUpConfig as defaultSignUpConfig } from "./config"; import Login from "./signUp"; -import { useHistory, useLocation } from "react-router-dom";
13-21
: Remove unused store fetch (moduleCode/useStore) or add TODO
The store data is computed but unused; remove to avoid unnecessary work.- const moduleCode = ["privacy-policy"]; - const language = Digit.StoreData.getCurrentLanguage(); - const modulePrefix = "digit"; - const { data: store } = Digit.Services.useStore({ - stateCode, - moduleCode, - language, - modulePrefix - });
136-136
: Remove commented-out legacy state logic
Keep the file free of commented code for clarity.- // let SignUpConfig = mdmsData?.config ? mdmsData?.config : defaultSignUpConfig;
138-144
: Use strict equality and simplify condition
Replace loose equality.- useEffect(() => { - if(isLoading == false && mdmsData?.config) - { - setSignUpConfig(mdmsData?.config) - }else{ - setSignUpConfig(defaultSignUpConfig) - } - },[mdmsData, isLoading]) + useEffect(() => { + if (!isLoading && mdmsData?.config) { + setSignUpConfig(mdmsData.config); + } else { + setSignUpConfig(defaultSignUpConfig); + } + }, [mdmsData, isLoading]);
146-161
: finalConfig should use SignUpConfig state (not default)
Mirror prior advice; otherwise MDMS-driven config is ignored.- const finalConfig = useMemo(() => { - - if (loadingimages) { - return defaultSignUpConfig; - } - if (mdmsBannerImages) { - return [ - { - ...defaultSignUpConfig[0], - bannerImages: mdmsBannerImages, - }, - ]; - } - return defaultSignUpConfig; - - }, [loadingimages, mdmsBannerImages]); + const finalConfig = useMemo(() => { + const base = SignUpConfig || defaultSignUpConfig; + if (loadingimages) return base; + if (mdmsBannerImages) { + return [{ ...base[0], bannerImages: mdmsBannerImages }]; + } + return base; + }, [loadingimages, mdmsBannerImages, SignUpConfig]);
178-180
: Include MDMS config loading in Loader condition
Show loader while either resource is loading.- if (loadingimages) { + if (loadingimages || isLoading) { return <Loader />; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/ApiCacheService.js
(3 hunks)micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js
(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit Configuration File
check
Files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js
micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/ApiCacheService.js
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js
🧠 Learnings (8)
📚 Learning: 2025-06-27T05:52:33.156Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js:0-0
Timestamp: 2025-06-27T05:52:33.156Z
Learning: The PrivacyComponent in `micro-ui/web/micro-ui-internals/packages/modules/core/src/components/PrivacyComponent.js` is mostly used in login and signup pages, especially in formcomposer where the UI is stable, making layout changes less likely to cause regressions.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js
📚 Learning: 2025-06-26T09:43:00.597Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/components/LoginSignupSelector.js:24-24
Timestamp: 2025-06-26T09:43:00.597Z
Learning: In LoginSignupSelector component, the URL checking using window.location.href.includes("sandbox-ui/user/sign-up") is intentional for URL validation purposes, not navigation. The team prefers this approach over React Router's useLocation hook for specific URL checking needs.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js
📚 Learning: 2025-06-04T04:08:48.234Z
Learnt from: jagankumar-egov
PR: egovernments/DIGIT-Frontend#2488
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js:152-152
Timestamp: 2025-06-04T04:08:48.234Z
Learning: In the employee login component (micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login/login.js), prefer using a `defaultTenant` variable to store `Digit.ULBService.getStateId()` instead of calling the function multiple times inline in the `defaultValue` object. This improves code readability and avoids function call duplication.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js
📚 Learning: 2024-10-08T20:11:07.772Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#876
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/Module.js:13-40
Timestamp: 2024-10-08T20:11:07.772Z
Learning: Error handling for the `Digit.Services.useStore` method is managed internally and does not require explicit external error checks.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js
📚 Learning: 2024-11-07T11:02:33.520Z
Learnt from: rachana-egov
PR: egovernments/DIGIT-Frontend#1770
File: health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js:320-322
Timestamp: 2024-11-07T11:02:33.520Z
Learning: In `health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/viewVillage.js`, the `data?.additionalFields` object is guaranteed to be defined, so checking for its existence before accessing its keys is unnecessary.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js
📚 Learning: 2025-01-02T04:41:15.957Z
Learnt from: jagankumar-egov
PR: egovernments/DIGIT-Frontend#2089
File: micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js:111-111
Timestamp: 2025-01-02T04:41:15.957Z
Learning: In micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js, always use "mdms[0].uniqueIdentifier" for the jsonPath instead of differentiating between "SOR" and other masters.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js
📚 Learning: 2024-06-14T14:10:20.359Z
Learnt from: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#876
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js:3-39
Timestamp: 2024-06-14T14:10:20.359Z
Learning: The `CustomScaleControl` component in the `micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/CustomScaleControl.js` file should not include a "loading map" message as there is another component handling this functionality to avoid redundancy.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js
📚 Learning: 2025-06-26T10:17:02.717Z
Learnt from: Hari-egov
PR: egovernments/DIGIT-Frontend#2644
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Otp/index.js:0-0
Timestamp: 2025-06-26T10:17:02.717Z
Learning: In the DIGIT Frontend project, banner images and similar configuration data should be managed through MDMS (Master Data Management Service) rather than hardcoded arrays or separate config files, as indicated by the user Hari-egov.
Applied to files:
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js
🧬 Code Graph Analysis (2)
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js (2)
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/config.js (2)
SignUpConfig
(1-110)SignUpConfig
(1-110)micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/signUp.js (2)
Digit
(66-66)Login
(9-162)
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js (2)
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/config-v2.js (2)
SignUpConfig
(1-121)SignUpConfig
(1-121)micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/signUp.js (3)
Digit
(82-82)history
(17-17)Login
(10-168)
⏰ Context from checks skipped due to timeout of 10000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/ApiCacheService.js (1)
6-7
: Localization cache to 1 hour looks reasonableKeeping localization cached for an hour is a good balance for UX and network savings.
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js (1)
27-59
: Storage purge: manual audit required for preserved keys
Our automated scan didn’t detect any other calls to Digit.LocalStorage/SessionStorage or native localStorage/sessionStorage getters/setters, but absence of matches isn’t proof of absence. Please perform a manual audit to ensure no other critical keys—such as language preferences, telemetry consent flags, feature-toggle states or sandbox-specific tokens—are being stored and would be lost by this clear-and-restore logic. Update thepreserveKeys
array as needed to include any additional keys you identify.micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js (1)
26-74
: Storage purge: verify preserve list suffices for signup flow
Confirm no other critical keys (e.g., language, feature flags) are being wiped for SignUp.Reuse the scan script from the Login-v2 comment to enumerate keys used across the codebase and adjust preserveKeys accordingly.
cacheTimeInSecs: 0, | ||
// cacheTimeInSecs: 3600, | ||
debounceTimeInMS: 100, | ||
moduleSettings: [ |
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.
🧹 Nitpick (assertive)
🛠️ Refactor suggestion
Disabling MDMS cache entirely can thrash MDMS and slow down UX
cacheTimeInSecs: 0 forces a network call on every read. For banner/config-driven pages, that can degrade performance and hammer MDMS. Prefer a short-but-nonzero TTL in production, with 0 only for local/dev.
Example improvement (env-gated):
- cacheTimeInSecs: 0,
- // cacheTimeInSecs: 3600,
+ cacheTimeInSecs: process.env.NODE_ENV === "production" ? 300 : 0,
+ // If needed, override via ApiCacheService.saveSetting() from app init.
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/ApiCacheService.js
around lines 18-21, the config currently sets cacheTimeInSecs: 0 which forces a
network call on every read; change this to use a short-but-nonzero TTL in
production and 0 only for local/dev by reading an env-guarded value (e.g.,
process.env.MDMS_CACHE_TTL or NODE_ENV check) and falling back to a sensible
default (eg 60–3600 seconds) so production uses caching while development can
disable it.
serviceName: "mdms-v2", | ||
cacheTimeInSecs: 3600, | ||
cacheTimeInSecs: 0, | ||
// cacheTimeInSecs: 3600, | ||
debounceTimeInMS: 100, | ||
moduleSettings: [ |
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.
🧹 Nitpick (assertive)
🛠️ Refactor suggestion
Same concern for mdms-v2: avoid cache=0 in production
Apply the same nonzero TTL guidance to mdms-v2 for consistency and to reduce load.
- cacheTimeInSecs: 0,
- // cacheTimeInSecs: 3600,
+ cacheTimeInSecs: process.env.NODE_ENV === "production" ? 300 : 0,
+ // cacheTimeInSecs: 3600,
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
serviceName: "mdms-v2", | |
cacheTimeInSecs: 3600, | |
cacheTimeInSecs: 0, | |
// cacheTimeInSecs: 3600, | |
debounceTimeInMS: 100, | |
moduleSettings: [ | |
serviceName: "mdms-v2", | |
- cacheTimeInSecs: 0, | |
cacheTimeInSecs: process.env.NODE_ENV === "production" ? 300 : 0, | |
// cacheTimeInSecs: 3600, | |
debounceTimeInMS: 100, | |
moduleSettings: [ |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/libraries/src/services/atoms/ApiCacheService.js
around lines 29 to 33, the mdms-v2 entry sets cacheTimeInSecs to 0 which
disables caching in production; change it to a nonzero TTL (e.g., 3600 or a
value from a shared config/env var) to avoid cache=0 in production, remove or
update the commented-out example as needed, and ensure the value is driven by
environment/config so different environments can override it.
const moduleCode = ["privacy-policy"]; | ||
const language = Digit.StoreData.getCurrentLanguage(); | ||
const modulePrefix = "digit"; | ||
const { data: store } = Digit.Services.useStore({ | ||
stateCode, | ||
moduleCode, | ||
language, | ||
modulePrefix | ||
}); |
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.
🧹 Nitpick (assertive)
Remove unused store fetch (moduleCode/useStore) or add TODO
The fetched store is not used; drop it to reduce network and render work.
- const moduleCode = ["privacy-policy"];
- const language = Digit.StoreData.getCurrentLanguage();
- const modulePrefix = "digit";
- const { data: store } = Digit.Services.useStore({
- stateCode,
- moduleCode,
- language,
- modulePrefix
- });
If you plan to use it later, add a TODO instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const moduleCode = ["privacy-policy"]; | |
const language = Digit.StoreData.getCurrentLanguage(); | |
const modulePrefix = "digit"; | |
const { data: store } = Digit.Services.useStore({ | |
stateCode, | |
moduleCode, | |
language, | |
modulePrefix | |
}); |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js
around lines 14 to 22, the call to Digit.Services.useStore with
moduleCode/language/modulePrefix fetches a store that is never used; either
remove the moduleCode const and the useStore call entirely to avoid unnecessary
network/render work, or keep the call but add a clear TODO comment above it
explaining why the store fetch is retained for future use so reviewers know it’s
intentional.
// Clear API cache service | ||
if (window.Digit?.ApiCacheService) { | ||
window.Digit.ApiCacheService.clearAllCache(); | ||
} | ||
|
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.
🛠️ Refactor suggestion
Guard against missing ApiCacheService.clearAllCache to avoid runtime error
ApiCacheService in this repo doesn’t export clearAllCache by default. Either implement it (preferred) or guard the call.
- if (window.Digit?.ApiCacheService) {
- window.Digit.ApiCacheService.clearAllCache();
- }
+ if (typeof window.Digit?.ApiCacheService?.clearAllCache === "function") {
+ window.Digit.ApiCacheService.clearAllCache();
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Clear API cache service | |
if (window.Digit?.ApiCacheService) { | |
window.Digit.ApiCacheService.clearAllCache(); | |
} | |
// Clear API cache service | |
if (typeof window.Digit?.ApiCacheService?.clearAllCache === "function") { | |
window.Digit.ApiCacheService.clearAllCache(); | |
} |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js
around lines 65 to 69, the code assumes
window.Digit.ApiCacheService.clearAllCache exists and will throw if it is
undefined; either add a clearAllCache export on ApiCacheService implementation
or guard the call here by checking that
window.Digit?.ApiCacheService?.clearAllCache is a function before invoking it;
update the implementation if you opt to add the method to ApiCacheService so it
performs the intended cache clearing and export it, or if guarding, call it only
when present to avoid runtime errors.
// Timestamp handling | ||
useEffect(() => { | ||
const query = new URLSearchParams(location.search); | ||
if (!query.get("ts")) { | ||
const ts = Date.now(); | ||
history.replace({ | ||
pathname: location.pathname, | ||
search: `?ts=${ts}` | ||
}); | ||
} | ||
}, [location, history]); |
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.
🛠️ Refactor suggestion
Preserve existing query params when appending ts
Current logic drops existing params. Merge ts into the existing query instead.
- const query = new URLSearchParams(location.search);
- if (!query.get("ts")) {
- const ts = Date.now();
- history.replace({
- pathname: location.pathname,
- search: `?ts=${ts}`
- });
- }
+ const query = new URLSearchParams(location.search);
+ if (!query.get("ts")) {
+ query.set("ts", String(Date.now()));
+ history.replace({
+ pathname: location.pathname,
+ search: `?${query.toString()}`
+ });
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Timestamp handling | |
useEffect(() => { | |
const query = new URLSearchParams(location.search); | |
if (!query.get("ts")) { | |
const ts = Date.now(); | |
history.replace({ | |
pathname: location.pathname, | |
search: `?ts=${ts}` | |
}); | |
} | |
}, [location, history]); | |
// Timestamp handling | |
useEffect(() => { | |
const query = new URLSearchParams(location.search); | |
if (!query.get("ts")) { | |
query.set("ts", String(Date.now())); | |
history.replace({ | |
pathname: location.pathname, | |
search: `?${query.toString()}` | |
}); | |
} | |
}, [location, history]); |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Login-v2/index.js
around lines 117 to 127, the useEffect that appends a ts query parameter
currently discards any existing query params; update it to parse the existing
URLSearchParams from location.search, set/overwrite the "ts" param on that same
object, and then call history.replace with the pathname and the serialized
updated search so all other params are preserved; also use location.search (or
the existing search string) in the effect dependency array to avoid unnecessary
reruns and ensure the new search string is used when building the replace call.
if (window.Digit?.ApiCacheService) { | ||
window.Digit.ApiCacheService.clearAllCache(); | ||
} |
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.
🛠️ Refactor suggestion
Guard against missing ApiCacheService.clearAllCache to avoid runtime error
Same as Login-v2; make the call safe.
- if (window.Digit?.ApiCacheService) {
- window.Digit.ApiCacheService.clearAllCache();
- }
+ if (typeof window.Digit?.ApiCacheService?.clearAllCache === "function") {
+ window.Digit.ApiCacheService.clearAllCache();
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (window.Digit?.ApiCacheService) { | |
window.Digit.ApiCacheService.clearAllCache(); | |
} | |
if (typeof window.Digit?.ApiCacheService?.clearAllCache === "function") { | |
window.Digit.ApiCacheService.clearAllCache(); | |
} |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js
around lines 65 to 67, the code calls
window.Digit.ApiCacheService.clearAllCache() without confirming the method
exists, which can throw if ApiCacheService or clearAllCache is missing; update
the guard to check that window.Digit?.ApiCacheService?.clearAllCache is a
function (e.g., typeof ... === "function") before invoking it so the call is
safe at runtime.
useEffect(() => { | ||
const query = new URLSearchParams(location.search); | ||
if (!query.get("ts")) { | ||
const ts = Date.now(); | ||
history.replace({ | ||
pathname: location.pathname, | ||
search: `?ts=${ts}` | ||
}); | ||
} | ||
}, [location, history]); |
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.
🛠️ Refactor suggestion
Preserve existing query params when appending ts
Don’t discard other query params.
- const query = new URLSearchParams(location.search);
- if (!query.get("ts")) {
- const ts = Date.now();
- history.replace({
- pathname: location.pathname,
- search: `?ts=${ts}`
- });
- }
+ const query = new URLSearchParams(location.search);
+ if (!query.get("ts")) {
+ query.set("ts", String(Date.now()));
+ history.replace({
+ pathname: location.pathname,
+ search: `?${query.toString()}`
+ });
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
useEffect(() => { | |
const query = new URLSearchParams(location.search); | |
if (!query.get("ts")) { | |
const ts = Date.now(); | |
history.replace({ | |
pathname: location.pathname, | |
search: `?ts=${ts}` | |
}); | |
} | |
}, [location, history]); | |
useEffect(() => { | |
const query = new URLSearchParams(location.search); | |
if (!query.get("ts")) { | |
query.set("ts", String(Date.now())); | |
history.replace({ | |
pathname: location.pathname, | |
search: `?${query.toString()}` | |
}); | |
} | |
}, [location, history]); |
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/SignUp-v2/index.js
around lines 116 to 125, the effect currently overwrites the URL search string
when adding the ts param and thus discards any existing query parameters;
instead, build a URLSearchParams from location.search, set or append the ts
value on that same params object, and call history.replace with
location.pathname and the updated params.toString() so that all existing query
params are preserved while adding the ts.
* Update package.json * Update index.html * pgr inbox default filter set to all --------- Co-authored-by: syedkaif866 <[email protected]>
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (17)
micro-ui/web/micro-ui-internals/packages/modules/pgr/src/pages/employee/Inbox.js (6)
21-23
: Bug: unsafe optional chaining before join() + filter-key mismatch will throw/under-count.If
applicationStatus
is undefined,.join()
on the result of the optional.map()
will throw. Also, you readpgrfilters
while the initial state useswfFilters
. Use a safe default and fall back across both keys.Apply this diff:
- const applicationStatus = searchParams?.filters?.pgrfilters?.applicationStatus?.map(e => e.code).join(",") + // Support both legacy and current filter buckets; default to [] + const activeFilters = + searchParams?.filters?.pgrfilters ?? searchParams?.filters?.wfFilters ?? {}; + const applicationStatus = (activeFilters?.applicationStatus ?? []) + .map((e) => e.code) + .join(",");
19-27
: Stale state update risk in async effect + missing dependency on tenantId.If the component unmounts or tenant changes quickly,
setTotalRecords
may run on an unmounted component or with a stale tenant. Add a cancellation flag and includetenantId
in deps.Apply this diff:
- useEffect(() => { - (async () => { + useEffect(() => { + let cancelled = false; + (async () => { const applicationStatus = searchParams?.filters?.pgrfilters?.applicationStatus?.map(e => e.code).join(",") let response = await Digit.PGRService.count(tenantId, applicationStatus?.length > 0 ? {applicationStatus} : {} ); - if (response?.count) { - setTotalRecords(response.count); - } + if (!cancelled && typeof response?.count === "number") { + setTotalRecords(response.count); + } })(); - }, [searchParams]); + return () => { + cancelled = true; + }; + }, [tenantId, searchParams]);Note: If you apply the earlier “activeFilters” change, reuse that variable here to avoid duplication.
29-35
: Pagination step hard-coded to 10 and can go negative; also offset should reset on page-size change.Use
pageSize
as the step and clamp to zero. Reset offset when page size changes to avoid empty pages.Apply this diff:
const fetchNextPage = () => { - setPageOffset((prevState) => prevState + 10); + setPageOffset((prev) => prev + pageSize); }; const fetchPrevPage = () => { - setPageOffset((prevState) => prevState - 10); + setPageOffset((prev) => Math.max(prev - pageSize, 0)); }; const handlePageSizeChange = (e) => { - setPageSize(Number(e.target.value)); + const next = Number(e.target.value); + setPageSize(next); + setPageOffset(0); };Also applies to: 37-39
41-43
: Reset pagination on filter change to avoid empty pages.When filters change, the current offset may point beyond the new result set. Reset to the first page.
Apply this diff:
const handleFilterChange = (filterParam) => { - setSearchParams({ ...searchParams, filters: filterParam }); + setPageOffset(0); + setSearchParams({ ...searchParams, filters: filterParam }); };
54-81
: Loader logic is effectively dead;complaints?.length !== null
is always true. Gate byisLoading
and default data to [].The current condition renders the inbox even when
complaints
is undefined. Show Loader while loading, then render based on device.Apply this diff:
- if (complaints?.length !== null) { - if (isMobile) { - return ( - <MobileInbox data={complaints} isLoading={isLoading} onFilterChange={handleFilterChange} onSearch={onSearch} searchParams={searchParams} /> - ); - } else { - return ( - <div> - <Header>{t("ES_COMMON_INBOX")}</Header> - <DesktopInbox - data={complaints} - isLoading={isLoading} - onFilterChange={handleFilterChange} - onSearch={onSearch} - searchParams={searchParams} - onNextPage={fetchNextPage} - onPrevPage={fetchPrevPage} - onPageSizeChange={handlePageSizeChange} - currentPage={Math.floor(pageOffset / pageSize)} - totalRecords={totalRecords} - pageSizeLimit={pageSize} - /> - </div> - ); - } - } else { - return <Loader />; - } + if (isLoading) { + return <Loader />; + } + + if (isMobile) { + return ( + <MobileInbox + data={complaints || []} + isLoading={isLoading} + onFilterChange={handleFilterChange} + onSearch={onSearch} + searchParams={searchParams} + /> + ); + } + + return ( + <div> + <Header>{t("ES_COMMON_INBOX")}</Header> + <DesktopInbox + data={complaints || []} + isLoading={isLoading} + onFilterChange={handleFilterChange} + onSearch={onSearch} + searchParams={searchParams} + onNextPage={fetchNextPage} + onPrevPage={fetchPrevPage} + onPageSizeChange={handlePageSizeChange} + currentPage={Math.floor(pageOffset / pageSize)} + totalRecords={totalRecords} + pageSizeLimit={pageSize} + /> + </div> + );
49-51
: Minor: Prefer memoizing handlers to reduce downstream renders if DesktopInbox/MobileInbox depend on prop identity.Not a blocker, but wrapping handlers in
useCallback
with tight deps can help avoid unnecessary re-renders in large lists.I can provide a compact
useCallback
pass if desired.micro-ui/web/micro-ui-internals/packages/modules/pgr/src/components/inbox/Filter.js (11)
11-14
: Fix “Assigned to me” detection: compare to current user’s UUID instead of checking mere presence.Current logic sets “Assigned to me” when any assignee code exists, even if it’s not the current user. Also avoid mutating
uuid
later by renaming it immutably.Apply this diff:
- let { uuid } = Digit.UserService.getUser().info; + const { uuid: userUuid } = Digit.UserService.getUser().info; const { searchParams } = props; const { t } = useTranslation(); - const isAssignedToMe = searchParams?.filters?.wfFilters?.assignee && searchParams?.filters?.wfFilters?.assignee[0]?.code ? true : false; + const isAssignedToMe = + (searchParams?.filters?.wfFilters?.assignee?.[0]?.code ?? "") === userUuid;
26-26
: Sync selected radio when searchParams change; expand effect dependencies.With
[t]
only, switching filters externally won’t update the selected radio. IncludeisAssignedToMe
(and the options) in dependencies.Apply this diff:
-useEffect(() => setSelectedAssigned(isAssignedToMe ? assignedToOptions[0] : assignedToOptions[1]), [t]); +useEffect( + () => setSelectedAssigned(isAssignedToMe ? assignedToOptions[0] : assignedToOptions[1]), + [t, isAssignedToMe, assignedToOptions] +);
49-53
: Do not mutateuuid
; setassignee
to [] for “ALL” to match new empty-state semantics.Mutating
uuid
causes a hard bug: once set to""
, switching back to “Assigned to me” won’t recover the real UUID. Also avoid using[{ code: "" }]
for ALL.Apply this diff:
const onRadioChange = (value) => { setSelectedAssigned(value); - uuid = value.code === "ASSIGNED_TO_ME" ? uuid : ""; - setWfFilters({ ...wfFilters, assignee: [{ code: uuid }] }); + const assigneeCode = value.code === "ASSIGNED_TO_ME" ? userUuid : undefined; + setWfFilters({ + ...wfFilters, + assignee: assigneeCode ? [{ code: assigneeCode }] : [], + }); };
69-78
: Don’t wipe the entirewfQuery
when one filter entry is empty; delete the specific property instead.Resetting the whole object drops other wf filters unintentionally (TOCTOU-like behavior). Mirror the
pgrQuery
logic.Apply this diff:
for (const property in wfFilters) { if (Array.isArray(wfFilters[property])) { let params = wfFilters[property].map((prop) => prop.code).join(); if (params) { wfQuery[property] = params; - } else { - wfQuery = {}; + } else { + delete wfQuery?.[property]; } } }
142-151
: Fix CLEAR ALL: wrong key (assigned
vsassignee
), invalid shape, and invalid radio selectedOption.
- Uses
assigned
instead ofassignee
.- Sets
code: []
(should be string/undefined).- Sets
selectedAssigned
to""
which breaksRadioButtons
expecting an option object.Apply this diff:
function clearAll() { let pgrReset = { serviceCode: [], locality: [], applicationStatus: [] }; - let wfRest = { assigned: [{ code: [] }] }; + let wfReset = { assignee: [] }; setPgrFilters(pgrReset); - setWfFilters(wfRest); + setWfFilters(wfReset); pgrQuery = {}; wfQuery = {}; - setSelectedAssigned(""); + setSelectedAssigned(assignedToOptions[1]); // ASSIGNED_TO_ALL setSelectedComplaintType(null); setSelectedLocality(null); }
116-122
: Locality selected label usesname
butDropdown
expectsi18nkey
; this can render an empty label and break slicing.Align the placeholder object to the
optionKey="i18nkey"
you pass toDropdown
.Apply this diff:
useEffect(() => { if (pgrfilters.locality.length > 1) { - setSelectedLocality({ name: `${pgrfilters.locality.length} selected` }); + setSelectedLocality({ i18nkey: `${pgrfilters.locality.length} selected` }); } else { setSelectedLocality(pgrfilters.locality[0]); } }, [pgrfilters.locality]);Also applies to: 208-209
158-165
: Nit: “lable” typo in helper; minor readability improvement.Apply this diff:
-const GetSelectOptions = (lable, options, selected = null, select, optionKey, onRemove, key) => { +const GetSelectOptions = (label, options, selected = null, select, optionKey, onRemove, key) => { selected = selected || { [optionKey]: " ", code: "" }; return ( <div> - <div className="filter-label">{lable}</div> + <div className="filter-label">{label}</div> {<Dropdown option={options} selected={selected} select={(value) => select(value, key)} optionKey={optionKey} />}
166-169
: Guard against undefined labels in tags; avoid always appending ellipsis.
value[optionKey]
can be undefined and.slice
will throw. Also append...
only when truncated.Apply this diff:
- {pgrfilters[key].length > 0 && - pgrfilters[key].map((value, index) => { - return <RemoveableTag key={index} text={`${value[optionKey].slice(0, 22)} ...`} onClick={() => onRemove(index, key)} />; - })} + {pgrfilters[key].length > 0 && + pgrfilters[key].map((value, index) => { + const text = value?.[optionKey] || ""; + const displayText = text.length > 22 ? `${text.slice(0, 22)} ...` : text; + return <RemoveableTag key={index} text={displayText} onClick={() => onRemove(index, key)} />; + })}
2-2
: Remove unused importRoundedLabel
.Apply this diff:
-import { Dropdown, RadioButtons, ActionBar, RemoveableTag, RoundedLabel } from "@egovernments/digit-ui-react-components"; +import { Dropdown, RadioButtons, ActionBar, RemoveableTag } from "@egovernments/digit-ui-react-components";
7-9
: Avoid mutable module-levelpgrQuery
/wfQuery
; compute per-render or keep in refs/state.Global mutable objects can leak across component instances and cause stale reads in React’s concurrent rendering. Build queries inside the effect, compare with previous, then emit via
onFilterChange
, e.g., use localconst nextPgrQuery = {}
/const nextWfQuery = {}
anduseRef
to avoid unnecessary emits.Would you like a patch that removes the globals and performs a shallow-equal check before calling
onFilterChange
?
81-86
: Desktop: FrequentonFilterChange
calls—consider dedupe/debounce to avoid excessive network traffic.Auto-apply on every state change is UX-friendly but can be chatty. Guard by comparing the newly computed queries to the previous ones, or add a small debounce.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
micro-ui/web/micro-ui-internals/packages/modules/pgr/src/components/inbox/Filter.js
(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/pgr/src/pages/employee/Inbox.js
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit configuration file
check
Files:
micro-ui/web/micro-ui-internals/packages/modules/pgr/src/pages/employee/Inbox.js
micro-ui/web/micro-ui-internals/packages/modules/pgr/src/components/inbox/Filter.js
⏰ Context from checks skipped due to timeout of 10000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
micro-ui/web/micro-ui-internals/packages/modules/pgr/src/components/inbox/Filter.js (1)
38-42
: DefaultingwfFilters.assignee
to an empty array changes initial semantics—verify downstream handling.Initializing to
[]
is reasonable, but ensure all handlers treat “Assigned to All” as an empty list (not[{ code: "" }]
) and that query builders and counts don’t assume a one-item default. Follow-up fixes are proposed in separate comments for Lines 49-53, 69-78, and 142-151.
// const [searchParams, setSearchParams] = useState({ filters: { wfFilters: { assignee: [{ code: uuid }] } }, search: "", sort: {} }); | ||
|
||
const [searchParams, setSearchParams] = useState({ filters: { wfFilters: { assignee: [] } }, search: "", sort: {} }); | ||
|
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.
🧹 Nitpick (assertive)
Defaulting assignee to [] changes the initial scope to "all" — verify product intent and cross-file consistency.
Previously this inbox opened “Assigned to me”. With assignee: []
it will fetch everything unless user narrows filters, which can hurt performance and UX for large tenants. Also, this file initializes wfFilters
, but the count effect below reads from pgrfilters
, risking a mismatch between the grid and the count. Decide one canonical key and confirm the default behavior with Product.
If the intent is to start unassigned, I can align Filter.js and the API adapters on a single key and wire an “Assigned to me” quick filter as the default radio without injecting into initial state. Want me to raise a follow-up PR?
🤖 Prompt for AI Agents
In
micro-ui/web/micro-ui-internals/packages/modules/pgr/src/pages/employee/Inbox.js
around lines 15-18, the initial state was changed from assignee: [{ code: uuid
}] to assignee: [], which expands the inbox from “Assigned to me” to “All” and
also exposes a mismatch between the wfFilters key here and the pgrfilters key
used by the count effect; confirm product intent (should default be “Assigned to
me” or “All”), then make one of two fixes: if default should be “Assigned to
me”, restore assignee to [{ code: uuid }] and ensure the count effect reads the
same wfFilters key; otherwise, if default should be “All”, keep assignee: [] but
update Filter.js and API adapters to consistently use wfFilters (or rename
wfFilters→pgrfilters everywhere) and change the count effect to read the
canonical key so grid and count align, and wire the “Assigned to me”
quick-filter as a default radio option instead of injecting into initial state.
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
♻️ Duplicate comments (2)
micro-ui/web/public/index.html (2)
10-11
: Add SRI + crossorigin to external CSS (repeat): protect against CDN tamperingPrevious review already requested this; still missing on both links.
Apply this patch (rehash if assets change):
- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> - <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> + <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" + integrity="sha384-ujgPEZxME0vI6gml3R9fLnJBJJtuX7crZuyXk7dSmmGevF1r5ZO0O7ty3PBJPFkC" + crossorigin="anonymous" /> + <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" + integrity="sha384-2EfdlCC1EigMYJ59qOhrK2CfvNH6nzQpPONIej1kQQqdSp4/kpLXsw9Zm3/y7XnC" + crossorigin="anonymous" />
100-119
: Invalid HTML: <style> is outside and centering is incorrect (repeat)Move this block into the head. The current negative margin (-10.3rem) is wrong for a 7.5rem-wide tooltip; use transform for robust centering and add font fallbacks.
Remove from here:
-<style> - .digit-header-content.label .info-icon .infotext { - position: absolute; - text-align: center; - font-family: Roboto; - font-style: normal; - font-weight: 400; - line-height: 1.37rem; - visibility: hidden; - width: 7.5rem; - background-color: #555; - color: #fff; - padding: .5rem 0; - z-index: 1; - bottom: 100%; - left: 50%; - margin-left: -10.3rem; - border-radius: .375rem; - } -</style>Add inside the head (just before
</head>
):<style> .digit-header-content.label .info-icon .infotext { position: absolute; text-align: center; font-family: "Roboto", Arial, sans-serif; font-style: normal; font-weight: 400; line-height: 1.37rem; visibility: hidden; width: 7.5rem; background-color: #555; color: #fff; padding: .5rem 0; z-index: 1000; bottom: 100%; left: 50%; transform: translateX(-50%); border-radius: .375rem; } </style>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
micro-ui/web/public/index.html
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-03-25T10:47:10.916Z
Learnt from: Tulika-eGov
PR: egovernments/DIGIT-Frontend#2288
File: micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/Landing/LandingComponent.js:33-33
Timestamp: 2025-03-25T10:47:10.916Z
Learning: In DIGIT-Frontend, window.contextPath is dynamically determined and cannot be given a static fallback value like 'digit-ui'. The application relies on this dynamic context path for proper routing.
Applied to files:
micro-ui/web/public/index.html
<!-- Matomo --> | ||
<script> | ||
var _paq = window._paq = window._paq || []; | ||
/* tracker methods like "setCustomDimension" should be called before "trackPageView" */ | ||
_paq.push(["setExcludedQueryParams", ["account", "accountnum", "address", "address1", "address2", "address3", "addressline1", "addressline2", "adres", "adresse", "age", "alter", "auth", "authpw", "bic", "billingaddress", "billingaddress1", "billingaddress2", "calle", "cardnumber", "cc", "ccc", "cccsc", "cccvc", "cccvv", "ccexpiry", "ccexpmonth", "ccexpyear", "ccname", "ccnumber", "cctype", "cell", "cellphone", "city", "clientid", "clientsecret", "company", "consumerkey", "consumersecret", "contrasenya", "contrase\u00f1a", "creditcard", "creditcardnumber", "cvc", "cvv", "dateofbirth", "debitcard", "direcci\u00f3n", "dob", "domain", "ebost", "email", "emailaddress", "emailadresse", "epos", "epost", "eposta", "exp", "familyname", "firma", "firstname", "formlogin", "fullname", "gender", "geschlecht", "gst", "gstnumber", "handynummer", "has\u0142o", "heslo", "iban", "ibanaccountnum", "ibanaccountnumber", "id", "identifier", "indirizzo", "kartakredytowa", "kennwort", "keyconsumerkey", "keyconsumersecret", "konto", "kontonr", "kontonummer", "kredietkaart", "kreditkarte", "kreditkort", "lastname", "login", "mail", "mobiili", "mobile", "mobilne", "nachname", "name", "nickname", "osoite", "parole", "pass", "passord", "password", "passwort", "pasword", "paswort", "paword", "phone", "pin", "plz", "postalcode", "postcode", "postleitzahl", "privatekey", "publickey", "pw", "pwd", "pword", "pwrd", "rue", "secret", "secretq", "secretquestion", "shippingaddress", "shippingaddress1", "shippingaddress2", "socialsec", "socialsecuritynumber", "socsec", "sokak", "ssn", "steuernummer", "strasse", "street", "surname", "swift", "tax", "taxnumber", "tel", "telefon", "telefonnr", "telefonnummer", "telefono", "telephone", "token", "token_auth", "tokenauth", "t\u00e9l\u00e9phone", "ulica", "user", "username", "vat", "vatnumber", "via", "vorname", "wachtwoord", "wagwoord", "webhooksecret", "website", "zip", "zipcode"]]); | ||
_paq.push(['trackPageView']); | ||
_paq.push(['enableLinkTracking']); | ||
(function () { | ||
var u = "https://unified-demo.digit.org/matomo/"; | ||
_paq.push(['setTrackerUrl', u + 'matomo.php']); | ||
_paq.push(['setSiteId', '1']); | ||
// Adding a second tracker | ||
_paq.push(['addTracker', 'https://digit-analytics.matomo.cloud/matomo.php', '2']); | ||
var d = document, g = d.createElement('script'), s = d.getElementsByTagName('script')[0]; | ||
g.async = true; g.src = u + 'matomo.js'; | ||
s.parentNode.insertBefore(g, s); | ||
})(); |
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.
🧹 Nitpick (assertive)
🛠️ Refactor suggestion
Matomo: ensure both trackers receive the initial pageview; add privacy controls
Currently, the first trackPageView
fires before addTracker
, so tracker "2" may miss the initial PV. Also add basic privacy settings (GDPR-friendly).
<script>
var _paq = window._paq = window._paq || [];
- /* tracker methods like "setCustomDimension" should be called before "trackPageView" */
+ /* tracker methods like "setCustomDimension" should be called before "trackPageView" */
+ // Privacy hardening
+ _paq.push(['disableCookies']); // cookieless analytics
+ _paq.push(['anonymizeIp']); // mask IP
+ _paq.push(['setDoNotTrack', true]); // honor DNT
+ _paq.push(['setCookieSameSite', 'Lax']); // safer defaults for any residual cookies
_paq.push(["setExcludedQueryParams", [/* …existing list… */]]);
- _paq.push(['trackPageView']);
- _paq.push(['enableLinkTracking']);
(function () {
var u = "https://unified-demo.digit.org/matomo/";
_paq.push(['setTrackerUrl', u + 'matomo.php']);
_paq.push(['setSiteId', '1']);
- // Adding a second tracker
- _paq.push(['addTracker', 'https://digit-analytics.matomo.cloud/matomo.php', '2']);
+ // Add second tracker before tracking so both get the initial PV
+ _paq.push(['addTracker', 'https://digit-analytics.matomo.cloud/matomo.php', '2']);
+ _paq.push(['enableLinkTracking']);
+ _paq.push(['trackPageView']); // initial PV goes to all configured trackers
var d = document, g = d.createElement('script'), s = d.getElementsByTagName('script')[0];
g.async = true; g.src = u + 'matomo.js';
s.parentNode.insertBefore(g, s);
})();
Optional: If consent is required, gate tracking until consent with _paq.push(['requireConsent']);
or _paq.push(['requireCookieConsent']);
and trigger once obtained.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<!-- Matomo --> | |
<script> | |
var _paq = window._paq = window._paq || []; | |
/* tracker methods like "setCustomDimension" should be called before "trackPageView" */ | |
_paq.push(["setExcludedQueryParams", ["account", "accountnum", "address", "address1", "address2", "address3", "addressline1", "addressline2", "adres", "adresse", "age", "alter", "auth", "authpw", "bic", "billingaddress", "billingaddress1", "billingaddress2", "calle", "cardnumber", "cc", "ccc", "cccsc", "cccvc", "cccvv", "ccexpiry", "ccexpmonth", "ccexpyear", "ccname", "ccnumber", "cctype", "cell", "cellphone", "city", "clientid", "clientsecret", "company", "consumerkey", "consumersecret", "contrasenya", "contrase\u00f1a", "creditcard", "creditcardnumber", "cvc", "cvv", "dateofbirth", "debitcard", "direcci\u00f3n", "dob", "domain", "ebost", "email", "emailaddress", "emailadresse", "epos", "epost", "eposta", "exp", "familyname", "firma", "firstname", "formlogin", "fullname", "gender", "geschlecht", "gst", "gstnumber", "handynummer", "has\u0142o", "heslo", "iban", "ibanaccountnum", "ibanaccountnumber", "id", "identifier", "indirizzo", "kartakredytowa", "kennwort", "keyconsumerkey", "keyconsumersecret", "konto", "kontonr", "kontonummer", "kredietkaart", "kreditkarte", "kreditkort", "lastname", "login", "mail", "mobiili", "mobile", "mobilne", "nachname", "name", "nickname", "osoite", "parole", "pass", "passord", "password", "passwort", "pasword", "paswort", "paword", "phone", "pin", "plz", "postalcode", "postcode", "postleitzahl", "privatekey", "publickey", "pw", "pwd", "pword", "pwrd", "rue", "secret", "secretq", "secretquestion", "shippingaddress", "shippingaddress1", "shippingaddress2", "socialsec", "socialsecuritynumber", "socsec", "sokak", "ssn", "steuernummer", "strasse", "street", "surname", "swift", "tax", "taxnumber", "tel", "telefon", "telefonnr", "telefonnummer", "telefono", "telephone", "token", "token_auth", "tokenauth", "t\u00e9l\u00e9phone", "ulica", "user", "username", "vat", "vatnumber", "via", "vorname", "wachtwoord", "wagwoord", "webhooksecret", "website", "zip", "zipcode"]]); | |
_paq.push(['trackPageView']); | |
_paq.push(['enableLinkTracking']); | |
(function () { | |
var u = "https://unified-demo.digit.org/matomo/"; | |
_paq.push(['setTrackerUrl', u + 'matomo.php']); | |
_paq.push(['setSiteId', '1']); | |
// Adding a second tracker | |
_paq.push(['addTracker', 'https://digit-analytics.matomo.cloud/matomo.php', '2']); | |
var d = document, g = d.createElement('script'), s = d.getElementsByTagName('script')[0]; | |
g.async = true; g.src = u + 'matomo.js'; | |
s.parentNode.insertBefore(g, s); | |
})(); | |
<!-- Matomo --> | |
<script> | |
var _paq = window._paq = window._paq || []; | |
/* tracker methods like "setCustomDimension" should be called before "trackPageView" */ | |
// Privacy hardening | |
_paq.push(['disableCookies']); // cookieless analytics | |
_paq.push(['anonymizeIp']); // mask IP | |
_paq.push(['setDoNotTrack', true]); // honor DNT | |
_paq.push(['setCookieSameSite', 'Lax']); // safer defaults for any residual cookies | |
_paq.push(["setExcludedQueryParams", ["account", "accountnum", "address", "address1", "address2", "address3", "addressline1", "addressline2", "adres", "adresse", "age", "alter", "auth", "authpw", "bic", "billingaddress", "billingaddress1", "billingaddress2", "calle", "cardnumber", "cc", "ccc", "cccsc", "cccvc", "cccvv", "ccexpiry", "ccexpmonth", "ccexpyear", "ccname", "ccnumber", "cctype", "cell", "cellphone", "city", "clientid", "clientsecret", "company", "consumerkey", "consumersecret", "contrasenya", "contrase\u00f1a", "creditcard", "creditcardnumber", "cvc", "cvv", "dateofbirth", "debitcard", "direcci\u00f3n", "dob", "domain", "ebost", "email", "emailaddress", "emailadresse", "epos", "epost", "eposta", "exp", "familyname", "firma", "firstname", "formlogin", "fullname", "gender", "geschlecht", "gst", "gstnumber", "handynummer", "has\u0142o", "heslo", "iban", "ibanaccountnum", "ibanaccountnumber", "id", "identifier", "indirizzo", "kartakredytowa", "kennwort", "keyconsumerkey", "keyconsumersecret", "konto", "kontonr", "kontonummer", "kredietkaart", "kreditkarte", "kreditkort", "lastname", "login", "mail", "mobiili", "mobile", "mobilne", "nachname", "name", "nickname", "osoite", "parole", "pass", "passord", "password", "passwort", "pasword", "paswort", "paword", "phone", "pin", "plz", "postalcode", "postcode", "postleitzahl", "privatekey", "publickey", "pw", "pwd", "pword", "pwrd", "rue", "secret", "secretq", "secretquestion", "shippingaddress", "shippingaddress1", "shippingaddress2", "socialsec", "socialsecuritynumber", "socsec", "sokak", "ssn", "steuernummer", "strasse", "street", "surname", "swift", "tax", "taxnumber", "tel", "telefon", "telefonnr", "telefonnummer", "telefono", "telephone", "token", "token_auth", "tokenauth", "t\u00e9l\u00e9phone", "ulica", "user", "username", "vat", "vatnumber", "via", "vorname", "wachtwoord", "wagwoord", "webhooksecret", "website", "zip", "zipcode"]]); | |
(function () { | |
var u = "https://unified-demo.digit.org/matomo/"; | |
_paq.push(['setTrackerUrl', u + 'matomo.php']); | |
_paq.push(['setSiteId', '1']); | |
// Add second tracker before tracking so both get the initial PV | |
_paq.push(['addTracker', 'https://digit-analytics.matomo.cloud/matomo.php', '2']); | |
_paq.push(['enableLinkTracking']); | |
_paq.push(['trackPageView']); // initial PV goes to all configured trackers | |
var d = document, g = d.createElement('script'), s = d.getElementsByTagName('script')[0]; | |
g.async = true; g.src = u + 'matomo.js'; | |
s.parentNode.insertBefore(g, s); | |
})(); | |
</script> |
🤖 Prompt for AI Agents
In micro-ui/web/public/index.html around lines 17-33, the initial
_paq.push(['trackPageView']) runs before the second tracker is added so tracker
"2" may miss the first pageview; move the trackPageView and enableLinkTracking
calls to after _paq.push(['addTracker', ...]) (or explicitly send pageview to
both trackers via tracker-scoped calls), and add privacy controls by pushing
requireConsent or requireCookieConsent at init and honoring navigator.doNotTrack
(e.g., push ['setDoNotTrack', true] or skip tracking when DNT is set) so
tracking is gated until consent.
let lastUrl = location.href; | ||
|
||
function trackUrlChange() { | ||
const url = location.href; | ||
if (url !== lastUrl) { | ||
lastUrl = url; | ||
_paq.push(['setCustomUrl', url]); | ||
_paq.push(['trackPageView']); | ||
} | ||
} | ||
|
||
// Wrap pushState | ||
history.pushState = (function (original) { | ||
return function pushState() { | ||
const result = original.apply(this, arguments); | ||
trackUrlChange(); | ||
return result; | ||
}; | ||
})(history.pushState); | ||
|
||
// Wrap replaceState | ||
history.replaceState = (function (original) { | ||
return function replaceState() { | ||
const result = original.apply(this, arguments); | ||
trackUrlChange(); | ||
return result; | ||
}; | ||
})(history.replaceState); | ||
|
||
// Listen for back/forward | ||
window.addEventListener('popstate', trackUrlChange); | ||
})(); |
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.
🧹 Nitpick (assertive)
SPA tracking wrapper: add idempotency + hashchange; verify no duplicate PVs
Avoid accidental double-wrapping and track hash-based navigations if any routes use hashes. Verify no duplicate pageviews fire on initial render or single navigation.
(function () {
let lastUrl = location.href;
+ if (window.__matomoSpaWrapped) return;
+ window.__matomoSpaWrapped = true;
function trackUrlChange() {
const url = location.href;
if (url !== lastUrl) {
lastUrl = url;
_paq.push(['setCustomUrl', url]);
_paq.push(['trackPageView']);
}
}
// Wrap pushState
history.pushState = (function (original) {
return function pushState() {
const result = original.apply(this, arguments);
trackUrlChange();
return result;
};
})(history.pushState);
// Wrap replaceState
history.replaceState = (function (original) {
return function replaceState() {
const result = original.apply(this, arguments);
trackUrlChange();
return result;
};
})(history.replaceState);
// Listen for back/forward
window.addEventListener('popstate', trackUrlChange);
+ // Also cover hash-based routing (if any)
+ window.addEventListener('hashchange', trackUrlChange);
})();
Manual check: open DevTools network tab, filter “matomo.php”, navigate across routes; expect exactly one PV per route transition across both trackers.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
let lastUrl = location.href; | |
function trackUrlChange() { | |
const url = location.href; | |
if (url !== lastUrl) { | |
lastUrl = url; | |
_paq.push(['setCustomUrl', url]); | |
_paq.push(['trackPageView']); | |
} | |
} | |
// Wrap pushState | |
history.pushState = (function (original) { | |
return function pushState() { | |
const result = original.apply(this, arguments); | |
trackUrlChange(); | |
return result; | |
}; | |
})(history.pushState); | |
// Wrap replaceState | |
history.replaceState = (function (original) { | |
return function replaceState() { | |
const result = original.apply(this, arguments); | |
trackUrlChange(); | |
return result; | |
}; | |
})(history.replaceState); | |
// Listen for back/forward | |
window.addEventListener('popstate', trackUrlChange); | |
})(); | |
(function () { | |
let lastUrl = location.href; | |
if (window.__matomoSpaWrapped) return; | |
window.__matomoSpaWrapped = true; | |
function trackUrlChange() { | |
const url = location.href; | |
if (url !== lastUrl) { | |
lastUrl = url; | |
_paq.push(['setCustomUrl', url]); | |
_paq.push(['trackPageView']); | |
} | |
} | |
// Wrap pushState | |
history.pushState = (function (original) { | |
return function pushState() { | |
const result = original.apply(this, arguments); | |
trackUrlChange(); | |
return result; | |
}; | |
})(history.pushState); | |
// Wrap replaceState | |
history.replaceState = (function (original) { | |
return function replaceState() { | |
const result = original.apply(this, arguments); | |
trackUrlChange(); | |
return result; | |
}; | |
})(history.replaceState); | |
// Listen for back/forward | |
window.addEventListener('popstate', trackUrlChange); | |
// Also cover hash-based routing (if any) | |
window.addEventListener('hashchange', trackUrlChange); | |
})(); |
🤖 Prompt for AI Agents
In micro-ui/web/public/index.html around lines 35 to 66, the SPA history
wrappers can be double-wrapped and they don’t handle hash-based navigation which
may cause duplicate pageviews; make the wrapper idempotent by checking and
skipping if pushState/replaceState have already been wrapped (e.g. by a marker
on the function or a flag), add window.addEventListener('hashchange',
trackUrlChange) so hash navigations are tracked, and ensure trackUrlChange
prevents duplicate pageviews on initial render by initializing lastUrl after
initial push or by guarding the first call (so only one PV fires per route
transition).
// Track clicks on buttons & links | ||
(function () { | ||
document.addEventListener("click", function (e) { | ||
const selector = ` | ||
button, | ||
input[type="button"], | ||
input[type="submit"], | ||
input[type="actionButton"], | ||
[role="button"], | ||
a[href]:not([href^="mailto:"]):not([href^="tel:"]):not([href^="#"]):not([href^="javascript:"]) | ||
`; | ||
const target = e.target.closest(selector); | ||
if (target) { | ||
const elementType = target.tagName.toLowerCase(); | ||
const text = target.innerText.trim() || target.value || 'Unnamed'; | ||
const id = target.id || 'no-id'; | ||
const classes = target.className || 'no-class'; | ||
const href = target.getAttribute('href') || ''; | ||
|
||
_paq.push([ | ||
'trackEvent', | ||
'UI Interaction', // Category | ||
`${elementType} click`, // Action | ||
`Text: ${text}, ID: ${id}, Class: ${classes}, Href: ${href}` | ||
]); | ||
} | ||
}); | ||
})(); |
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.
🛠️ Refactor suggestion
Click tracking may leak PII via full href/text/classes; sanitize before sending
The event label currently includes raw href (with query), full text, id, and classes. On auth pages this risks capturing tokens or user-identifying info.
// Track clicks on buttons & links
(function () {
document.addEventListener("click", function (e) {
const selector = `
button,
input[type="button"],
input[type="submit"],
input[type="actionButton"],
[role="button"],
a[href]:not([href^="mailto:"]):not([href^="tel:"]):not([href^="#"]):not([href^="javascript:"])
`;
const target = e.target.closest(selector);
- if (target) {
+ if (target && !target.closest('[data-analytics="off"]')) {
const elementType = target.tagName.toLowerCase();
- const text = target.innerText.trim() || target.value || 'Unnamed';
- const id = target.id || 'no-id';
- const classes = target.className || 'no-class';
- const href = target.getAttribute('href') || '';
+ const rawText = (target.innerText || target.value || '').trim();
+ const text = rawText.slice(0, 120) || 'Unnamed';
+ const id = target.id ? 'has-id' : 'no-id';
+ const classes = (target.dataset && target.dataset.analyticsLabel) ? target.dataset.analyticsLabel : 'generic';
+ const hrefAttr = target.getAttribute('href') || '';
+ let sanitizedHref = '';
+ try {
+ const u = new URL(hrefAttr, location.href);
+ sanitizedHref = u.origin + u.pathname; // drop query/hash entirely
+ } catch (_) {}
_paq.push([
'trackEvent',
'UI Interaction', // Category
`${elementType} click`, // Action
- `Text: ${text}, ID: ${id}, Class: ${classes}, Href: ${href}`
+ `Text: ${text}, ID: ${id}, Label: ${classes}, Href: ${sanitizedHref}`
]);
}
});
})();
Usage: allow components to set a stable label via data-analytics-label="login-submit"
and opt out via data-analytics="off"
.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Track clicks on buttons & links | |
(function () { | |
document.addEventListener("click", function (e) { | |
const selector = ` | |
button, | |
input[type="button"], | |
input[type="submit"], | |
input[type="actionButton"], | |
[role="button"], | |
a[href]:not([href^="mailto:"]):not([href^="tel:"]):not([href^="#"]):not([href^="javascript:"]) | |
`; | |
const target = e.target.closest(selector); | |
if (target) { | |
const elementType = target.tagName.toLowerCase(); | |
const text = target.innerText.trim() || target.value || 'Unnamed'; | |
const id = target.id || 'no-id'; | |
const classes = target.className || 'no-class'; | |
const href = target.getAttribute('href') || ''; | |
_paq.push([ | |
'trackEvent', | |
'UI Interaction', // Category | |
`${elementType} click`, // Action | |
`Text: ${text}, ID: ${id}, Class: ${classes}, Href: ${href}` | |
]); | |
} | |
}); | |
})(); | |
// Track clicks on buttons & links | |
(function () { | |
document.addEventListener("click", function (e) { | |
const selector = ` | |
button, | |
input[type="button"], | |
input[type="submit"], | |
input[type="actionButton"], | |
[role="button"], | |
a[href]:not([href^="mailto:"]):not([href^="tel:"]):not([href^="#"]):not([href^="javascript:"]) | |
`; | |
const target = e.target.closest(selector); | |
// opt out of analytics if ancestor has data-analytics="off" | |
if (target && !target.closest('[data-analytics="off"]')) { | |
const elementType = target.tagName.toLowerCase(); | |
const rawText = (target.innerText || target.value || '').trim(); | |
const text = rawText.slice(0, 120) || 'Unnamed'; | |
// only indicate presence of an ID rather than its value | |
const id = target.id ? 'has-id' : 'no-id'; | |
// use a stable, component-provided label if present | |
const classes = target.dataset.analyticsLabel || 'generic'; | |
const hrefAttr = target.getAttribute('href') || ''; | |
let sanitizedHref = ''; | |
try { | |
const u = new URL(hrefAttr, location.href); | |
// drop query and hash to avoid leaking tokens/user data | |
sanitizedHref = u.origin + u.pathname; | |
} catch (_) {} | |
_paq.push([ | |
'trackEvent', | |
'UI Interaction', | |
`${elementType} click`, | |
`Text: ${text}, ID: ${id}, Label: ${classes}, Href: ${sanitizedHref}` | |
]); | |
} | |
}); | |
})(); |
🤖 Prompt for AI Agents
In micro-ui/web/public/index.html around lines 68 to 95, the click-tracking code
currently sends full href, innerText, id and classes which can leak PII or
tokens; update it to first check for an opt-out attribute data-analytics="off"
and skip tracking, then prefer a stable label from data-analytics-label if
present (use that as the action/label), otherwise sanitize any generated label
by stripping href query/fragment (keep only origin+pathname), truncating
innerText to a safe length (e.g. 100 chars) and omitting or hashing classes/ids
instead of sending raw values; ensure links without href or sensitive protocols
are handled, and only push the sanitized values to _paq.
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
New Features
UI/UX Improvements
Behavioral Changes
Chores
Performance