Skip to content

♿ fix: Pass Every Axe Scan on the Grants and Access Pages - #203

Merged
danny-avila merged 1 commit into
mainfrom
pr/fu-a11y-axe-clean
Oct 28, 2026
Merged

danny-avila merged 1 commit into
mainfrom
pr/fu-a11y-axe-clean

Conversation

@danny-avila

Copy link
Copy Markdown
Contributor

Pull Request Template

⚠️ Before Submitting a PR, Please Review:

  • Please ensure that you have thoroughly read and understood the Contributing Docs before submitting your Pull Request.

⚠️ Documentation Updates Notice:

  • Kindly note that documentation updates are managed in this repository: librechat.ai

Summary

On main (click-ui 0.15, @axe-core/playwright 4.14), five axe scans in the mock e2e suite fail:

  • the Access page's Groups and Roles tabs (e2e/access.spec.ts);
  • the Grants page's management tab, edit dialog and audit log tab (e2e/grants.spec.ts).

This PR fixes each failure in the panel. It doesn't disable any rule, exclude any element or loosen any tag. With it, the full mock suite exits 0, and the same scans pass live in light and dark.

Color contrast (Active and Granted badges)

click-ui 0.15 still draws the success badge in #008a0b on its #ebffec tint. At the badge's 10px size that is 4.32:1, and 4.02:1 on a hovered row; AA needs 4.5:1. The unlayered :root[data-cui-theme] block in src/styles.css already maps the switch tokens to the panel's own tokens. It now sets the success badge text the same way:

--click-badge-opaque-color-text-success: var(--settings-success-fg);
Theme Before (row / hovered row) After (row / hovered row)
Light #008a0b: 4.32 / 4.02 #146c2e: 6.23 / 5.80
Dark #ccffd0: 8.84 / 7.91 #86efac: 7.04 / 6.30

Dark already passed. It switches only so that both themes use the panel's success green, the one StateChip uses. The other badge states the panel renders (default, danger, warning, neutral) pass in both themes. Info doesn't (3.42:1 in light), but no badge uses it.

Nested interactive controls

  • Capability category headers. Each header in the Edit capabilities dialog was a div role="button" that held its "Select all" switch. A new shared SelectAllHeader puts a disclosure <button> (chevron, label, optional description) next to the count and the switch instead. The button has aria-expanded and aria-controls.
    • Click, Enter and Space still open and close the category, and Tab still goes from the header to its switch. The switch still selects the whole category without opening or closing it.
    • The focus ring sits inside the card: .select-all-toggle shares .sidebar-toggle's inset ring rule.
    • The Edit role dialog's Permissions tab (RolePermissionsPanel) had the same header and now uses the same component. No e2e scan opens that dialog, but a live axe scan of it is clean.
  • Audit log date filters. click-ui's DatePicker puts its readonly input inside the trigger button, and the input stays focusable even with tabIndex = -1. The new DateFilter component replaces the old DatePickerCell patch:
    • It makes the input inert, so the input is still shown but can't take focus or clicks and leaves the accessibility tree.
    • It moves the label's for to the trigger button. The trigger used to be named "calendar". It's now "From", or "From Oct 01, 2026" once a date is picked (aria-labelledby = the label plus the input).
    • Clicking the label now opens the picker. Enter on the trigger still moves focus into the calendar.
    • click-ui exposes no props for either element, so an effect sets them, and sets them again after the Clear button remounts the picker.

Label in name (WCAG 2.5.3, new in axe 4.14)

