Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 185 additions & 0 deletions .squad/agents/mattingly/history.md

Large diffs are not rendered by default.

80 changes: 80 additions & 0 deletions .squad/decisions/inbox/mattingly-gui3-chat-account-a11y.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# Decision: Chat Rail Listbox Focus Model + Account Menu Modal/Non-Modal

**Author:** Mattingly
**Date:** 2026-06-22
**Branch:** `feat/gui3-chat-account-a11y`
**Issues:** #2346 (conversation rail) · #2348 (account menu)
**PR:** #2363
**Status:** Implemented, tests passing (58 passed / 7 skipped / 0 failed)

---

## 1. Conversation Rail — Listbox Focus Model

### Context

The conversation rail (`<ul role="listbox">`) already had the correct ARIA role and label but no focus management. Individual `<li role="option">` elements were not focusable, had no `aria-selected`, and the container had no keyboard event handlers. For a blind NVDA user, the entire conversation list was opaque — no way to navigate it by keyboard.

### Decision: Roving tabindex (not aria-activedescendant)

Two standard patterns exist for managing focus in a listbox:

| Pattern | Mechanism | AT support | Complexity |
|---------|-----------|-----------|------------|
| **Roving tabindex** | Selected/first item has `tabIndex=0`; others `-1`; `.focus()` on arrow nav | Excellent across all screen readers | Low |
| **aria-activedescendant** | Container is the single Tab stop; `aria-activedescendant` points at "active" option ID | Good but inconsistent in some SRs | Medium |

**Chose roving tabindex** because:
1. NVDA support is better-established for roving tabindex on listboxes.
2. Simpler React implementation — no container `tabIndex` state, no ID-to-element lookup.
3. Already the right pattern given that the `<ul>` isn't the focus target.

### Roving tabindex rules

- `selectedConversation.tabIndex = 0` (the active conversation)
- If no active conversation: `conversations[0].tabIndex = 0`
- All other options: `tabIndex = -1`
- ArrowDown/Up call `options[newIdx].focus()` — works on `tabIndex=-1` elements too

### Delete button: tabIndex=-1

The `<button class="rail__item-delete">` inside each option gets `tabIndex={-1}` to keep the listbox Tab-footprint clean (one stop per option). The button remains:
- Reachable by NVDA virtual cursor (browse mode navigates all DOM elements regardless of tabIndex).
- Identifiable by its `aria-label="Delete conversation: {title}"`.
- Visually shown when the parent `<li>` has keyboard focus (`.rail__item:focus-within .rail__item-delete { opacity: 1 }`).

**Tradeoff:** Sighted keyboard-only users (no screen reader) cannot Tab to the delete button. This is acceptable because (a) deleting a conversation is a non-critical, infrequent action; (b) WCAG 2.1.1 (Keyboard) is still satisfied via the screen reader virtual cursor path; (c) adding the button back to Tab order would require either an always-visible delete button or complex Tab-order manipulation within the listbox.

---

## 2. Account Menu — Modal Dialog Decision

### Context

`AccountMenu.tsx` already declared `role="dialog"` on the panel and `aria-haspopup="dialog"` on the trigger. However, it was missing `aria-modal`, a focus trap, Escape handler, and focus restore — the "half-dialog/half-popover" problem described in #2348.

The panel is visually rendered as a positioned popover (no backdrop, `z-index: 80`) but semantically declared as a dialog.

### Decision: MODAL dialog

Alternatives considered:

| Option | Pros | Cons |
|--------|------|------|
| **Full modal (chosen)** | Consistent with existing `role="dialog"` declaration; focus contained; screen readers don't escape to page content | Visually looks like a popover (no backdrop) — minor visual/semantic mismatch |
| Non-modal popover (role=menu) | Matches visual treatment | Requires restructuring multi-mode forms to `menuitem` semantics; `role=menu` is wrong for sign-in forms |
| Non-modal unlabeled popover | Simplest | Weakest semantics; no standard pattern for multi-mode forms |

