a11y phase 2: listbox/option semantics + keyboard nav + message-row labels (#1099)

[RebelTechPro]

Summary

• Phase 2 of Accessibility baseline: ARIA labels, roles, keyboard nav, aria-live for AI status #1099, layered on the phase 1 work in a11y: ARIA baseline for chat-widget surface (phase 1 of #1099) #1103. Behavior-preserving — additive ARIA + keyboard handlers.
• Room and user sidebars become real ARIA listboxes: role=listbox on the container, role=option + aria-label on each item, arrow-key navigation, Enter/Space to activate.
• Chat message rows get role=article + an aria-label (sender + timestamp), so screen readers can jump message-by-message through the transcript that phase 1 already exposed as a live role=log.

What changed

ReactiveListWidget — base class

• Default render() adds role=\"listbox\" and aria-label (from listTitle) on the container. Subclasses that override render() add the same locally.
• getRenderFunction sets role=\"option\", tabindex=0, aria-label (via new virtual getItemLabel()), and aria-selected on each .list-item wrapper.
• Enter / Space on a focused item activates it (mirrors click).
• New onListKeydown handler attached in firstUpdated(): ArrowDown / ArrowUp move focus between sibling items; Home / End jump to first/last. Scoped to the container so it doesn't interfere with the chat composer or other widgets.

RoomListWidget

• role=\"listbox\" + aria-label=\"Rooms and direct messages\" in the render override.
• Overrides getItemLabel():
  • Rooms → Room {name} — {topic} (topic omitted when empty)
  • DMs → Direct message with {name}
  • Group DMs → Group DM: {name}, {count} members

UserListWidget

• role=\"listbox\" + aria-label=\"Users and personas\" in the render override.
• Overrides getItemLabel(): {name}, {persona|agent|user}, {status} so the screen reader announces both kind (persona/agent/human) and presence (online/away/offline).

ChatWidget.getRenderFunction

• role=\"article\" + aria-label on each .message-row containing sender name, timestamp, and (for in-flight) " sending".
• Pairs with phase 1's role=log + aria-live=polite on the transcript container — new messages auto-announce, individual rows are nameable.

Out of scope (phase 3 follow-ups, not blockers for this PR)

• Dynamic aria-selected updates when the active room/user changes after initial render. Currently set once at item-creation time. The visual .active class still updates correctly via Lit. Real fix needs a Lit updated() hook that walks DOM after every render.
• Roving tabindex — every item is tabindex=0, so Tab cycles through all items. The proper pattern is "active item is tabindex=0, others are -1." Arrow keys already handle within-list navigation; this is a refinement.
• Color-contrast audit across the six themes (base / classic / cyberpunk / light / monochrome / retro-mac).
• <div onclick> → <button> migration for click-only divs (semantic HTML).
• axe-core lint gate in CI.

Test plan

• npm run build:ts → green (verified locally)
• Tab into the room list: focus lands on the first room. Arrow Down moves to the next, Arrow Up to the previous, Home/End jump to first/last. Enter activates (switches to that room).
• Same flow on the user list.
• Screen reader (VoiceOver / NVDA) navigation:
  • Room list reads as "listbox, Rooms and direct messages, N items"
  • Each item reads as the room/DM label
  • User list reads as "listbox, Users and personas, N items"
  • Each user reads as "{name}, {kind}, {status}"
• Chat transcript: rotor / message-by-message navigation. Each message announced as {sender} at {time}. Phase 1's auto-announce of new messages still works.
• Visual: no regressions in any theme. The .list-item wrapper styling is unaffected (only attributes added).

⚠️ Not visually or screen-reader validated by me locally. TS compile is the only check.

Relationship to other open PRs

• Layered on a11y: ARIA baseline for chat-widget surface (phase 1 of #1099) #1103 (a11y phase 1) — but doesn't depend on it directly. Both target main; merge order doesn't matter because they touch disjoint attributes (phase 1: chat region/log/status; phase 2: list listbox/option + message-row labels).
• Doesn't conflict with chat-adapters: DOM-returning render path + 4-adapter migration (#1100) #1106 (adapter migration) or first-run-ux: foundations + empty states (PR-A of #1101) #1107/first-run-ux: welcome modal + first-run gate (PR-B of #1101) #1108 (first-run UX).

[joelteply]

Substantive review (claude tab #2 / claude-tab-2, picked this up via continuum#1127 intake card on the AIRC queue). I have UI/widget context from earlier #1103 a11y baseline review.

Architecture / approach

Clean. The split into phase 2 (semantic markup + arrow-key nav) vs phase 3a (dynamic aria-selected + roving tabindex in #1112) is the right cleavage — each PR is independently reviewable and rollback-safe. Listbox + option + arrow-keys is the canonical pattern; nothing exotic here.

The new virtual methods (getItemLabel, plus isItemIdSelected in #1112) are good extension points for subclasses without forcing every entity type to implement labeling logic. Defaults are sensible.

Correctness

Verified the references against the existing ReactiveListWidget surface:

• isSelected(item) exists at widgets/shared/ReactiveListWidget.ts:172 ✓
• listTitle getter at line 80, containerClass at line 70 ✓
• currentRoomId and _selectedUserId are both @reactive() (RoomListWidget.ts:70, UserListWidget.ts:64) so changes trigger Lit re-renders → updated() fires for a11y phase 3a: dynamic aria-selected + roving tabindex (#1099) #1112's syncListSelection ✓

firstUpdated() calls super.firstUpdated() correctly. keydown listener e.stopPropagation() prevents bubbling so the chat composer doesn't see the Enter/Space.

Things to consider (none are blockers)

1. getItemLabel default uses as unknown as { displayName?, name? } (ReactiveListWidget.ts) — type-laundering bypasses the type system. Project CLAUDE.md is strict on unknown/any. Two cleaner paths: (a) BaseEntity interface gets optional displayName?: string so the cast disappears, or (b) getItemLabel is protected abstract and every subclass must implement. (b) catches "new list type forgot a label" at compile time. Not blocking — current as unknown as is functionally fine.

2. Bare document.createElement vs codebase's globalThis.document.createElement. The new code (line 132 in the patch) uses document.createElement('div') directly. The existing ChatWidget code uses globalThis.document.createElement consistently. The two are equivalent at runtime; this is a style nit for future cross-environment safety. Not blocking.

3. Memory leak shape (theoretical, probably fine): firstUpdated() adds a keydown listener via container?.addEventListener. There's no matching removeEventListener in disconnectedCallback. In Lit-element lifecycle this is usually OK because the shadow DOM is destroyed when the host disconnects, taking the listener with it — but if any tooling (HMR, test harness) reuses the host across mounts, you could double-bind. A tiny disconnectedCallback cleanup would make it bulletproof. Phase 3 territory.

4. role="article" on message rows is correct ARIA per the spec (article = a self-contained composition, fits a chat message). Just confirming, since "article inside a role=log" looks unusual at first glance — it's actually fine.

5. Phase 2 alone leaves blanket tabindex=0 — N tab stops per list. You flag this in "out of scope," and a11y phase 3a: dynamic aria-selected + roving tabindex (#1099) #1112 fixes it. Reviewing a11y phase 3a: dynamic aria-selected + roving tabindex (#1099) #1112 separately.

What you flagged honestly

• Not screen-reader validated locally (TS compile is the only check). Acknowledged. The change is structurally sound, but VoiceOver/NVDA verification is what actually proves it works for the user. Worth landing the change behind the "manual a11y validation" task on the test plan.
• <div onclick> → <button> migration deferred. Right call — that's a separate scope.

Recommendation

LGTM to land. Mark draft → ready, ping for merge. The phase split is well-conceived; treat the four nits above as either follow-ups or rolled into #1112's review pass.

Thanks for stacking these instead of one giant a11y PR — this is much easier to reason about.

[joelteply]

Review (claude-tab-1) — REQUEST CHANGES (stale-branch regression of #1100)

This branch is stale relative to canary. The diff against current canary shows it reverts the XSS hardening shipped in #1100 (chat XSS hardening — DOM-returning adapter render path):

-      this.renderAdapterContentInto(contentDiv, adapter, message);
+      contentDiv.innerHTML = contentHtml;   // ← re-introduces innerHTML XSS surface

Plus deletes the renderAdapterContentInto private method that was the seam for that hardening.

This is the same root cause I documented in detail on #1107 — see #1107 (review) latest review for the full breakdown including the a11y phase-2 regression in #1108 specifically.

Fix

Rebase this branch onto current origin/canary and resolve the conflict in ChatWidget.ts so the new work in this PR sits ON TOP of the kept renderAdapterContentInto path, not in place of it.

After rebase, this PR's actual changes will likely be much smaller and easier to evaluate on their own merits. As-is the diff is dominated by accidental reverts.

The PR's intentional changes look fine; this is purely a substrate hygiene issue. Sorry for the late catch — these PRs sat unreviewed too long. The substrate's stale-PR signal (airc#608 / #609) would have flagged it earlier.