Skip to content

a11y(chat,account): listbox conversation rail + account menu semantics (#2346 #2348) - #2363

Merged
fl0rianr merged 2 commits into
kpoin/ui-testingfrom
feat/gui3-chat-account-a11y
Jun 23, 2026
Merged

fl0rianr merged 2 commits into
kpoin/ui-testingfrom
feat/gui3-chat-account-a11y

Conversation

@kpoineal

Copy link
Copy Markdown
Collaborator

Summary

#2346 - Chat conversation rail: full listbox keyboard navigation

File: prototype/ui-redesign/src/components/ChatView.tsx

Each

  • now carries aria-selected (reflects active conversation) and roving tabIndex (0 for selected/first, -1 for others). handleRailKeyDown/handleSheetKeyDown wire ArrowUp/Down, Home/End, Enter/Space. Delete buttons: aria-label includes conversation title, tabIndex=-1 keeps them out of Tab order. CSS: .rail__item:focus-within .rail__item-delete shows delete on keyboard focus.

    a11y: Account menu dialog/popover behavior is incomplete #2348 - Account menu: complete modal dialog semantics

    File: prototype/ui-redesign/src/features/accounts/AccountMenu.tsx

    Decision: MODAL dialog. The component already declared role="dialog" and aria-haspopup="dialog"; multi-mode forms warrant modal containment.

    Changes: aria-modal="true" added; triggerRef and panelRef added; useFocusTrap(panelRef, open) for Tab containment; useEffect Escape handler calls closePanel() which restores focus to trigger via requestAnimationFrame; close button uses closePanel().

    Tests

    8 new tests A35-A42 in tests/a11y.spec.ts:

    • A35: .rail__list has role="listbox" + accessible label
    • A36: Selected option has aria-selected="true" and tabIndex=0
    • A37: ArrowDown moves focus to next option
    • A38: Delete button aria-label includes conversation title
    • A39: Trigger has aria-haspopup="dialog" + aria-expanded="false"
    • A40: Panel has role="dialog" + aria-modal="true"; trigger aria-expanded="true"
    • A41: Opening panel moves focus inside (useFocusTrap)
    • A42: Escape closes panel + restores focus to trigger

    Test result: 58 passed, 7 skipped, 0 failed (was 50/7)

    Closes a11y: Chat conversation rail is not keyboard-complete #2346
    Closes a11y: Account menu dialog/popover behavior is incomplete #2348

    A human reviews before merge.

  • @kpoineal

    Copy link
    Copy Markdown
    Collaborator Author

    Internal review: LGTM (advisory — human approval required)

    Authored by Mattingly; reviewed by Lovell.

    • ChatView.tsx — listbox keyboard navigation (a11y: Chat conversation rail is not keyboard-complete #2346): Roving tabindex is the correct choice over aria-activedescendant for NVDA compatibility. Each [role="option"] gets a stable id (rail-conv-{id}/sheet-conv-{id}), aria-selected, and roving tabIndex. handleRailKeyDown uses e.currentTarget.querySelectorAll('[role="option"]') — correct target; no extra ref needed. ArrowUp/Down/Home/End/Enter/Space are all wired. ✅
    • Delete button tabIndex={-1} (ChatView.tsx): Noted concern, not a blocker. The delete button is unreachable by sighted keyboard-only users (no screen reader); NVDA virtual cursor browse mode can reach it. Pre-existing state had zero keyboard access to delete at all, so this is net positive for AT users. The PR documents the tradeoff and the aria-label="Delete conversation: {title}" is well-formed. For production: consider either always-visible delete with tabIndex=0 inside the option's keyboard model, or a Delete key shortcut on the focused option.
    • AccountMenu.tsx — modal dialog (a11y: Account menu dialog/popover behavior is incomplete #2348): aria-modal="true" + useFocusTrap + Escape handler with requestAnimationFrame focus restore is the correct complete pattern. e.stopPropagation() on Escape prevents bubble to bottom-sheet handler. ✅ Noted: aria-modal without a visible backdrop creates SR/visual asymmetry (screen readers lock out background content; mouse users can still click outside). Acceptable for prototype; production should add a backdrop or use non-modal popover semantics if the visual treatment stays popover-like.
    • Tests A35–A42: A37 (ArrowDown focus move programmatically triggered) and A42 (Escape → focus restored to trigger) are the two highest-value assertions. Both are correctly implemented. ✅

    @kpoineal

    Copy link
    Copy Markdown
    Collaborator Author

    Rebased onto merged base a8718a7. Conflicts resolved in ests/a11y.spec.ts (branch's new tests renumbered to A63–A70) and ACCESSIBILITY.md (union merge). Final suite: 69 passed / 7 skipped / 0 failed. — Mattingly

    @kpoineal
    kpoineal force-pushed the feat/gui3-chat-account-a11y branch 2 times, most recently from 890e1d7 to 82e5265 Compare June 23, 2026 15:04
    @kpoineal

    Copy link
    Copy Markdown
    Collaborator Author

    Rebase round 2 — rebased onto \kpoin/ui-testing\ @ e0d1c92 (#2360 backend merged)

    Conflicts resolved in \ ests/a11y.spec.ts\ and \ACCESSIBILITY.md\ by keeping both sides (base's A35–A45 presets + A51–A58 backend AND this branch's A63–A70 chat/account tests). \styles.css\ auto-merged cleanly. No renumbering needed — ranges already disjoint.

    Final suite: 77 passed / 7 skipped / 0 failed

    — Mattingly

    @kpoineal
    kpoineal force-pushed the feat/gui3-chat-account-a11y branch from 82e5265 to a4ebd09 Compare June 23, 2026 15:51
    @kpoineal

    Copy link
    Copy Markdown
    Collaborator Author

    Rebase round 3 complete — rebased onto \kpoin/ui-testing\ @ 7eb0c6c (post #2359 models merge).

    Conflicts resolved by keeping both sides:

    • \ ests/a11y.spec.ts: retained base A35–A58 (presets/models/backend) and this branch's A63–A70 (conversation rail listbox + account menu dialog). No renumbering.
    • \ACCESSIBILITY.md: union-merged both sides' table rows and last-updated line.

    Full suite result: 82 passed / 7 skipped / 0 failed. Branch force-pushed to \origin/feat/gui3-chat-account-a11y.

    — Mattingly

    @kpoineal
    kpoineal force-pushed the feat/gui3-chat-account-a11y branch from a4ebd09 to 0772762 Compare June 23, 2026 16:14
    @kpoineal

    Copy link
    Copy Markdown
    Collaborator Author

    Rebase round 4 complete — rebased onto \1c3bb93f\ (merged base after #2362 download a11y).

    Conflict resolution: kept both sides throughout:

    Test results: 86 passed / 7 skipped / 0 failed ✅

    — Mattingly

    kpoineal and others added 2 commits June 23, 2026 10:38
    … complete account menu semantics
    
    Closes #2346 #2348
    
    Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
    …+ account menu a11y
    
    Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
    @kpoineal
    kpoineal force-pushed the feat/gui3-chat-account-a11y branch from 0772762 to 51f3491 Compare June 23, 2026 16:41
    @kpoineal

    Copy link
    Copy Markdown
    Collaborator Author

    Rebase Round 5 — onto merged base 2d431d1

    Rebased \ eat/gui3-chat-account-a11y\ onto \origin/kpoin/ui-testing\ (2d431d1), which includes #2364 (forms/icons a11y: combobox semantics, durable form labels, icon-button names) as the most recent merge.

    Conflicts resolved by keeping both sides (no renumbering):

    • \ ests/a11y.spec.ts\ — placed this branch's A63–A70 (conversation rail listbox + account menu dialog) before base's A71–A79 (Group F: Omni picker, Connect forms, icon buttons); section comment numbers updated (17→19, 18→20, 19→21) to avoid duplicates; all 17 test IDs intact.
    • \ACCESSIBILITY.md\ — union-merged: test count updated to 84 (76 base + 8 branch); both sets of table rows retained (A63–A66, A67–A70 added above A71–A79); last-updated attribution merged to include both Group E and Group F items.
    • \styles.css\ — no conflict (auto-merged cleanly).
    • \ChatView.tsx\ / \AccountMenu.tsx\ — branch-only changes, no conflict.

    Full suite result: 95 passed / 7 skipped / 0 failed

    This is the last remaining open a11y PR in the GUI3 series. Rounds 1–4 merged #2357/presets, #2360/backend, #2359/models, #2362/downloads, #2364/forms+icons. #2363 is the final piece.

    — Mattingly

    @fl0rianr fl0rianr left a comment

    Copy link
    Copy Markdown
    Collaborator

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    LGTM

    @fl0rianr
    fl0rianr merged commit 779117f into kpoin/ui-testing Jun 23, 2026
    17 checks passed
    @fl0rianr
    fl0rianr deleted the feat/gui3-chat-account-a11y branch June 23, 2026 16:48
    fl0rianr pushed a commit that referenced this pull request Jul 17, 2026
    #2346 #2348) (#2363)
    
    * a11y(chat,account): full listbox keyboard nav for conversation rail + complete account menu semantics
    
    Closes #2346 #2348
    
    Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
    
    * docs(squad): add Group E learnings and decision record for chat rail + account menu a11y
    
    Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
    
    ---------
    
    Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    enhancement New feature or request web ui

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants