Repository navigation
fix(dashboard): pickers announce selection and filtering (A11Y-11) - #18
Merged
Merged
Conversation
…ics (A11Y-11)
Verified live first: 0 listbox/combobox/option roles existed anywhere under
pages/Chat. The menu was a stack of divs and buttons, so the highlight that the
arrow keys move was invisible to assistive tech - a screen reader got the text of
every suggestion with no indication which one Enter would pick.
The menu is now role="listbox" with role="option" children and aria-selected on
the active one, and the composer textarea is role="combobox" with
aria-activedescendant pointing at the active option's id. Focus stays in the
textarea and the highlighted item is announced, which is the ARIA 1.2 composite
pattern for a text input with a popup listbox.
Ids come from mentionOptionId(index) so they stay stable across re-renders and
solvable by getElementById - an aria-activedescendant that resolves to nothing is
inert. The empty state is a listbox too, because aria-controls points at
MENTION_LISTBOX_ID whenever the menu is open and a non-listbox target would break
the relationship.
Two things this surfaced:
1. role="combobox" on the textarea is a role CHANGE - a combobox is a composite
widget, not a textbox, so getByRole("textbox") stops matching. That is correct
per spec, and the four queries in ChatInput.prefill.test.tsx are updated
rather than the role being weakened. A dynamic role (combobox only while
expanded) was rejected: ARIA warns against roles that change, because AT
caches them.
2. A combobox must have an accessible name, and it had none - only a
placeholder, which disappears on first keystroke and is ignored by some AT.
aria-label added.
Scope note: this covers the mention picker only. The six sibling picker popovers
(Expert, Knowledge, Skill, Subagent, Connector, HITL) still need the same
treatment; they do not all share this menu component.
Verified: tsc -b clean, eslint 0 errors (3 pre-existing exhaustive-deps warnings
in ChatInput.tsx, untouched), 7 new cases in MentionPickerMenu.a11y.test.tsx and
14 passing across the chat suites. Mutation-verified - removing role/aria-selected
fails 5 cases, and giving every option the same id fails exactly the one
id-resolution case.
…te (A11Y-11) SearchablePickerPanel backs five of the six chat pickers (expert, knowledge, skill, subagent, connector) - they are thin wrappers that own no list markup, so this one component was the whole remaining surface for those five. It had zero ARIA: the search input was named only by a placeholder, the list was a bare div, filtering was silent, and three callers carried their selected state in a CSS class alone. What changed: - The search input gets aria-label (a new required ariaLabel prop, wired at all five call sites from existing translation keys - no new strings). - The list becomes role="group" with that same label. - A visually hidden role="status" aria-live="polite" announces the result count. Why group+aria-pressed and NOT listbox+aria-selected: these are multi-select toggles in a popover, and the rows contain real <button>s (switch, hide, select). An option may not contain interactive descendants, so listbox would have been invalid nesting. The items therefore keep toggle semantics. aria-pressed added where selection was CSS-only: SubagentPickerPopover and ExpertPickerPopover. Connector and Knowledge need nothing - they already use an antd Switch, which exposes role="switch" with aria-checked. The live region is rendered in the empty branch too. An aria-live region must exist before its content changes or the change is not announced, so rendering it only alongside results would break precisely the case that needs announcing most; there is a test pinning that. Verified: tsc -b clean, eslint 0 errors on all eight files, 6 new cases in SearchablePickerPanel.a11y.test.tsx. All 5 call sites updated (the prop is required, so tsc proves there are no others). Note on the wider suite: TrajectoryInspector > "loads full content on the Preview tab" failed once and passed twice across three runs of the same command with the same working tree, and passes in isolation. That is the pre-existing load-order flakiness (roadmap item 6), not this change.
The last picker with a live gap, and the only one that does not use
SearchablePickerPanel. Its two mode buttons ("ask" / "allow all") carried the
active mode in a CSS class alone - a screen reader heard two identical toggles
with no way to tell which policy is in force, which for a tool-approval control
is the whole point of the widget.
aria-pressed added to both mode buttons, and the menu is now role="group" with
the existing chat.hitl.policy.picker label.
The trigger button and the per-tool remove buttons already had aria-label, so
they are untouched.
Verified: tsc -b clean, eslint 0 errors, 5 passing cases in
HitlPolicyPicker.test.tsx (2 new). Mutation-verified - removing both
aria-pressed attributes and the group role fails exactly the 2 new cases while
the 3 pre-existing ones still pass.
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.
A11Y-11 — pickers announce what they do
Closes the last verified-live accessibility item. Before this, there were 0 listbox/combobox/option roles anywhere under pages/Chat: the highlight that arrow keys move was invisible to assistive tech, and three pickers carried their selected state in a CSS class alone.
Three commits
1. Mention picker → real combobox/listbox. MentionPickerMenu is now role="listbox" with role="option" children and aria-selected on the active one; the composer textarea is role="combobox" with aria-activedescendant → mentionOptionId(index). Ids are stable and solvable, because an aria-activedescendant that resolves to nothing is inert.
2. Shared picker panel. SearchablePickerPanel backs five of the six pickers (expert, knowledge, skill, subagent, connector) — they are thin wrappers that own no list markup. It gained a required ariaLabel, a named search input, role="group" on the list, and a visually hidden role="status" live region announcing the result count.
3. HITL policy picker. aria-pressed on both mode buttons + role="group". For a tool-approval control, not knowing which mode is in force was the whole point of the widget.
Two judgement calls worth reviewing
These are toggles, not comboboxes. I used role="group" + aria-pressed for the panel rather than listbox + aria-selected, because the rows contain real
<button>s (switch, hide, select) and an option may not contain interactive descendants. The mention picker is a combobox and got the combobox treatment.role="combobox" on the textarea is a role change. A combobox is a composite widget, not a textbox, so getByRole("textbox") stops matching. I updated the 4 queries in ChatInput.prefill.test.tsx rather than weakening the role. I rejected making the role dynamic (combobox only while expanded) because ARIA warns against roles that change — AT caches them. I also found the textarea had no accessible name at all, only a placeholder.
Verification
One honest caveat
TrajectoryInspector > "loads full content on the Preview tab" failed once and passed twice across three back-to-back runs of the same command on the same tree, and passes in isolation. That is pre-existing load-order flakiness, not this change — reproduced as a minimal case for the separate vitest investigation. Flagging it so a flaky red here isn't mistaken for a regression from this PR.
Scope note
HitlPolicyPicker is the only picker not using the shared panel. Everything else in A11Y-11's row is now closed; document.title was already done in #16.