fix(a11y): fix invalid ARIA, unnamed buttons, list markup, and touch targets on flow page - #14210
fix(a11y): fix invalid ARIA, unnamed buttons, list markup, and touch targets on flow page#14210olayinkaadelakun wants to merge 2 commits into
Conversation
…targets on flow page
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughFrontend controls now expose accessible names, use semantic buttons, and include localized labels. Generic node and sidebar structures receive accessibility-focused tests, while locale files add the required translations. ChangesFrontend accessibility updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/sidebarDraggableComponent.tsx`:
- Around line 210-212: Add regression coverage for the add-component button
rendered by the sidebarDraggableComponent, using a representative display_name
and the localized sidebar.addComponentToCanvas interpolation. Render the
component with the required test setup, then query the button by its localized
accessible name and assert it is present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1fc1fb55-b1a7-4bbd-9680-4eb96e94955b
📒 Files selected for processing (20)
src/frontend/src/CustomNodes/GenericNode/components/NodeOutputfield/__tests__/InspectButton.a11y.test.tsxsrc/frontend/src/CustomNodes/GenericNode/components/NodeOutputfield/index.tsxsrc/frontend/src/CustomNodes/GenericNode/components/NodeStatus/__tests__/NodeStatus.a11y.test.tsxsrc/frontend/src/CustomNodes/GenericNode/components/NodeStatus/index.tsxsrc/frontend/src/components/core/parameterRenderComponent/components/queryComponent/__tests__/queryComponent.a11y.test.tsxsrc/frontend/src/components/core/parameterRenderComponent/components/queryComponent/index.tsxsrc/frontend/src/components/core/parameterRenderComponent/components/textAreaComponent/__tests__/textAreaComponent.a11y.test.tsxsrc/frontend/src/components/core/parameterRenderComponent/components/textAreaComponent/index.tsxsrc/frontend/src/locales/de.jsonsrc/frontend/src/locales/en.jsonsrc/frontend/src/locales/es.jsonsrc/frontend/src/locales/fr.jsonsrc/frontend/src/locales/ja.jsonsrc/frontend/src/locales/pt.jsonsrc/frontend/src/locales/zh-Hans.jsonsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/bundleItems.a11y.test.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/categoryDisclouse.a11y.test.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/bundleItems.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/categoryDisclouse.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/sidebarDraggableComponent.tsx
| aria-label={t("sidebar.addComponentToCanvas", { | ||
| name: display_name, | ||
| })} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add regression coverage for the localized add-button name.
No supplied test renders this component and verifies sidebar.addComponentToCanvas interpolation. Add a test that queries the button by its localized accessible name using a representative display_name.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/sidebarDraggableComponent.tsx`
around lines 210 - 212, Add regression coverage for the add-component button
rendered by the sidebarDraggableComponent, using a representative display_name
and the localized sidebar.addComponentToCanvas interpolation. Render the
component with the required test setup, then query the button by its localized
accessible name and assert it is present.
Source: Coding guidelines
Summary
aria-allowed-attr:textAreaComponent/queryComponent's expand-to-editor icon rendered Radix's clonedaria-haspopup/aria-expandedon a bare<div>with no role. Fixed by making the icon's own tightly-sized wrapper a real<Button unstyled>, withrenderIcon()/renderGradient()split apart so only the icon sits inside the interactive element.button-name:NodeStatus's run/connect buttons,NodeOutputfield'sInspectButton, and the sidebar's "add to canvas" hover button (sidebarDraggableComponent) had no accessible name. Addedaria-labels using each button's already-computed tooltip text (or a new i18n key where none existed).list/listitem:categoryDisclouse.tsx/bundleItems.tsxnested<Disclosure><SidebarMenuItem>— puttingDisclosure's wrapper<div>between the sidebar's<ul>and each<li>. Swapped to<SidebarMenuItem><Disclosure>— same content, valid list structure.target-size: the new expand-editor button was 16×16px (below the 24×24 WCAG 2.5.8 minimum). Padded the hit area with abefore:pseudo-element rather than resizing the visible icon.Also renamed the misleading
input.editTextTitle("Edit Text") to a newinput.expandTextEditor("Expand text editor") key — the control expands into a larger editor, it doesn't perform the edit itself — and fixed two stray debugconsole.logs.Why
These are shared-component bugs — every node's run/inspect buttons, every text/query field, every sidebar row and add-button inherited the same failure. Fixing the shared components resolves the audits app-wide
Test coverage added
New jest-axe suites for
NodeStatus,InspectButton(newly exported fromNodeOutputfield),textAreaComponent,queryComponent, and real-DOM (unmockedSidebarMenu/Disclosure) list-structure tests forcategoryDisclouse/bundleItems— 27 suites, 503 tests passing.Translations
New keys (
input.expandTextEditor,sidebar.addComponentToCanvas) pushed through the GP pipeline; all 6 locales updated with real translations as isolated, single-purpose diffs.Test plan
Automated:
tsc/biomeclean on all touched files (pre-existing, unrelated warnings elsewhere untouched)Manual QA:
NodeStatus) — Tab to each, confirm VoiceOver/NVDA announces "Run component"/"Stop build" and "Connect" (or customauth_tooltip); click still builds/connects as beforeNodeOutputfield) — Tab to it in all 3 states (not built / has output / can't display), confirm the announced name matches the visible tooltip in each; click still opens the inspection paneltabIndex={-1})<li>is a direct child of its<ul>(no<div>in between)Summary by CodeRabbit
Accessibility
Localization
Tests