Skip to content

fix: resolve command palette aria-activedescendant wiring and restore coverage gate #499 - #587

Closed
RemmyAcee wants to merge 1 commit into
RevoraOrg:masterfrom
RemmyAcee:uiux/command-palette-grouped-results
Closed

fix: resolve command palette aria-activedescendant wiring and restore coverage gate #499#587
RemmyAcee wants to merge 1 commit into
RevoraOrg:masterfrom
RemmyAcee:uiux/command-palette-grouped-results

Conversation

@RemmyAcee

Copy link
Copy Markdown

Overview

During verification of the grouped-results command palette, two defects were found in the shipped implementation: the combobox's aria-activedescendant referenced element ids that did not exist anywhere in the DOM (so screen readers never announced the active option — a WCAG 2.1 AA / 4.1.2 failure), and branch coverage on the component sat at 91.48% against the project's enforced 95% per-file threshold, meaning CI would fail on merge.

This PR fixes both: option rows now carry stable ids that resolve the ARIA reference, and targeted tests close every reachable coverage gap (98.58% branches). The grouped-results feature itself (section headers, Navigate/Actions/Settings groups, 5-per-group limits, Recent Actions top-5 with clear-history control) was verified working end-to-end on master.

Related Issue

Closes #499

Changes

🛠 Accessibility (ARIA combobox wiring)

  • [MODIFY] src/components/CommandPalette/CommandPalette.tsx
    • Result-row buttons (role="option") now carry a stable id="cp-item-<flatIndex>" so the input's aria-activedescendant always resolves to a real element.
    • Screen readers now announce the keyboard-highlighted result during ↑/↓ navigation, completing the WAI-ARIA APG combobox pattern.
  • [MODIFY] docs/uiux/command-palette-grouped-results.md
    • Documents the stable option-id contract and corrects the listbox description to match the actual per-group role="listbox" wiring (aria-label="<GroupName> commands").

✅ Test Coverage (restores CI gate)

  • [MODIFY] src/components/CommandPalette/CommandPalette.test.tsx
    • [ADD] aria-activedescendant wiring tests: id match on first highlight, cross-group flat-index navigation, and cleared attribute when no item is active.
    • [ADD] Focus-trap wrap-around tests for Shift+Tab from the first focusable and Tab from the last, asserting both the wrap (preventDefault + focus move).
    • [ADD] Edge cases: unregistered icon names render gracefully; ArrowDown→ArrowUp round-trip returns to the previous item; Enter is a no-op when the active index points past a shrunk item list; destructive items without confirmLabel fall back to item.label in the confirm row, announcement, and button text.

Verification Results

npx vitest run src/components/CommandPalette src/hooks/useCommandPalette --coverage
✅ 130/130 passed (CommandPalette.test.tsx 101 + useCommandPalette.test.tsx 29)

Coverage (v8):
File               | % Stmts | % Branch | % Funcs | % Lines
CommandPalette.tsx |   99.44 |    98.58 |     100 |     100
commandPaletteData |     100 |      100 |     100 |     100
useCommandPalette  |     100 |      100 |     100 |     100

axe (jest-axe): no violations across empty-query, recents, results,
no-results, and inline-confirmation states.
eslint: clean for all touched files.

Notes for reviewers:

  • Remaining uncovered branches (lines 354, 391) are defensive guards unreachable via normal render flows (empty-focusable early return; timer cleanup race).
  • jsdom's CSS selector engine mishandles [tabindex="-1"]:not([tabindex="-1"]); the new trap tests are structured to stay environment-independent without changing component behavior.

Acceptance Criteria

Criteria Status
Grouped results render with labelled section headers (Navigate / Actions / Settings) ✅ verified present and axe-clean
Per-section result limits enforced (≤5 per group) ✅ covered by existing + updated tests
Recent Actions section surfaces the five most-recent commands RECENT_LIMIT = 5, per-user storage key
Recent actions are per-user and clearable ("Clear history") ✅ scoped key revora:recent-commands:<userId> + clearRecent()
Accessible (WCAG 2.1 AA) ✅ fixed aria-activedescendant wiring; axe passes in all five UI states
Responsive & RTL-safe ✅ logical properties + [dir="rtl"] overrides unchanged and verified
≥95% test coverage ✅ 98.58% branches / 99.44% statements / 100% functions & lines
Empty-query state emphasises recents ✅ Recent section on open; placeholder when history is empty

… coverage

The grouped-results command palette (issue RevoraOrg#499) shipped with two defects
that broke WCAG compliance and the project coverage gate:

- aria-activedescendant on the combobox input referenced cp-item-N ids
  that no element carried, so screen readers never announced the active
  option. Option buttons now carry stable ids matching the reference.
- Branch coverage sat at 91.48% against the enforced 95% threshold.
  Added tests for Tab/Shift+Tab trap wrap-around, activedescendant wiring,
  unregistered icon names, arrow-key round trips, stale-index Enter, and
  destructive items without confirmLabel (98.58% branches).

Design-system doc updated to match the actual per-group listbox wiring.
@drips-wave

drips-wave Bot commented Aug 24, 2026

Copy link
Copy Markdown

@RemmyAcee Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

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.

[UI/UX Design] Design a Command Palette grouped-results and recent-actions section

2 participants