axe's rule only checks elements that have aria-label or aria-labelledby. For those, it requires the visible text, including aria-hidden text that's still on screen, to appear as a run of words in the accessible name. Text in parentheses doesn't count.

  • Header search button. It was named "Command menu" but shows "Search" and "Ctrl+K". It no longer has an aria-label, so its name is its text, "Search Ctrl+K" (or "Search ⌘K" on a Mac). On phones only its icon shows, so the "Search" text becomes screen-reader-only there (sr-only sm:not-sr-only) and the button is still named "Search".
    • Its title ("Search (Ctrl+K)") is gone too. Once the name came from the text, the title only repeated it as the button's description, so a screen reader read "Search Ctrl+K, button, Search (Ctrl+K)".
  • Sidebar avatar. It shows the initials but was named "User menu, Test Admin". It's now named {{initials}}, user menu for {{name}}, e.g. "TA, user menu for Test Admin". The initials come first, as WCAG's guidance for 2.5.3 recommends. The new key com_nav_user_menu_label replaces com_nav_user_menu.
  • Grants and audit log rows. These were the failures not listed in the follow-up: tr role="button" with an aria-label that left out the row's visible text. On a <tr>, the role also removed the row's table semantics, so its cells could no longer be read with their column headers.
    • Each row is a plain table row again. One cell holds a button that a ::after stretches over the whole row. This is the endpoints list's existing row-link pattern; the classes are in grants/rows.ts.
    • Grants rows: the button is the role's name, named "Edit capabilities for ADMIN", which is the same name as before.
    • Audit rows: the button is the target's name, named "Open audit log entry for {{target}}". Before, every row was named "Open audit log entry".
    • Clicking anywhere on a row, or pressing Enter or Space, opens the dialog or drawer. When it closes, focus goes back to the row's button, and the row shows the ring and the hover background.
    • One trade-off, as in the endpoints list: the stretched button covers the cells, so dragging across a row no longer selects its text (the actor, capability key, timestamp or count). The audit log drawer's copy buttons still copy those values.
    • AuditLogTableRow moved to its own file so it can be tested.

Checked, no change

  • Toasts. With click-ui 0.15 (@radix-ui/react-toast 1.2.23), a scan with a toast open no longer fails. The toast <li> has no role="status", so the list rule passes, and the focus-proxy spans have no aria-hidden, so aria-hidden-focus passes. Live scans with a success toast and with an error toast open had no violations, with the WCAG tags and with every rule, in both themes. So there's nothing to fix in the app.
  • Access ConfirmDialog. Cancel and Escape still drop focus on <body> (checked live). In 0.15, ConfirmationDialogProps still extends HTMLAttributes<HTMLDivElement>. At runtime it spreads its other props onto Dialog.Content, but its types don't include onCloseAutoFocus, so useReturnFocus can't be passed without a cast. That's left for a follow-up: either click-ui types the prop, or the dialog is rebuilt on Dialog the way NavigationGuardDialog is.

Change Type

  • Bug fix (non-breaking change which fixes an issue)

Testing

Unit tests. Each change has a new test that fails without it:

  • src/__tests__/badges.test.ts: for each badge state the panel renders, in both themes, it resolves the text color (the app override, else click-ui's token) and checks contrast over the row and the hovered row. Dark reads the dark-only :root[data-cui-theme='dark'] overrides as well as the shared block. Without the override it fails on success at 4.32:1 and 4.02:1. The styles.css readers are in src/test/styles.ts (the Sidebar test's rule reader moved there and now handles selector lists), the contrast math in src/test/contrast.ts.
  • src/test/a11y.ts adds nestedControls, which lists every control that holds another focusable element, the way axe's nested-interactive rule reports them. Inert elements don't count.
  • CapabilityPanel.test.tsx and RolePermissionsPanel.test.tsx check that no control is nested in another, that click, Enter and Space open and close a category, and that "Select all" turns the whole group on without opening or closing it. CapabilityPanel.test.tsx also checks that the header button gets the inset ring.
  • DateFilter.test.tsx checks that the input is inert, again after a reset, and that the trigger is named by its label, then by its label and the chosen date.
  • rows.test.tsx checks that both rows stay table rows, that their button sits in a cell and its name contains the text the button shows, and that click, Enter and Space open the dialog or drawer.
  • Header.test.tsx checks that the search button has no aria-label or title and is named by its text. Sidebar.test.tsx checks the avatar's name.

E2E. Updated the locators that used the old names or focused the rows themselves:

  • e2e/access.ts: grantsRow now returns the row that holds the "Edit capabilities for …" button.
  • e2e/grants.spec.ts: the keyboard test tabs from the search field to the first row's button; the focus-return test checks the row's button; the date-label test checks each picker's name.
  • e2e/sidebar.spec.ts and e2e/a11y/menus.spec.ts: the avatar locator.

Runs:

  • Mock suite: on main, 86 passed, 5 failed (the scans above). With this PR it exits 0: 91 passed, 148 skipped (live-only).
  • Live, light (instance on the LibreChat dev backend): e2e/grants.spec.ts, e2e/access.spec.ts, e2e/sidebar.spec.ts and e2e/a11y/menus.spec.ts: 45 passed, 17 skipped (mock fixtures). The audit log had Granted and Revoked entries during the scans.
  • Live, dark (a temporary config with colorScheme: 'dark'): the Access and Grants scans and the keyboard, focus and date tests all passed. In the same run, login.spec.ts's "has zero WCAG 2.1 AA violations on 2FA step" failed. It signs in as the mock backend's 2fa@test.com, so it can't pass against a live backend, and this PR doesn't change it.