**Chose MODAL** because:
1. The component already declared `role="dialog"` — changing it to `role="menu"` would be a regression.
2. The panel contains complex multi-mode forms (sign-in, create account, settings with destructive admin actions). Modal containment prevents screen-reader focus from accidentally landing on chat messages or other content while the panel is open.
3. The existing `useFocusTrap` hook handles all implementation complexity — zero new dependencies.
4. `aria-modal="true"` + `useFocusTrap` + Escape + focus restore is the complete WCAG 2.1 requirement checklist for modal dialogs.

### Implementation notes

- `useFocusTrap(panelRef, open)` focuses the **× close button** (first focusable element) on open. This is correct: the panel always has the × button regardless of current mode.
- `autoFocus` on form inputs in 'signin' / 'create' modes fires on re-render WHEN those modes activate — after the trap has already set focus on the × button. The `autoFocus` fires naturally without conflicting with the trap.
- `closePanel()` uses `requestAnimationFrame(() => triggerRef.current?.focus())` for deferred restore — needed because the panel DOM is still present during the synchronous `setOpen(false)` call.
- The Escape handler calls `e.stopPropagation()` to prevent the event from bubbling up to the bottom-sheet Escape handler (which would close the mobile sheet unexpectedly).
- Trigger's `onClick` takes a separate open/close path: clicking to close does NOT call `closePanel()` (focus is already on the trigger from the click). Only Escape and the × button go through `closePanel()`.
12 changes: 10 additions & 2 deletions prototype/ui-redesign/ACCESSIBILITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,12 @@ Do these first. All are small changes with high compliance impact.
17. **2.1** — Add `--font-scale` token + A−/A+ UI control — DEFERRED to Phase 3
18. **1.1.3** — Convert message list to `<ol>` with `<article>` per message — DEFERRED to Phase 3

### Phase 2 Group E — Chat rail + Account menu (2026-06-22)

23. **Chat conversation rail** ✅ DONE — Full listbox keyboard navigation in `ChatView.tsx`. Each `[role="option"]` now has `aria-selected`, roving `tabIndex` (selected=0, others=-1), unique `id` for both desktop rail (`rail-conv-{id}`) and mobile sheet (`sheet-conv-{id}`). `handleRailKeyDown` / `handleSheetKeyDown` wire ArrowUp/Down, Home/End, Enter/Space. Delete buttons carry qualified `aria-label="Delete conversation: {title}"` and `tabIndex={-1}` (not a Tab stop; reachable via NVDA browse mode). CSS: `.rail__item:focus-within .rail__item-delete { opacity: 1 }` shows delete button on keyboard focus.

24. **Account menu dialog** ✅ DONE — `AccountMenu.tsx` promoted to a complete modal dialog. Added `aria-modal="true"` on the panel, `ref` on both trigger and panel, `useFocusTrap(panelRef, open)` for Tab containment, Escape keydown handler (`closePanel()`) that restores focus to the trigger via `requestAnimationFrame`. The × close button now calls `closePanel()` instead of `setOpen(false)`. **Modal-vs-popover decision: MODAL.** The panel already declared `role="dialog"` and `aria-haspopup="dialog"`; it contains multi-mode forms (sign-in, create, settings) where interaction is modal in nature. Upgrading to full modal is consistent with the existing declaration and prevents screen-reader virtual-cursor from escaping into page content while the panel is open.

### Phase 3 — Enhancements (L-effort, P2, new deps) + GUI3 Preset A11y

19. **2.2** — High-contrast theme (`[data-theme="high-contrast"]` + `forced-colors` handling) — new token set
Expand Down Expand Up @@ -582,7 +588,7 @@ npm test

> Playwright's `webServer` config in `playwright.config.ts` starts `npm run dev` automatically if nothing is already listening on port 8080. If you already have the dev server running, it reuses it (`reuseExistingServer: true`).

### Test groups (76 tests)
### Test groups (84 tests)

