fix(a11y): resolve ARIA violations in MCP selector and sidebar settings, add axe coverage - #14250
fix(a11y): resolve ARIA violations in MCP selector and sidebar settings, add axe coverage#14250olayinkaadelakun wants to merge 11 commits into
Conversation
…targets on flow page
…gs, add axe coverage
|
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds accessibility improvements across the Flow UI: ShadTooltip supports an ChangesFrontend accessibility updates
Estimated code review effort: 3 (Moderate) | ~30 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 5❌ Failed checks (1 warning, 4 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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! 🎉
|
github-actions
Bot
added
bug
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-1.12.0 #14250 +/- ##
==================================================
- Coverage 61.30% 61.29% -0.02%
==================================================
Files 2342 2342
Lines 237114 236781 -333
Branches 33340 33280 -60
==================================================
- Hits 145354 145124 -230
+ Misses 89963 89860 -103
Partials 1797 1797
🚀 New features to boost your workflow:
|
github-actions
Bot
added
bug
github-actions
Bot
added
bug
github-actions
Bot
added
bug
…e button, refresh flowSettingsModal a11y tests
github-actions
Bot
added
bug
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/frontend/src/components/core/canvasControlsComponent/__tests__/Dropdowns.test.tsx (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAxe check validates the mocked ARIA roles, not real Radix accessibility output.
@/components/ui/dropdown-menuis fully mocked here with hand-authoredrole="menu"/role="menuitem"/role="menuitemcheckbox"attributes, then the newshould_have_no_axe_violationstest runs axe againstHelpDropdownbuilt on this mock. This mostly proves the mock's own roles are internally consistent rather than exercising the real RadixDropdownMenuDOM/ARIA output that ships to users —DropdownControlButton.test.tsxandCanvasControlsDropdown.test.tsxmount the real Radix primitives for their equivalent axe checks, which gives genuine confidence. As per coding guidelines, "prefer integration tests when unit tests are overly mocked" — consider dropping the dropdown-menu mock (or scoping the axe test to real primitives) so this check reflects actual runtime accessibility.Also applies to: 24-52, 116-124
🤖 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/components/core/canvasControlsComponent/__tests__/Dropdowns.test.tsx` at line 3, Update the axe accessibility test for HelpDropdown to exercise the real Radix dropdown-menu primitives instead of the hand-authored mock roles. Remove or scope the `@/components/ui/dropdown-menu` mock while preserving the existing HelpDropdown behavior and assertions, matching the real-primitive approach used by DropdownControlButton.test.tsx and CanvasControlsDropdown.test.tsx.Source: Coding guidelines
src/frontend/src/components/core/parameterRenderComponent/components/mcpComponent/__tests__/McpComponent.test.tsx (1)
148-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
rerenderover a second unmountedrendercall.This test renders
McpComponenttwice without unmounting the first instance, leaving two live DOM trees;getAllByTestId(...)[1]then relies on insertion order to pick out the second instance's button. Using thererenderfunction returned by the firstrender()call would swap props on the same instance and make the "distinctly named in each state" intent explicit, without depending on cross-tree DOM ordering.♻️ Proposed refactor
- render(<McpComponent {...defaultProps()} />); - expect(screen.getByTestId("mcp-server-dropdown")).toHaveAccessibleName( - "broken-server", - ); - - render( - <McpComponent - {...defaultProps()} - value={{ name: "", config: { command: "test" } }} - />, - ); - expect( - screen.getAllByTestId("mcp-server-dropdown")[1], - ).toHaveAccessibleName("Clear selected server"); + const { rerender } = render(<McpComponent {...defaultProps()} />); + expect(screen.getByTestId("mcp-server-dropdown")).toHaveAccessibleName( + "broken-server", + ); + + rerender( + <McpComponent + {...defaultProps()} + value={{ name: "", config: { command: "test" } }} + />, + ); + expect(screen.getByTestId("mcp-server-dropdown")).toHaveAccessibleName( + "Clear selected server", + );🤖 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/components/core/parameterRenderComponent/components/mcpComponent/__tests__/McpComponent.test.tsx` around lines 148 - 163, Update the test using the render result’s rerender function instead of calling render a second time. Reuse the same mcp-server-dropdown query after changing props so assertions target the single McpComponent instance and no longer depend on getAllByTestId insertion order.src/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/sidebarDraggableComponent.test.tsx (1)
194-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAxe test is validated mostly against hand-authored mocks, not real components.
ShadTooltip,Button, and the entireselect-custommodule (Select,SelectTrigger,SelectContent,SelectItem) are all replaced by simplified stand-ins here — theSelectTriggermock even hardcodesaria-expanded={false}regardless of actual state. The newshould_have_no_axe_violationstest (Lines 271-279) therefore asserts mostly against markup this test file itself authored, not the real Radix/Button/ShadTooltip output, so it offers limited protection against real a11y regressions in those primitives.Compare with
sidebarHeader.a11y.test.tsxin this same PR, which deliberately keepsButton/ShadTooltip/sidebar primitives unmocked specifically to exercise real ARIA output, while only mocking child sections that have their own dedicated a11y coverage. Consider a similar unmocked a11y suite here (or narrowing which pieces are mocked) for a more meaningful signal.As per coding guidelines: "Warn when frontend test files rely on excessive mocks that obscure what is actually being tested, replace mocks with real objects or test doubles when mocks become excessive, and prefer integration tests when unit tests are overly mocked."
Also applies to: 271-279
🤖 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/__tests__/sidebarDraggableComponent.test.tsx` around lines 194 - 203, The accessibility test relies on mocked ShadTooltip, Button, and select-custom primitives, so it validates authored stand-ins rather than real ARIA behavior. Update the suite around should_have_no_axe_violations to remove those primitive mocks, or narrow mocking to child sections with dedicated coverage, while retaining real Select, SelectTrigger, SelectContent, SelectItem, Button, and ShadTooltip implementations. Ensure the SelectTrigger uses its actual aria-expanded state.Source: Coding guidelines
🤖 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 107-114: Update handleKeyDown and the role="button" element in
sidebarDraggableComponent so disabled items cannot be activated by keyboard:
guard addComponent with the same disabled check used by onDoubleClick, and
remove disabled items from the tab order while keeping enabled items focusable.
---
Nitpick comments:
In
`@src/frontend/src/components/core/canvasControlsComponent/__tests__/Dropdowns.test.tsx`:
- Line 3: Update the axe accessibility test for HelpDropdown to exercise the
real Radix dropdown-menu primitives instead of the hand-authored mock roles.
Remove or scope the `@/components/ui/dropdown-menu` mock while preserving the
existing HelpDropdown behavior and assertions, matching the real-primitive
approach used by DropdownControlButton.test.tsx and
CanvasControlsDropdown.test.tsx.
In
`@src/frontend/src/components/core/parameterRenderComponent/components/mcpComponent/__tests__/McpComponent.test.tsx`:
- Around line 148-163: Update the test using the render result’s rerender
function instead of calling render a second time. Reuse the same
mcp-server-dropdown query after changing props so assertions target the single
McpComponent instance and no longer depend on getAllByTestId insertion order.
In
`@src/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/sidebarDraggableComponent.test.tsx`:
- Around line 194-203: The accessibility test relies on mocked ShadTooltip,
Button, and select-custom primitives, so it validates authored stand-ins rather
than real ARIA behavior. Update the suite around should_have_no_axe_violations
to remove those primitive mocks, or narrow mocking to child sections with
dedicated coverage, while retaining real Select, SelectTrigger, SelectContent,
SelectItem, Button, and ShadTooltip implementations. Ensure the SelectTrigger
uses its actual aria-expanded state.
🪄 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 Plus
Run ID: b3a8846e-5636-4cd0-8135-ea18f1445ddf
📒 Files selected for processing (47)
src/frontend/src/components/common/shadTooltipComponent/__tests__/shadTooltip.a11y.test.tsxsrc/frontend/src/components/common/shadTooltipComponent/__tests__/shadTooltip.test.tsxsrc/frontend/src/components/common/shadTooltipComponent/index.tsxsrc/frontend/src/components/core/appHeaderComponent/__tests__/appHeader.a11y.test.tsxsrc/frontend/src/components/core/appHeaderComponent/__tests__/appHeader.notificationDropdown.test.tsxsrc/frontend/src/components/core/appHeaderComponent/components/__tests__/langflow-counts.test.tsxsrc/frontend/src/components/core/appHeaderComponent/components/langflow-counts.tsxsrc/frontend/src/components/core/appHeaderComponent/index.tsxsrc/frontend/src/components/core/canvasControlsComponent/CanvasControlButton.tsxsrc/frontend/src/components/core/canvasControlsComponent/CanvasControlsDropdown.tsxsrc/frontend/src/components/core/canvasControlsComponent/DropdownControlButton.tsxsrc/frontend/src/components/core/canvasControlsComponent/HelpDropdownView.tsxsrc/frontend/src/components/core/canvasControlsComponent/__tests__/CanvasControlButton.test.tsxsrc/frontend/src/components/core/canvasControlsComponent/__tests__/CanvasControlsDropdown.test.tsxsrc/frontend/src/components/core/canvasControlsComponent/__tests__/DropdownControlButton.test.tsxsrc/frontend/src/components/core/canvasControlsComponent/__tests__/Dropdowns.test.tsxsrc/frontend/src/components/core/editFlowSettingsComponent/__tests__/editFlowSettingsComponent.test.tsxsrc/frontend/src/components/core/editFlowSettingsComponent/index.tsxsrc/frontend/src/components/core/parameterRenderComponent/components/mcpComponent/__tests__/McpComponent.test.tsxsrc/frontend/src/components/core/parameterRenderComponent/components/mcpComponent/index.tsxsrc/frontend/src/icons/freezeAll/freezeAll.jsxsrc/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/InspectionPanel/__tests__/InspectionPanelParameterRow.test.tsxsrc/frontend/src/pages/FlowPage/components/InspectionPanel/components/InspectionPanelParameterRow.tsxsrc/frontend/src/pages/FlowPage/components/PageComponent/index.tsxsrc/frontend/src/pages/FlowPage/components/PageComponent/utils/__tests__/get-node-aria-label.test.tssrc/frontend/src/pages/FlowPage/components/PageComponent/utils/get-node-aria-label.tssrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/searchConfigTrigger.a11y.test.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/searchInput.test.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/sidebarDraggableComponent.test.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/sidebarHeader.a11y.test.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/sidebarHeader.test.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/sidebarSegmentedNav.a11y.test.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/searchConfigTrigger.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/searchInput.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/sidebarDraggableComponent.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/sidebarHeader.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/sidebarSegmentedNav.tsxsrc/frontend/src/pages/FlowPage/components/nodeToolbarComponent/components/__tests__/toolbar-button.test.tsxsrc/frontend/src/pages/FlowPage/components/nodeToolbarComponent/components/toolbar-button.tsxsrc/frontend/src/types/components/index.ts
💤 Files with no reviewable changes (2)
- src/frontend/src/icons/freezeAll/freezeAll.jsx
- src/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/tests/sidebarHeader.test.tsx
| const handleKeyDown = (e) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| addComponent(apiClass, itemName); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Keyboard Enter/Space bypasses the disabled guard that onDoubleClick enforces.
handleKeyDown calls addComponent(apiClass, itemName) unconditionally on Enter/Space, but the sibling onDoubleClick handler explicitly checks if (!disabled) before doing the same. Since the role="button" div keeps tabIndex={0} even when disabled is true, a keyboard user can still Tab to it and press Enter/Space to add a component that is supposed to be blocked (mouse users are blocked via pointer-events-none on the wrapper, but that CSS property has no effect on keyboard-dispatched events).
🐛 Proposed fix
const handleKeyDown = (e) => {
- if (e.key === "Enter" || e.key === " ") {
+ if (!disabled && (e.key === "Enter" || e.key === " ")) {
e.preventDefault();
e.stopPropagation();
addComponent(apiClass, itemName);
}
};and, for defense in depth, remove the element from the tab order too:
- tabIndex={0}
+ tabIndex={disabled ? -1 : 0}Also applies to: 143-152
🤖 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 107 - 114, Update handleKeyDown and the role="button" element in
sidebarDraggableComponent so disabled items cannot be activated by keyboard:
guard addComponent with the same disabled check used by onDoubleClick, and
remove disabled items from the tab order while keeping enabled items focusable.
2 participants
Summary
Accessibility (a11y) pass across the sidebar, canvas controls, app header, MCP component, node toolbar, and inspection panel. Introduces a reusable mechanism for suppressing redundant tooltip descriptions, fixes several nested-interactive-control and missing-ARIA-attribute defects, adds explicit accessible names where icon-only controls had none, and backs all of it with axe-core test coverage. Also fills in translation gaps left by the a11y work.
What changed
Core mechanism:
ShadTooltipariaDescribedBy overrideshadTooltipComponent/index.tsxandtypes/components/index.tsadd a new optionalariaDescribedByprop. When a caller passes it explicitly (includingundefined), it overrides Radix's automaticaria-describedbywiring on the tooltip trigger; when the prop is omitted entirely, Radix's default behavior is untouched. This is used at ~10 call sites in this PR to passariaDescribedBy={undefined}specifically where the tooltip's text just repeats the trigger's ownaria-label, which previously caused screen readers to announce the same label twice. Call sites:CanvasControlButton,AppHeader(notification bell),LangflowCounts(GitHub/Discord buttons),McpComponent(refresh button),SearchConfigTrigger,SidebarHeaderComponent,SidebarSegmentedNav,ToolbarButton.Nested-interactive-control fixes
DropdownControlButton.tsx: previously rendered a plainButtonwith aToggleShadComponentswitch nested inside it for toggle items — two independently interactive elements in one row. Replaced with Radix's realDropdownMenuItem/DropdownMenuCheckboxItem, giving toggle items the correctrole="menuitemcheckbox"pattern instead of nesting a switch inside a menu item.CanvasControlButton.tsx:ShadTooltipwas wrapping only the icondivinside the button, not the button itself, so the tooltip trigger wasn't attached to the actual interactive element. Restructured soShadTooltipwraps the wholeControlButton.sidebarHeader.tsx:DisclosureTriggerwas cloningrole="button"/tabIndex=0/onClickonto a wrapper<div>, while the real settingsButtonsat nested inside it (axe:nested-interactive). Removed the wrapper; the toggle behavior is now wired directly onto the real button, matching the pattern insearchConfigTrigger.tsx.sidebarDraggableComponent.tsx:role="button"/aria-label/tabIndex/onKeyDownwere on the outer row wrapper, which also contained a real, separately-clickable "Add" button and an options menu trigger nested inside it. Moved the button role onto just the inner draggable-name element and made the Add button/options menu siblings instead of descendants; compensated thegroup-focus→group-focus-withinCSS so the Add button's hover/focus affordance still works.Missing/incorrect ARIA fixes
mcpComponent/index.tsx: server-select button hadrole="combobox"without requiredaria-expanded/aria-controls, and no reliable accessible name (it opens a modal dialog, not a listbox, socomboboxwas the wrong role to begin with). Replaced witharia-haspopup="dialog"+aria-expanded+ explicitaria-labelfor both the "select server" and "clear server" (mcp.clearServer, new key) states.InspectionPanelParameterRow.tsx: "Add" button usedtext-muted-foreground, which fails contrast; changed totext-foreground.freezeAll.jsx: removed a redundant<title>snowflake-svg</title>inside the SVG — the icon is decorative and always paired with a visible label/tooltip from its parent (ToolbarButton/ToolbarSelectItem), so the title was dead/conflicting markup, not the icon's only name source.New explicit accessible names
CanvasControlsDropdown.tsx: zoom trigger now hasaria-labelannouncing the control and current zoom percentage (canvas.zoomControlAriaLabel).HelpDropdownView.tsx: help button gets an explicitaria-label(previously relied ontitleonly).editFlowSettingsComponent/index.tsx: lock-flow switch getsaria-label(flow.lockFlowAriaLabel).searchInput.tsx: sidebar search input getsaria-label(sidebar.searchAriaLabel), since its placeholder text alone isn't a reliable accessible name.PageComponent/index.tsx+ newutils/get-node-aria-label.ts: nodearia-labelcomputation extracted to a pure, unit-tested function, and note nodes now get a fixed, non-blank label (noteNode.ariaLabel) instead of falling through to an emptydisplay_name.Translations
Filled in five a11y-related keys that existed only in
en.json:sidebar.searchAriaLabel,canvas.zoomControlAriaLabel,flow.lockFlowAriaLabel,mcp.clearServer(missing from all 6 other locales), andnoteNode.ariaLabel(missing fromfr.json).Test coverage added
New or extended axe-core coverage for:
ShadTooltip,CanvasControlButton,CanvasControlsDropdown,DropdownControlButton,HelpDropdown,InspectionPanelParameterRow,McpComponent,LangflowCounts,AppHeader(+ dedicated unmocked notification-dropdown suite),SearchConfigTrigger,SidebarHeaderComponent,SidebarSegmentedNav,EditFlowSettings,ToolbarButton,SearchInput,SidebarDraggableComponent,getNodeAriaLabel. Several are new*.a11y.test.tsxfiles rendering real, unmocked components — existing suites mocked away the exactButton/ShadTooltip/Radix primitives whose attributes needed verifying.Also fixed a test-mock bug in
Dropdowns.test.tsx: theflowStoremock ignored the Zustand selector argument, producing a bogusaria-checked="[object Object]"that masked what axe should have caught. Fixed the mock and added correct ARIA roles (menu/menuitem/separator) to the mocked dropdown primitives.Known issues, not fixed (flagged for follow-up)
appHeaderComponent/index.tsx: a redundantAlertDropdownnested inside anotherAlertDropdownaround the notification bell. Confirmed via test to be dead code (the outerPopovertrigger never receives clicks —ShadTooltipsilently drops the clonedonClick), not a duplicate-render bug; axe passes on it. Recommend a small cleanup ticket.DropdownControlButton.tsx's move to a real RadixDropdownMenuItemmeans the canvas zoom dropdown now closes on every click of Zoom In/Out/Reset/Fit View (Radix auto-closes on item select by default). The toggle branch already prevents this (onSelectguard); the plain-item branch does not. Previously, as a plainButton, the menu likely stayed open across repeated clicks. Needs a product call on whether that's desired.sidebar.addComponentToCanvaswas removed fromen.jsonon this branch but still lingers, unused, in the other 6 locale files. Harmless, but worth cleaning up.QE / How to validate
Manual — tooltip double-announcement fix
Manual — MCP server selector
Manual — Sidebar settings toggle (legacy sidebar)
Manual — Sidebar draggable component row
Manual — Canvas zoom dropdown (known behavior change)
Manual — Translations
mcp.clearServer,flow.lockFlowAriaLabel) are ever visible or announced — everything should show translated text.Testing performed
tsc --noEmit: no new errors in touched filesSummary by CodeRabbit
Accessibility Improvements
Bug Fixes
Tests