fix(a11y): name command palette, dialogs, and crash screen (LE-2043) - #14313
Merged
Conversation
Pre-existing gaps documented by the round-2 FINDING tests. cmdk hardcodes aria-labelledby and role="separator" after spreading caller props, so the first two needed the primitive reworked rather than a prop. - CommandInput drops cmdk's aria-labelledby when given aria-label, so the caller's name wins instead of resolving to an empty label element - CommandSeparator renders as presentational; separator is not an allowed child of CommandList's listbox - CommandDialog requires a label, naming the dialog and the search input - popoverObject names its Command, the last unnamed combobox call site - EditFlowSettings hides the lock switch when no setLocked is passed, so shareModal stops shipping a focusable inert control - BaseModal ariaLabel now applies to the dialog paths; templatesModal uses it and announces as "Templates" instead of "Dialog" - Crash screen: h1 title, link-wrapped report button, role="alert" Findings 3 and 8 are covered by #14250 and #14110 and left to those PRs.
Contributor
|
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:
WalkthroughAccessibility semantics and coverage were updated across command controls, crash handling, flow settings, and modal dialogs. Changes include explicit accessible names, presentational separators, conditional lock controls, localized labels, and expanded axe and interaction tests. ChangesAccessibility updates
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (8 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
Collaborator
Author
QA plan — LE-2043 (PR #14313)Accessibility fixes across three surfaces. Every case below is a manual check; automated coverage (jest-axe + IBM Equal Access) already runs in CI on this PR. Environment / tooling
Regression risk is concentrated in dropdowns/comboboxes (a shared primitive changed) and the Share modal (a control was removed). Give those the most time. 1. Dropdown search field has a name (finding 1)Where: any component parameter rendered as a dropdown — e.g. a model provider select, or any node input with a list of options. Two different search boxes exist behind these dropdowns and both were fixed; check both.
Pass criteria: every dropdown search box announces a name. Check at least 3 different dropdown types (a standard component parameter dropdown, a model/provider select, and an object/key-value picker). 2. Divider inside dropdown lists (finding 2)Where: dropdowns that show an extra action row at the bottom — e.g. a "+ Add …" / connect-new-provider row under the options.
This is the highest-regression-risk item — the divider was reimplemented. Visual parity matters as much as the a11y result. 3. Share modal has no dead lock switch (finding 4)Where: Share a flow, and share a component (right-click a component → Share).
4. Templates modal announces its name (finding 5)
Regression sweep (no behaviour should change)
Explicitly out of scope for this PR
Sign-offShip when: cases 1–4 pass with a screen reader, the regression sweep shows no behaviour change, and no new scanner violations appear on the dropdown, Share modal or Templates modal. |
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/frontend/src/modals/baseModal/index.tsx (1)
334-382: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the newly changed content branches.
The current test exercises the default
type="dialog"/DialogContentpath. Add focused accessibility assertions fortype="modal"anddialogContentWithouFixedso each newariaLabelforwarding path is verified.🤖 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/modals/baseModal/index.tsx` around lines 334 - 382, Add focused accessibility tests for the BaseModal branches using type="modal" and dialogContentWithouFixed. Assert that each branch forwards its provided ariaLabel to the rendered dialog content, while preserving the existing default type="dialog" coverage.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/components/core/parameterRenderComponent/components/inputComponent/components/popoverObject/index.tsx`:
- Line 127: Add the input.searchOptions translation key to every shipped locale
file under src/frontend/src/locales/, providing the appropriate localized
search-options label while preserving the existing locale structure and key
naming.
---
Nitpick comments:
In `@src/frontend/src/modals/baseModal/index.tsx`:
- Around line 334-382: Add focused accessibility tests for the BaseModal
branches using type="modal" and dialogContentWithouFixed. Assert that each
branch forwards its provided ariaLabel to the rendered dialog content, while
preserving the existing default type="dialog" coverage.
🪄 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: 3a4604dc-8706-4bd9-b7f4-0513b9bcc6d3
📒 Files selected for processing (10)
src/frontend/src/components/common/crashErrorComponent/__tests__/crashErrorComponent.a11y.test.tsxsrc/frontend/src/components/common/crashErrorComponent/index.tsxsrc/frontend/src/components/core/editFlowSettingsComponent/index.tsxsrc/frontend/src/components/core/parameterRenderComponent/components/inputComponent/components/popoverObject/index.tsxsrc/frontend/src/components/ui/__tests__/command.a11y.test.tsxsrc/frontend/src/components/ui/command.tsxsrc/frontend/src/modals/baseModal/index.tsxsrc/frontend/src/modals/shareModal/__tests__/shareModal.a11y.test.tsxsrc/frontend/src/modals/templatesModal/__tests__/templatesModal.a11y.test.tsxsrc/frontend/src/modals/templatesModal/index.tsx
| align="center" | ||
| > | ||
| <Command | ||
| label={optionsPlaceholder || t("input.searchOptions")} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -e json . -p 'locales' src/frontend/src | xargs -I{} sh -c 'echo === {} ===; jq ".input.searchOptions" {} 2>/dev/null || grep -n "searchOptions" {}'Repository: langflow-ai/langflow
Length of output: 539
Add input.searchOptions to all shipped locale files.
The locale files under src/frontend/src/locales/ do not define this key, so non-English users will get a raw key or blank label instead of the translated search-options label.
🤖 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/inputComponent/components/popoverObject/index.tsx`
at line 127, Add the input.searchOptions translation key to every shipped locale
file under src/frontend/src/locales/, providing the appropriate localized
search-options label while preserving the existing locale structure and key
naming.
Review follow-ups. BaseModal suppressed the DialogTitle when ariaLabel was set but rendered nothing in its place, so Radix logged "DialogContent requires a DialogTitle" on every render of templatesModal and of the canvas playground modal, which already passed ariaLabel with no Header. Render a VisuallyHidden DialogTitle from ariaLabel instead; full-screen keeps the attribute since it is a plain div, not a Radix dialog. dropdownComponent's search box is a bare input that had only a placeholder, so the most common dropdown in the product still shipped an unnamed search field.
github-actions
Bot
added
bug
Contributor
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-1.12.0 #14313 +/- ##
==================================================
+ Coverage 61.63% 62.53% +0.90%
==================================================
Files 2416 2364 -52
Lines 242162 239176 -2986
Branches 36186 35828 -358
==================================================
+ Hits 149249 149562 +313
+ Misses 90995 87794 -3201
+ Partials 1918 1820 -98
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
keval718
approved these changes
Jul 29, 2026
github-actions
Bot
added
bug
github-actions
Bot
added
bug
viktoravelino
enabled auto-merge
August 5, 2026 12:42
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 5, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 5, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 5, 2026
viktoravelino
enabled auto-merge
August 5, 2026 17:59
github-actions
Bot
added
bug
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the pre-existing accessibility bugs that the round-2 test coverage (#14302) documented with
FINDING (documented, not fixed)comments. Each fix flips its documenting assertion, so the tests now pin the correct behaviour instead of the bug.Ticket: LE-2043 — parent epic LE-1505.
What changed
components/ui/command.tsxcmdk 1.1.1 hardcodes
aria-labelledbyon the input androle="separator"on the separator after spreading caller props, so neither could be fixed by passing a prop — the primitives needed reworking.CommandInputdrops cmdk'saria-labelledbywhen the caller passesaria-label. Previously that reference always won the accessible-name computation, and when the wrapping<Command>had nolabelit resolved to an empty element, leaving the combobox with no accessible name at all (WCAG 4.1.2). With noaria-label, cmdk's<Command label>naming is untouched.CommandSeparatorrenders as a presentational divider instead ofrole="separator", which is not an allowed child ofCommandList'slistbox(axearia-required-children). It keeps cmdk's hide-while-searching behaviour viauseCommandState.CommandDialognow takes a requiredlabel, which names both the dialog and the search input. Without it the palette announced as the literal string "Dialog". Required so the gap cannot silently return; there are no existing call sites.popoverObject— the oneCommandInputcall site with no name at all now labels its<Command>, mirroring the siblingpopover. No new i18n keys anywhere in this PR.editFlowSettingsComponent— the Lock Flow row only renders whensetLockedis passed. ShareModal passes none, so it was shipping a focusable, unnamed, inert switch. ExportModal and FlowSettings both pass it and are unaffected.baseModal+templatesModal—ariaLabelnow applies to the dialog/modal paths, not justtype="full-screen", and suppresses the fallback title when set. TemplatesModal renders noBaseModal.Header, so it had no meaningful name; it now announces as "Templates" (WCAG 2.4.6 / 4.1.2).crashErrorComponent— title is an<h1>(the page had zero headings), the report action is a single link instead of a<button>nested in an<a href>(nested interactive, WCAG 4.1.2), and the card carriesrole="alert"so the failure is announced.role="alert"on<main>failsaria-allowed-role, hence the card.Out of scope — covered by other PRs
Two of the eight findings on the ticket are already fixed in open PRs and are deliberately untouched here:
aria-label. TheflowSettingsModalFINDING test still asserts the unnamed switch and will trip when that PR merges; flipping it belongs there.Testing
command.a11y.test.tsx,shareModal.a11y.test.tsxandtemplatesModal.a11y.test.tsxcrashErrorComponent.a11y.test.tsx— this finding came from manual review and had no covering testnpx jest src/modals src/components/ui src/components/common src/components/core/dropdownComponent src/components/core/parameterRenderComponent→ 114 suites, 1141 tests greenConflict watch
templatesModal/index.tsxcomponents/ui/dialog.tsx— fix(i18n): improve zh-Hans localization coverage #13523 replaces the literal"Dialog"fallback witht("common.dialog"), which would break the templatesModal assertion independently of this PRSummary by CodeRabbit
Accessibility Improvements
Bug Fixes