fix(a11y): admin and auth pages accessibility - #13998
Merged
Merged
Conversation
- Wrap /login/admin and /admin content in a <main> landmark - Add accessible names to admin controls: search, clear, active/superuser toggles, row edit/delete, back button - Fix New User modal label associations (explicit htmlFor on checkbox fields) - Bind /login/admin username/password inputs so the placeholder overlay clears once text is entered - Constrain the active/superuser toggle focus ring to the checkbox instead of spanning the full cell - Make the clear-search control a real focusable button (was a div onClick) - Add /admin and /login/admin a11y specs, IBM baselines for remaining framework-level debt, and a clear-search keyboard test - Add i18n keys admin.backButton and admin.clearSearch across all locales - Move baselineFolder to tests/a11y/baselines
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds accessibility improvements to the admin area: aria-label support on checkboxes and admin page controls, semantic markup changes (div-to-main, div-to-button), controlled login inputs, htmlFor label associations, new locale strings, accessibility Playwright tests, corresponding a11y baseline JSON files, an achecker config path update, and regenerated secrets baseline entries. ChangesAdmin Page Accessibility Enhancements
Estimated code review effort: 2 (Simple) | ~15 minutes Accessibility Test Suite and Baselines
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested labels: 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 |
Contributor
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
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.
🧹 Nitpick comments (1)
src/frontend/tests/a11y/admin-page.a11y.spec.ts (1)
5-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract
disableAnimationsto a shared utility.This function is duplicated verbatim in
auth-pages.a11y.spec.ts(lines 16–30). Extract it to a shared module (e.g., alongside the existing../utils/helpers) and import from both specs to keep a11y test setup DRY.♻️ Suggested extraction
-async function disableAnimations(page: LangflowPage) { - await page.addStyleTag({ - content: ` - *, - *::before, - *::after { - animation-duration: 0s !important; - animation-delay: 0s !important; - transition-duration: 0s !important; - transition-delay: 0s !important; - scroll-behavior: auto !important; - } - `, - }); -} +// In a shared file, e.g. src/frontend/tests/a11y/utils.ts +export async function disableAnimations(page: LangflowPage) { + await page.addStyleTag({ + content: ` + *, + *::before, + *::after { + animation-duration: 0s !important; + animation-delay: 0s !important; + transition-duration: 0s !important; + transition-delay: 0s !important; + scroll-behavior: auto !important; + } + `, + }); +}Then in both spec files:
+import { disableAnimations } from "./utils";🤖 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/admin-page.a11y.spec.ts` around lines 5 - 19, The disableAnimations helper is duplicated across a11y specs, so extract it into a shared utility under the existing utils helpers and import it from both test files. Move the shared implementation of disableAnimations(page: LangflowPage) into the common module, then update admin-page.a11y.spec.ts and auth-pages.a11y.spec.ts to use that shared helper so the animation-disabling setup stays DRY and consistent.
🤖 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.
Nitpick comments:
In `@src/frontend/tests/a11y/admin-page.a11y.spec.ts`:
- Around line 5-19: The disableAnimations helper is duplicated across a11y
specs, so extract it into a shared utility under the existing utils helpers and
import it from both test files. Move the shared implementation of
disableAnimations(page: LangflowPage) into the common module, then update
admin-page.a11y.spec.ts and auth-pages.a11y.spec.ts to use that shared helper so
the animation-disabling setup stays DRY and consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7b39f236-7013-46df-b7c8-22e1eefcf4fe
📒 Files selected for processing (18)
.secrets.baselinesrc/frontend/.achecker.ymlsrc/frontend/src/components/ui/checkbox.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/modals/userManagementModal/index.tsxsrc/frontend/src/pages/AdminPage/LoginPage/index.tsxsrc/frontend/src/pages/AdminPage/index.tsxsrc/frontend/tests/a11y/admin-page.a11y.spec.tssrc/frontend/tests/a11y/auth-pages.a11y.spec.tssrc/frontend/tests/a11y/baselines/chromium__admin-new-user-modal.jsonsrc/frontend/tests/a11y/baselines/chromium__admin-page-dark.jsonsrc/frontend/tests/a11y/baselines/chromium__admin-page-empty.json
Contributor
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (30.76%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## release-1.12.0 #13998 +/- ##
=================================================
Coverage ? 60.36%
=================================================
Files ? 2344
Lines ? 230134
Branches ? 34796
=================================================
Hits ? 138910
Misses ? 89619
Partials ? 1605
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
github-actions
Bot
added
bug
…dd dark scans Resolve the a11y debt that was previously baselined for the /admin page and harden the auth surfaces: - Toggle controls (active/superuser): convert to proper toggle buttons (aria-label + aria-pressed threaded to the trigger) and render the inner CheckBoxDiv as presentational, so a checkbox role is no longer nested inside an interactive button (IBM aria_descendant_valid, WCAG 1.3.1/4.1.2). - Row action icons (edit/delete): size each trigger to >=24x24 CSS px so they meet WCAG 2.5.8 target size and no longer trip target_spacing_sufficient. - Delete all three admin IBM baselines now that the violations are fixed. - New User modal: the password/confirm fields hand-rolled a raw input with the visibility toggle stuffed into the label. Replace with the shared InputComponent password primitive (eye toggle inside the input, keyboard operable, aria-label + aria-pressed), matching the login/signup surfaces; move validation to the getRequiredFieldError pattern. - Dark-mode coverage: run every auth state in both light and dark and add a dark new-user-modal scan. This surfaced insufficient error-text contrast in dark mode (~3.9:1); fix .field-invalid/.label-invalid to use the brighter accent-red so form errors meet WCAG AA 4.5:1.
github-actions
Bot
added
bug
olayinkaadelakun
approved these changes
Jul 10, 2026
github-actions
Bot
added
bug
… fix/admin-auth-pages-a11y
InputComponent lost the name attribute that Radix Form.Control asChild previously injected, breaking Playwright selectors input[name=password] and input[name=confirmpassword] in the New User modal.
github-actions
Bot
added
bug
…uth-pages-a11y # Conflicts: # .secrets.baseline
github-actions
Bot
added
bug
github-actions
Bot
added
bug
github-actions
Bot
added
bug
github-actions
Bot
added
bug
github-actions
Bot
added
bug
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
4 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.
Summary
Brings the auth surfaces and the
/adminuser-management page to WCAG 2.1 A/AA, verified with the IBM Equal Access (accessibility-checker) engine plus keyboard/focus interaction tests. Same approach used for /settings/api-keys (LE-1720 / #13953) and /assets/files (#13987).Covers LE-1760 (
/login/admin) and LE-1761 (/admin).Fixes
/login,/signup— already clean; added regression coverage./login/admin<main>landmark (fixesskip_main_exists+aria_content_in_landmark)./admin<main>landmark.<button>(was a<div onClick>— WCAG 2.1.1) + keyboard test.New User modal
htmlForon theis_active/is_superusercheckboxes;label_ref_valid).<input>(which stuffed the visibility toggle into the label, mispositioning the eye icon and leaving it keyboard-inoperable) with the sharedInputComponent passwordprimitive used by login/signup — eye toggle now sits inside the input, is keyboard operable, and exposesaria-label+aria-pressed. Validation moved to thegetRequiredFieldErrorpattern.Previously-baselined debt — now fixed (baselines removed)
The three admin IBM baselines are deleted; the underlying violations are resolved:
aria_descendant_valid(active/superuser toggles): the toggles nested arole="checkbox"indicator inside the confirmation dialog-trigger<button>. Converted them to proper toggle buttons —aria-label+aria-pressedthreaded throughConfirmationModal.Trigger→BaseModal.Trigger→ the RadixDialogTriggerbutton — and made the innerCheckBoxDivpresentational (aria-hidden, no role) via a newpresentationalprop.target_spacing_sufficient(row edit/delete icons): each icon trigger is now wrapped to a ≥24×24 CSS-px target, satisfying WCAG 2.5.8 (≥24px targets are exempt from the spacing requirement).Dark-mode coverage + contrast fix
states × themes), and a dark-mode New User modal scan was added. Previously only one dark scan existed per surface..field-invalid/.label-invalid) was only ~3.9:1 contrast — below the WCAG AA 4.5:1 minimum. Fixed by pointing those classes at the brighter--accent-red-foregroundin dark mode (light mode unchanged; app-wide improvement for every dark-mode form error).i18n
Keys
admin.backButtonandadmin.clearSearchadded to all 7 locale files.Validation
Tickets