fix: settings/db-provider accessibility - #13999
Conversation
…Area and Button components - Added `useTranslation` for internationalization in `AlertDisplayArea`, providing an aria-label for better accessibility. - Updated `Button` component to include `aria-busy` attribute during loading and ensured the accessible name is preserved with a visually hidden label. - Enhanced accessibility tests for both components to verify the presence of roles and attributes.
|
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:
WalkthroughAdds explicit accessibility semantics for alert regions and loading buttons, updates DB provider “coming soon” styling, and introduces end-to-end accessibility coverage for provider configuration, saving, and connection-testing states. ChangesAccessibility semantics and DB provider coverage
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 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 |
|
✅ Migration Validation Passed All migrations follow the Expand-Contract pattern correctly. |
|
Build successful! ✅ |
✅ 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/tests/a11y/db-providers.a11y.spec.ts`:
- Around line 352-360: Update the loading-state comment to acknowledge that the
sr-only label preserves the button’s accessible name during loading, and change
the locator associated with useChromaButton to page.getByRole("button", { name:
"Use Chroma" }) so the test verifies that fix.
🪄 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: 3227ef5a-168f-46b2-a5ab-790274029194
📒 Files selected for processing (6)
src/frontend/src/alerts/displayArea/__tests__/displayArea.a11y.test.tsxsrc/frontend/src/alerts/displayArea/index.tsxsrc/frontend/src/components/ui/__tests__/button.a11y.test.tsxsrc/frontend/src/components/ui/button.tsxsrc/frontend/src/pages/SettingsPage/pages/DBProvidersPage/index.tsxsrc/frontend/tests/a11y/db-providers.a11y.spec.ts
| // Locate by text content rather than accessible role name: the | ||
| // Button component visually hides its label (CSS `invisible`) while | ||
| // `loading` is true, which strips the accessible name from the a11y | ||
| // tree — a real gap this scan is designed to surface — so a | ||
| // name-based role query would stop matching mid-flight. | ||
| await page.getByTestId("db-provider-item-chroma").click(); | ||
| const useChromaButton = page | ||
| .getByRole("button") | ||
| .filter({ hasText: "Use Chroma" }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update loading-state comment and query to reflect the sr-only accessible-name fix.
The comment states the Button's loading state "strips the accessible name from the a11y tree" and that "a name-based role query would stop matching mid-flight." However, this PR's button.tsx changes add an sr-only span that preserves the accessible name during loading. The comment is now inconsistent with the fix it ships alongside.
Consider using getByRole("button", { name: "Use Chroma" }) instead of getByRole("button").filter({ hasText: "Use Chroma" }) to actually verify the sr-only fix works. If the text-based query is intentionally kept for robustness, update the comment to reflect the post-fix rationale.
💚 Proposed fix
// Locate by text content rather than accessible role name: the
// Button component visually hides its label (CSS `invisible`) while
- // `loading` is true, which strips the accessible name from the a11y
- // tree — a real gap this scan is designed to surface — so a
- // name-based role query would stop matching mid-flight.
+ // `loading` is true. The sr-only span added in button.tsx preserves
+ // the accessible name, so a name-based role query remains stable
+ // throughout the loading state.
await page.getByTestId("db-provider-item-chroma").click();
const useChromaButton = page
- .getByRole("button")
- .filter({ hasText: "Use Chroma" });
+ .getByRole("button", { name: "Use Chroma" });
await expect(useChromaButton).toBeEnabled();📝 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.
| // Locate by text content rather than accessible role name: the | |
| // Button component visually hides its label (CSS `invisible`) while | |
| // `loading` is true, which strips the accessible name from the a11y | |
| // tree — a real gap this scan is designed to surface — so a | |
| // name-based role query would stop matching mid-flight. | |
| await page.getByTestId("db-provider-item-chroma").click(); | |
| const useChromaButton = page | |
| .getByRole("button") | |
| .filter({ hasText: "Use Chroma" }); | |
| // Locate by text content rather than accessible role name: the | |
| // Button component visually hides its label (CSS `invisible`) while | |
| // `loading` is true. The sr-only span added in button.tsx preserves | |
| // the accessible name, so a name-based role query remains stable | |
| // throughout the loading state. | |
| await page.getByTestId("db-provider-item-chroma").click(); | |
| const useChromaButton = page | |
| .getByRole("button", { name: "Use Chroma" }); |
🤖 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/tests/a11y/db-providers.a11y.spec.ts` around lines 352 - 360,
Update the loading-state comment to acknowledge that the sr-only label preserves
the button’s accessible name during loading, and change the locator associated
with useChromaButton to page.getByRole("button", { name: "Use Chroma" }) so the
test verifies that fix.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-1.12.0 #13999 +/- ##
==================================================
+ Coverage 61.30% 61.42% +0.12%
==================================================
Files 2342 2339 -3
Lines 237114 237131 +17
Branches 33340 35400 +2060
==================================================
+ Hits 145354 145657 +303
+ Misses 89963 89677 -286
Partials 1797 1797
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This pull request introduces a comprehensive accessibility (a11y) testing framework for the Langflow frontend, including new documentation, GitHub Actions workflows for both route-based and component-level accessibility scans, and supporting skill guides. It also adds an internationalization (i18n) review skill and updates E2E testing tag requirements. These changes ensure that user-facing text and UI components are accessible and properly internationalized, and that E2E tests are consistently tagged for release coverage.
Accessibility Testing Infrastructure
.github/workflows/a11y-scan.ymlto run IBM Equal Access accessibility scans on frontend routes using Playwright, with job summary and artifact upload. This workflow runs on PRs, on a nightly schedule, and via manual dispatch..github/workflows/a11y-unit-tests.ymlto runjest-axecomponent-level accessibility unit tests on frontend changes, blocking PRs on regressions..agents/skills/ibm-a11y-automation/SKILL.mdwith detailed instructions for running and interpreting the accessibility scanner, including route selection, state actions, and reporting.Internationalization (i18n) Review
.agents/skills/frontend-i18n/SKILL.mddescribing the i18n system, review checklist, and rules for user-facing text in the frontend, ensuring all UI strings go through the translation system and are present in all locale files..agents/skills/frontend-code-review/SKILL.mdto reference the new i18n review skill for any diffs that add or change user-facing text.E2E Testing Tag Policy
.agents/skills/e2e-testing/SKILL.mdto require every test to be tagged with@releaseand clarified allowed domain tags, ensuring tests are included in release coverage. [1] [2]CI Workflow and Filter Updates
.github/workflows/ci.ymlto only run Python tests when relevant files are changed, improving CI efficiency.mainpagefilter from.github/changes-filter.yaml.QA:
/settings/db-providersaccessibilityAutomated
Optional route scan:
Expect 0 violations on default load.
Manual checklist
Open Settings → DB Providers. Prefer Chrome + one other browser. Use keyboard-only + VoiceOver/NVDA for SR checks.
Keyboard
Visual
States to cover
••••••••)Pass / fail
Pass: no critical axe issues; full keyboard path works; loading buttons + toasts announced; coming soon contrast OK in both themes.
Fail: invisible/trapped focus; unnamed loading button; unannounced toasts; unreadable coming soon UI.
Summary by CodeRabbit
Accessibility
UI Improvements
Tests