Screenshots, light and dark, desktop and 390 px.

  • The badge colors.
  • The row ring and hover.
  • The capability and permission headers (layout unchanged, ring inside the card).
  • The date filters with a chosen date.
  • The header search button (icon only on phones).

Chrome's own accessibility tree shows these names: "Search Ctrl+K" ("Search" at 390 px, and no description in either case), "TA, user menu for Test Admin", "Edit capabilities for ADMIN", "From" and "From Oct 01, 2026".

Test Configuration:

  • Chromium (Playwright 1.64), @axe-core/playwright 4.14, click-ui 0.15.0, LibreChat dev backend, MongoDB replica set.
  • npx tsc --noEmit, npx eslint src/ --max-warnings 0, npx prettier --check on the changed files, bun run test:lint-rules, npx vitest run (3538 tests), bun run build.

Not in scope. The audit log drawer's copy buttons (aria-label="Copy Actor ID" and similar, while each shows its value) fail the same label-in-name rule. No e2e scan opens the drawer.

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in any complex areas of my code
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes

On main, five axe scans of the mock e2e suite failed: the Access page's
Groups and Roles tabs, and the Grants page's management tab, edit dialog
and audit log tab. Each is fixed in the panel; no rule, tag or element is
excluded from a scan.

Color contrast. The success badge (Active on the grants table, Granted in
the audit log) used click-ui 0.15's #008a0b on its #ebffec tint: 4.32:1 at
10px, and 4.02:1 on a hovered row, short of AA's 4.5:1. The unlayered
`:root[data-cui-theme]` block in src/styles.css now sets
`--click-badge-opaque-color-text-success` to the panel's own
`--settings-success-fg`: #146c2e in light (6.23:1, 5.80:1 hovered) and
#86efac in dark (7.04:1, 6.30:1 hovered; click-ui's dark #ccffd0 already
passed, so dark only follows the panel's green).

Nested interactive controls.
- Each category header in the Edit capabilities dialog was a
  `div role="button"` holding its "Select all" switch. A shared
  SelectAllHeader now renders a disclosure `<button>` (chevron, label,
  optional description) beside the count and the switch. Click, Enter and
  Space still open and close the category; the switch still selects the
  whole category without toggling it. The Edit role dialog's Permissions
  tab had the same header and uses the same component.
- Each audit log date filter is click-ui's DatePicker, whose trigger button
  holds a readonly input that stays focusable at tabIndex -1. A DateFilter
  component makes that input inert and points the label at the trigger, so
  the trigger is named "From" (or "From Oct 01, 2026" once a date is
  picked) instead of "calendar", and clicking the label opens the picker.

Label in name (WCAG 2.5.3, new in axe 4.14).
- The header search button was labelled "Command menu" while it shows
  "Search Ctrl+K". It is now named by its own text; on phones, where only
  its icon shows, "Search" stays as screen-reader-only text. Its title,
  which now only repeated that name as a description, is gone.
- The sidebar avatar shows the user's initials but was named "User menu,
  Test Admin". It is now "TA, user menu for Test Admin".
- Grants and audit log rows were `tr role="button"` with an aria-label
  that left out the row's text, and the role took the cells' table
  semantics away. Each row is a table row again, and its name cell holds
  a button stretched over the row (the endpoints list's row-link
  pattern): "Edit capabilities for ADMIN", and "Open audit log entry for
  <target>". A click anywhere on the row, Enter or Space opens the dialog
  or drawer, and focus returns to the row's button when it closes. As in
  the endpoints list, a drag across a row no longer selects its text.

New unit tests cover each change and fail without it: badge contrast for
every badge state the panel renders on its row and hover surfaces in
both themes (dark also reading the dark-only click-ui overrides), no
control nested in another in both switch panels and in the date filter,
the date filter's trigger name, row semantics and opening, and the
search and avatar names. The e2e locators that used the old names or
focused the rows themselves now use the row buttons.
@danny-avila

Copy link
Copy Markdown
Contributor Author

@codex review

Pass 1. Head: b62e6d7

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T10:11:18.420318Z b62e6d7 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: b62e6d794e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danny-avila
danny-avila merged commit 7e19635 into main Oct 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant