Repository navigation
♿ fix: Pass Every Axe Scan on the Grants and Access Pages - #203
Merged
Merged
Conversation
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.
Contributor
Author
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
4 tasks done
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
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.
Pull Request Template
Summary
On
main(click-ui 0.15,@axe-core/playwright4.14), five axe scans in the mock e2e suite fail:e2e/access.spec.ts);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
#008a0bon its#ebffectint. 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 insrc/styles.cssalready maps the switch tokens to the panel's own tokens. It now sets the success badge text the same way:#008a0b: 4.32 / 4.02#146c2e: 6.23 / 5.80#ccffd0: 8.84 / 7.91#86efac: 7.04 / 6.30Dark already passed. It switches only so that both themes use the panel's success green, the one
StateChipuses. 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
div role="button"that held its "Select all" switch. A new sharedSelectAllHeaderputs a disclosure<button>(chevron, label, optional description) next to the count and the switch instead. The button hasaria-expandedandaria-controls..select-all-toggleshares.sidebar-toggle's inset ring rule.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.DatePickerputs its readonly input inside the trigger button, and the input stays focusable even withtabIndex = -1. The newDateFiltercomponent replaces the oldDatePickerCellpatch:inert, so the input is still shown but can't take focus or clicks and leaves the accessibility tree.forto 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).Label in name (WCAG 2.5.3, new in axe 4.14)
axe's rule only checks elements that have
aria-labeloraria-labelledby. For those, it requires the visible text, includingaria-hiddentext that's still on screen, to appear as a run of words in the accessible name. Text in parentheses doesn't count.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".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)".{{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 keycom_nav_user_menu_labelreplacescom_nav_user_menu.tr role="button"with anaria-labelthat 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.::afterstretches over the whole row. This is the endpoints list's existingrow-linkpattern; the classes are ingrants/rows.ts.AuditLogTableRowmoved to its own file so it can be tested.Checked, no change
@radix-ui/react-toast1.2.23), a scan with a toast open no longer fails. The toast<li>has norole="status", so thelistrule passes, and the focus-proxy spans have noaria-hidden, soaria-hidden-focuspasses. 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.ConfirmDialog. Cancel and Escape still drop focus on<body>(checked live). In 0.15,ConfirmationDialogPropsstill extendsHTMLAttributes<HTMLDivElement>. At runtime it spreads its other props ontoDialog.Content, but its types don't includeonCloseAutoFocus, souseReturnFocuscan't be passed without a cast. That's left for a follow-up: either click-ui types the prop, or the dialog is rebuilt onDialogthe wayNavigationGuardDialogis.Change Type
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 insrc/test/styles.ts(the Sidebar test's rule reader moved there and now handles selector lists), the contrast math insrc/test/contrast.ts.src/test/a11y.tsaddsnestedControls, 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.tsxandRolePermissionsPanel.test.tsxcheck 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.tsxalso checks that the header button gets the inset ring.DateFilter.test.tsxchecks 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.tsxchecks 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.tsxchecks that the search button has noaria-labelortitleand is named by its text.Sidebar.test.tsxchecks the avatar's name.E2E. Updated the locators that used the old names or focused the rows themselves:
e2e/access.ts:grantsRownow 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.tsande2e/a11y/menus.spec.ts: the avatar locator.Runs:
main, 86 passed, 5 failed (the scans above). With this PR it exits 0: 91 passed, 148 skipped (live-only).devbackend):e2e/grants.spec.ts,e2e/access.spec.ts,e2e/sidebar.spec.tsande2e/a11y/menus.spec.ts: 45 passed, 17 skipped (mock fixtures). The audit log had Granted and Revoked entries during the scans.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's2fa@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.
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:
@axe-core/playwright4.14, click-ui 0.15.0, LibreChatdevbackend, MongoDB replica set.npx tsc --noEmit,npx eslint src/ --max-warnings 0,npx prettier --checkon 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