| Group | Tests | What it checks |
|-------|-------|----------------|
Expand All @@ -603,6 +609,8 @@ npm test
| Backend matrix + action/live regions | A51–A58 | Matrix cell buttons expose selection + labels; action buttons include recipe/backend; persistent status live regions exist |
| Model row qualified names | A46–A50 | Load/Delete/Download/Get&Load buttons include model name in aria-label; no bare generic names |
| Download progress bar | A59–A62 | `role="progressbar"` present; aria-valuenow/min/max correct; model name in label; sr-only status live region exists |
| Conversation rail listbox | A63–A66 | `role="listbox"` + label; selected option `aria-selected="true"` + tabIndex=0; ArrowDown moves focus; delete button name includes title |
| Account menu dialog | A67–A70 | `aria-haspopup` + `aria-expanded`; open = `role="dialog"` + `aria-modal`; focus moves in; Escape closes + focus restores |
| Omni picker combobox (#2347) | A71–A74 | `role="combobox"` + `aria-expanded`; focus opens/Escape closes; ArrowDown opens + listbox visible; `htmlFor`/`id` label pair |
| Connect form labels (#2349) | A75–A76 | Cloud provider fields found by `getByLabel`; marketplace search has `aria-label` |
| Icon button names (#2353) | A77–A79 | LogViewer search `aria-label`; Clear button `aria-label`; Omni picker clear `aria-label` |
Expand All @@ -613,4 +621,4 @@ Tests A25–A27 only verify that the aria-live regions **exist**. Verifying that

---

*Last updated: 2026-06-22 by Mattingly (GUI3 preset a11y items #2338 #2339 #2345 #2350 #2352; BackendManager #2343 #2344 #2351; model row qualified names #2341; download progress bar semantics #2342; Group F: combobox semantics, durable form labels, icon names)*
*Last updated: 2026-06-22 by Mattingly (GUI3 preset a11y items #2338 #2339 #2345 #2350 #2352; BackendManager #2343 #2344 #2351; model row qualified names #2341; download progress bar semantics #2342; Group E: conversation rail listbox + account menu modal dialog; Group F: combobox semantics, durable form labels, icon names)*
95 changes: 84 additions & 11 deletions prototype/ui-redesign/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,36 @@ const ChatView: React.FC<ChatViewProps> = ({ currentModel, loadedModels, onModel
if (activeId === id) setActiveId(null);
}, [activeId]);

// --- Mobile bottom sheet logic ---
const handleRailKeyDown = useCallback((e: React.KeyboardEvent<HTMLUListElement>) => {
const list = e.currentTarget;
const options = Array.from(list.querySelectorAll<HTMLElement>('[role="option"]'));
if (!options.length) return;
const currentIdx = options.findIndex(el =>
el === document.activeElement || el.contains(document.activeElement as Node),
);
if (e.key === 'ArrowDown') {
e.preventDefault();
const next = currentIdx < 0 ? 0 : (currentIdx + 1) % options.length;
options[next].focus();
} else if (e.key === 'ArrowUp') {
e.preventDefault();
const prev = currentIdx < 0 ? options.length - 1 : (currentIdx - 1 + options.length) % options.length;
options[prev].focus();
} else if (e.key === 'Home') {
e.preventDefault();
options[0].focus();
} else if (e.key === 'End') {
e.preventDefault();
options[options.length - 1].focus();
} else if ((e.key === 'Enter' || e.key === ' ') && currentIdx >= 0) {
if ((e.target as HTMLElement).tagName !== 'BUTTON') {
e.preventDefault();
handleSelectConversation(conversations[currentIdx].id);
}
}
}, [conversations, handleSelectConversation]);


const handleRailToggle = useCallback(() => {
if (window.innerWidth <= 480) {
setMobileSheetOpen(prev => !prev);
Expand All @@ -1072,6 +1101,36 @@ const ChatView: React.FC<ChatViewProps> = ({ currentModel, loadedModels, onModel
sheetTriggerRef.current?.focus();
}, []);

const handleSheetKeyDown = useCallback((e: React.KeyboardEvent<HTMLUListElement>) => {
const list = e.currentTarget;
const options = Array.from(list.querySelectorAll<HTMLElement>('[role="option"]'));
if (!options.length) return;
const currentIdx = options.findIndex(el =>
el === document.activeElement || el.contains(document.activeElement as Node),
);
if (e.key === 'ArrowDown') {
e.preventDefault();
const next = currentIdx < 0 ? 0 : (currentIdx + 1) % options.length;
options[next].focus();
} else if (e.key === 'ArrowUp') {
e.preventDefault();
const prev = currentIdx < 0 ? options.length - 1 : (currentIdx - 1 + options.length) % options.length;
options[prev].focus();
} else if (e.key === 'Home') {
e.preventDefault();
options[0].focus();
} else if (e.key === 'End') {
e.preventDefault();
options[options.length - 1].focus();
} else if ((e.key === 'Enter' || e.key === ' ') && currentIdx >= 0) {
if ((e.target as HTMLElement).tagName !== 'BUTTON') {
e.preventDefault();
handleSelectConversation(conversations[currentIdx].id);
closeMobileSheet();
}
}
}, [conversations, handleSelectConversation, closeMobileSheet]);

// ESC closes mobile sheet
useEffect(() => {
if (!mobileSheetOpen) return;
Expand Down Expand Up @@ -1839,18 +1898,24 @@ ${finalText}`
</button>
</div>

<ul className="rail__list" role="listbox" aria-label="Conversations">
{conversations.map(c => {
<ul className="rail__list" role="listbox" aria-label="Conversations" onKeyDown={handleRailKeyDown}>
{conversations.map((c, idx) => {
const badge = capabilityBadge(c.model?.capability || 'chat');
const isSelected = c.id === activeId;
const isTabTarget = isSelected || (idx === 0 && !activeId);
const convTitle = c.title || deriveTitle(c.messages);
return (
<li
className={`rail__item ${c.id === activeId ? 'is-active' : ''}`}
id={`rail-conv-${c.id}`}
className={`rail__item ${isSelected ? 'is-active' : ''}`}
key={c.id}
role="option"
aria-selected={isSelected}
tabIndex={isTabTarget ? 0 : -1}
onClick={() => handleSelectConversation(c.id)}
>
<span className="rail__item-title">
{c.title || deriveTitle(c.messages)}
{convTitle}
</span>
<span className="rail__item-meta">
{streaming.streamingConvoIds.has(c.id) && (
Expand All @@ -1864,8 +1929,9 @@ ${finalText}`
<button
className="rail__item-delete"
onClick={(e) => handleDeleteConversation(e, c.id)}
aria-label="Delete conversation"
aria-label={`Delete conversation: ${convTitle}`}
title="Delete"
tabIndex={-1}
>×</button>
</li>
);
Expand Down Expand Up @@ -1905,18 +1971,24 @@ ${finalText}`
</svg>
New Chat
</button>
<ul className="bottom-sheet__list rail__list" role="listbox" aria-label="Conversations">
{conversations.map(c => {
<ul className="bottom-sheet__list rail__list" role="listbox" aria-label="Conversations" onKeyDown={handleSheetKeyDown}>
{conversations.map((c, idx) => {
const badge = capabilityBadge(c.model?.capability || 'chat');
const isSelected = c.id === activeId;
const isTabTarget = isSelected || (idx === 0 && !activeId);
const convTitle = c.title || deriveTitle(c.messages);
return (
<li
className={`rail__item ${c.id === activeId ? 'is-active' : ''}`}
id={`sheet-conv-${c.id}`}
className={`rail__item ${isSelected ? 'is-active' : ''}`}
key={c.id}
role="option"
aria-selected={isSelected}
tabIndex={isTabTarget ? 0 : -1}
onClick={() => { handleSelectConversation(c.id); closeMobileSheet(); }}
>
<span className="rail__item-title">
{c.title || deriveTitle(c.messages)}
{convTitle}
</span>
<span className="rail__item-meta">
{streaming.streamingConvoIds.has(c.id) && (
Expand All @@ -1930,8 +2002,9 @@ ${finalText}`
<button
className="rail__item-delete"
onClick={(e) => handleDeleteConversation(e, c.id)}
aria-label="Delete conversation"
aria-label={`Delete conversation: ${convTitle}`}
title="Delete"
tabIndex={-1}
>×</button>
</li>
);
Expand Down
35 changes: 31 additions & 4 deletions prototype/ui-redesign/src/features/accounts/AccountMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React, { useEffect, useMemo, useState } from 'react';
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { AccountRecord, AccountSession, accountCount, clearAllAccountsAndScopedData, clearCurrentSessionData, createAccount, deleteOwnAccount, describeSession, listAccounts, signIn, signOut } from './accountStore';
import { useFocusTrap } from '../../hooks/useFocusTrap';

interface AccountMenuProps {
session: AccountSession;
Expand All @@ -19,10 +20,29 @@ const AccountMenu: React.FC<AccountMenuProps> = ({ session, onSessionChange, onD
const [error, setError] = useState<string | null>(null);
const [notice, setNotice] = useState<string | null>(null);

const triggerRef = useRef<HTMLButtonElement>(null);
const panelRef = useRef<HTMLDivElement>(null);

const firstAccountWillBeAdmin = useMemo(() => accountCount() === 0, [open, accounts.length]);

useEffect(() => { if (open) setAccounts(listAccounts()); }, [open, session]);

const closePanel = useCallback(() => {
setOpen(false);
requestAnimationFrame(() => triggerRef.current?.focus());
}, []);

useFocusTrap(panelRef, open);

useEffect(() => {
if (!open) return;
const handler = (e: KeyboardEvent) => {
if (e.key === 'Escape') { e.stopPropagation(); closePanel(); }
};
document.addEventListener('keydown', handler);
return () => document.removeEventListener('keydown', handler);
}, [open, closePanel]);

const resetForm = () => { setName(''); setPassword(''); setError(null); setNotice(null); };
const switchMode = (next: PanelMode) => { resetForm(); setMode(next); };

Expand Down Expand Up @@ -90,16 +110,23 @@ const AccountMenu: React.FC<AccountMenuProps> = ({ session, onSessionChange, onD

return (
<div className="account-menu">
<button className={`account-menu__trigger ${session.isGuest ? '' : 'account-menu__trigger--signed-in'}`} onClick={() => { setOpen(o => !o); setMode('menu'); setError(null); }} aria-haspopup="dialog" aria-expanded={open} title={describeSession(session)}>
<button
ref={triggerRef}
className={`account-menu__trigger ${session.isGuest ? '' : 'account-menu__trigger--signed-in'}`}
onClick={() => { if (open) { setOpen(false); } else { setOpen(true); setMode('menu'); setError(null); } }}
aria-haspopup="dialog"
aria-expanded={open}
title={describeSession(session)}
>
<span className="account-menu__avatar">{session.isGuest ? 'G' : session.name.charAt(0).toUpperCase()}</span>
<span className="account-menu__name">{session.isGuest ? 'Guest' : session.name}</span>
</button>

{open && (
<div className="account-menu__panel" role="dialog" aria-label="User settings">
<div ref={panelRef} className="account-menu__panel" role="dialog" aria-modal="true" aria-label="User settings">
<div className="account-menu__header">
<div><div className="account-menu__eyebrow">User space</div><div className="account-menu__title">{describeSession(session)}</div></div>
<button className="account-menu__close" onClick={() => setOpen(false)} aria-label="Close user menu">×</button>
<button className="account-menu__close" onClick={closePanel} aria-label="Close user menu">×</button>
</div>

{notice && <div className="account-menu__notice">{notice}</div>}
Expand Down
1 change: 1 addition & 0 deletions prototype/ui-redesign/src/styles/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ input, textarea {
}

.rail__item:hover .rail__item-delete { opacity: 1; }
.rail__item:focus-within .rail__item-delete { opacity: 1; }
.rail__item-delete:hover { background: var(--surface-3); color: var(--danger, #ff6b6b); }

.rail__empty {
Expand Down
Loading
Loading