opensource-joe opened a new pull request, #3465: URL: https://github.com/apache/apisix-dashboard/pull/3465
Please answer these questions before submitting a pull request, **or your PR will get closed**. **Why submit this pull request?** - [x] Bugfix **What changes will this PR take into?** Completes the combobox/`role` semantics item from the a11y section of #3417, which #3442 explicitly left for follow-up. ### The plugin card grid was pretending to be a listbox `PluginCardList` rendered the cards inside a Mantine `Combobox`, so every card came out as `role="option"` under a `role="listbox"`. None of that was wired up: - `useVirtualizedCombobox()` created a store and passed it to `<Combobox store>`, but no `Combobox.Target` is rendered anywhere - `onOptionSubmit` does not appear anywhere in `src/`, so activating an "option" did nothing - no `aria-activedescendant`, no key handling - the search box (`PluginCardListSearch`) is rendered by the *parent*, outside the Combobox entirely, so it never controlled it So a screen reader was told this was a listbox to arrow through and select from, while in reality it is a grid of cards whose buttons are the only way to do anything. The concrete violation is that `role="option"` does not permit interactive descendants, and each card contains an Add / View / Edit / Delete button plus the docs link from #3462. Measured in the Select Plugins drawer against a live APISIX instance: ``` nested-interactive 106 violations (one per plugin card) ``` ### The fix is to stop lying about what it is It is a list of cards, so it is now a `ul` of `li`. Screen readers announce a list and its item count, the buttons and links are reachable in document order with no key handling to write, and the invalid ARIA is gone. `Combobox.Empty` becomes a plain dimmed `Text` for the no-data case. Because nothing is an ARIA option any more, the internal `Option` / `Options` / `OptionProps` names are renamed to `PluginListItem` / `PluginListItems` / `PluginListItemProps`. No exported name changes. ### Two nameless controls on the same surface Picked up while verifying the above, both `button-name` violations in the same drawer: - the search field's clear button (`CloseButton`) had no label at all - the Select Plugins drawer's close button had no label Both now use new `a11y.*` keys. `PluginEditorDrawer` was already labelling its close button, but with a hardcoded English `'Close'`, so it moves to the same key. New keys are added to all five locales with real translations, which the `locale-parity` guard requires. ### Verification Against a live APISIX instance (`e2e/server` compose) with axe-core, scoped to the Select Plugins drawer: | | before | after | |---|---|---| | `nested-interactive` | 106 | 0 | | `aria-required-children` | 0 | 0 | | `button-name` (drawer chrome + search clear) | 2 | 0 | The new spec is not vacuous. Reverting only `src/components` and re-running it fails 3 of its 4 tests, with the axe assertion producing a 3,490-line diff. **No visual change.** Drawer screenshots taken before and after are byte-identical, `sha256 0f93827bc3f9ff3b4eb1d81096e5be95290d24cf83f75e93e9121781c1ac28b4` both times. Also run: `pnpm build` clean, `pnpm lint` clean at `--max-warnings=0`, `pnpm test` 136 passed across 16 files (including `locale-parity`). The full 243-test e2e suite is running locally as a cross-check; CI runs it here too. ### One new devDependency `@axe-core/playwright`, for the assertion in the new spec. #3417 lists "Zero a11y assertions in e2e" as a test-suite gap, and #3442 added a11y coverage that could only assert on accessible names because there was no axe available. Happy to drop it and assert on the DOM shape alone if you would rather not take the dependency. ### Deliberately not fixed here Two things axe flags in the same drawer that are app-wide rather than plugin-card issues, so they want their own change and their own decision: - `color-contrast`, 33 nodes page-wide. Mantine's `variant="light"` buttons render `#228be6` on `#e9f3fd`, which is 3.16:1 against a 4.5:1 requirement. It hits Add, Docs, the language button and NavLink labels, so it is a theme-token decision, not a local one. - `landmark-no-duplicate-banner`. Mantine's `Drawer.Header` renders a `<header>`, giving a second banner landmark whenever a drawer is open. Affects every drawer in the app. Happy to open either as its own issue or PR if useful. **Related issues** Part of #3417 (please do not auto-close the tracking issue) **Checklist:** - [x] Did you explain what problem does this PR solve? Or what new features have been added? - [x] Have you added corresponding test cases? - [ ] Have you modified the corresponding document? (no user-facing document covers these controls) - [x] Is this PR backward compatible? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
