-
Notifications
You must be signed in to change notification settings - Fork 46
Accessibility added for few pages #3066
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: console
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughUpdates webpack devServer configuration for workbench UI routing and static path, adds keyboard accessibility (Enter/Space) to a campaign creation card, and switches component imports to a different package namespace. No public API changes beyond webpack config adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as Campaign Card
participant H as Handler
U->>C: Click
C->>H: onClick()
H-->>H: Trigger existing action
U->>C: KeyDown (Enter/Space)
C->>H: onKeyDown()
H-->>H: Trigger same action
note over C,H: New keyboard path mirrors click behavior
sequenceDiagram
autonumber
participant B as Browser
participant Dev as Webpack DevServer
participant SPA as Workbench UI
B->>Dev: GET /workbench-ui/...
Dev-->>Dev: historyApiFallback rewrite
Dev->>B: /workbench-ui/index.html
B->>Dev: Static assets (from build/)
Dev-->>B: Serve from build
note over Dev: Index set to /workbench-ui/index.html with rewrites
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
health/micro-ui/web/micro-ui-internals/example/webpack.config.js (1)
114-120: Tighten resolve.alias to avoid case-variant importsUppercase aliases (“React”, “ReactDOM”) can mask incorrect imports and increase bundle risk. Prefer only the canonical lowercase module names.
- alias: { - // Fix case sensitivity issues with React - "React": path.resolve(__dirname, "../node_modules/react"), - "react": path.resolve(__dirname, "../node_modules/react"), - "ReactDOM": path.resolve(__dirname, "../node_modules/react-dom"), - "react-dom": path.resolve(__dirname, "../node_modules/react-dom"), - }, + alias: { + react: path.resolve(__dirname, "../node_modules/react"), + "react-dom": path.resolve(__dirname, "../node_modules/react-dom"), + },
📜 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 ignored due to path filters (2)
health/micro-ui/web/micro-ui-internals/package.jsonis excluded by!**/*.jsonhealth/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/package.jsonis excluded by!**/*.json
📒 Files selected for processing (3)
health/micro-ui/web/micro-ui-internals/example/webpack.config.js(4 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/NewCampaignCreate/CampaignHome.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit configuration file
check
Files:
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/NewCampaignCreate/CampaignHome.jshealth/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.jshealth/micro-ui/web/micro-ui-internals/example/webpack.config.js
🧠 Learnings (4)
📚 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:
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/NewCampaignCreate/CampaignHome.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/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:
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.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:
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.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:
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.js
🧬 Code graph analysis (1)
health/micro-ui/web/micro-ui-internals/example/webpack.config.js (2)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/webpack.config.js (1)
isProduction(7-7)micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/webpack.config.js (1)
isProduction(3-3)
🪛 Biome (2.1.2)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/NewCampaignCreate/CampaignHome.js
[error] 140-140: The HTML element div is non-interactive. Do not use tabIndex.
Adding non-interactive elements to the keyboard navigation flow can confuse users.
Unsafe fix: Remove the tabIndex attribute.
(lint/a11y/noNoninteractiveTabindex)
🔇 Additional comments (3)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/SetupCampaign.js (1)
1-1: Import source switch looks goodFormComposerV2 now comes from @egovernments/digit-ui-components. No issues spotted with this change.
health/micro-ui/web/micro-ui-internals/example/webpack.config.js (2)
48-95: SplitChunks tuning looks reasonableVendor and module-specific groups with async loading for heavy modules are sensible defaults.
105-107: Injecting env into template only in dev: LGTMKeeps prod HTML clean while preserving local configurability.
| static: path.join(__dirname, "build"), | ||
| compress: true, | ||
| port: 3000, | ||
| hot: true, | ||
| historyApiFallback: true, | ||
| watchFiles:isProduction?undefined: { | ||
| historyApiFallback: { | ||
| index: '/workbench-ui/index.html', | ||
| rewrites: [ | ||
| { from: /^\/workbench-ui/, to: '/workbench-ui/index.html' } | ||
| ] | ||
| }, | ||
| watchFiles: isProduction ? undefined : { | ||
| paths: ["**/*"], // watch all project files |
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 routing base path: add dot-rule exception
Deep links containing dots (e.g., /workbench-ui/some.route) may 404. Add disableDotRule: true for robust fallbacks.
- historyApiFallback: {
+ historyApiFallback: {
+ disableDotRule: true,
index: '/workbench-ui/index.html',
rewrites: [
{ from: /^\/workbench-ui/, to: '/workbench-ui/index.html' }
]
},📝 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.
| static: path.join(__dirname, "build"), | |
| compress: true, | |
| port: 3000, | |
| hot: true, | |
| historyApiFallback: true, | |
| watchFiles:isProduction?undefined: { | |
| historyApiFallback: { | |
| index: '/workbench-ui/index.html', | |
| rewrites: [ | |
| { from: /^\/workbench-ui/, to: '/workbench-ui/index.html' } | |
| ] | |
| }, | |
| watchFiles: isProduction ? undefined : { | |
| paths: ["**/*"], // watch all project files | |
| static: path.join(__dirname, "build"), | |
| compress: true, | |
| port: 3000, | |
| hot: true, | |
| historyApiFallback: { | |
| disableDotRule: true, | |
| index: '/workbench-ui/index.html', | |
| rewrites: [ | |
| { from: /^\/workbench-ui/, to: '/workbench-ui/index.html' } | |
| ] | |
| }, | |
| watchFiles: isProduction ? undefined : { | |
| paths: ["**/*"], // watch all project files |
🤖 Prompt for AI Agents
In health/micro-ui/web/micro-ui-internals/example/webpack.config.js around lines
126 to 137 the historyApiFallback config can 404 on deep links containing dots
(e.g., /workbench-ui/some.route); add disableDotRule: true to the
historyApiFallback object so the fallback will match paths with dots, keeping
the existing index and rewrites intact.
| tabIndex={0} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| setShowPopUp(true); | ||
| }}} | ||
| className={"cardStyle"} |
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
Fix extra brace and make the card fully accessible
- There's a syntax error: an extra closing brace in onKeyDown (results in "}}}" instead of "}}").
- For a11y, add role="button" (or switch to a ) and prevent default on Space to avoid page scroll. This also satisfies the linter warning about non-interactive elements with tabIndex.
Apply:
- <div
- tabIndex={0}
- onKeyDown={(e) => {
- if (e.key === "Enter" || e.key === " ") {
- setShowPopUp(true);
- }}}
- className={"cardStyle"}
- onClick={() => {
- setShowPopUp(true);
- }}
- >
+ <div
+ role="button"
+ tabIndex={0}
+ className={"cardStyle"}
+ onClick={() => setShowPopUp(true)}
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ e.preventDefault();
+ setShowPopUp(true);
+ }
+ }}
+ aria-label={t("HCM_CREATE_NEW_CAMPAIGN")}
+ >📝 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.
| tabIndex={0} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| setShowPopUp(true); | |
| }}} | |
| className={"cardStyle"} | |
| <div | |
| role="button" | |
| tabIndex={0} | |
| className={"cardStyle"} | |
| onClick={() => setShowPopUp(true)} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); | |
| setShowPopUp(true); | |
| } | |
| }} | |
| aria-label={t("HCM_CREATE_NEW_CAMPAIGN")} | |
| > |
🧰 Tools
🪛 Biome (2.1.2)
[error] 140-140: The HTML element div is non-interactive. Do not use tabIndex.
Adding non-interactive elements to the keyboard navigation flow can confuse users.
Unsafe fix: Remove the tabIndex attribute.
(lint/a11y/noNoninteractiveTabindex)
🤖 Prompt for AI Agents
In
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/NewCampaignCreate/CampaignHome.js
around lines 140 to 145, fix the extra closing brace in the onKeyDown handler
and make the card accessible by adding role="button" (or changing the element to
a <button>); update the onKeyDown so it handles Enter and Space, calling
e.preventDefault() for the Space key to avoid page scroll and then
setShowPopUp(true), and ensure the element still has tabIndex={0} and an onClick
handler for mouse activation.
…ts/DIGIT-Frontend into console-accessibility
Choose the appropriate template for your PR:
Feature/Bugfix Request
JIRA ID
Module
Description
Summary by CodeRabbit
Accessibility
Bug Fixes
Chores