-
Notifications
You must be signed in to change notification settings - Fork 0
feat(ux): accessibility pass — live regions, skip link, landmarks, ARIA #618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| // SPDX-FileCopyrightText: 2026 soundminds.ai | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { fireEvent, render, screen } from '@testing-library/react'; | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { ariaSortForColumn } from '@/components/common/data-table-sort-header'; | ||
| import { MessageStream } from '@/components/chat/message-stream'; | ||
| import { ResponsiveLayout } from '@/components/studies/search-space-builder/responsive-layout'; | ||
|
|
||
| describe('chat stream is a live region', () => { | ||
| it('MessageStream root is role=log aria-live=polite so streaming is announced', () => { | ||
| render(<MessageStream messages={[]} />); | ||
| const region = screen.getByTestId('message-stream'); | ||
| expect(region).toHaveAttribute('role', 'log'); | ||
| expect(region).toHaveAttribute('aria-live', 'polite'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('search-space builder tab widget ARIA', () => { | ||
| function renderTabs() { | ||
| return render(<ResponsiveLayout builder={<div>BUILDER</div>} textarea={<div>JSON</div>} />); | ||
| } | ||
|
|
||
| it('tabs reference their panels and panels reference their tabs', () => { | ||
| renderTabs(); | ||
| const builderTab = screen.getByTestId('cs-builder-tab-builder'); | ||
| expect(builderTab).toHaveAttribute('role', 'tab'); | ||
| expect(builderTab).toHaveAttribute('aria-controls', 'cs-builder-panel-builder'); | ||
|
|
||
| const builderPanel = screen.getByTestId('cs-builder-slot-builder'); | ||
| expect(builderPanel).toHaveAttribute('role', 'tabpanel'); | ||
| expect(builderPanel).toHaveAttribute('aria-labelledby', 'cs-builder-tab-builder'); | ||
| }); | ||
|
|
||
| it('ArrowRight moves the active tab AND focus (roving tabindex)', () => { | ||
| renderTabs(); | ||
| const builderTab = screen.getByTestId('cs-builder-tab-builder'); | ||
| const jsonTab = screen.getByTestId('cs-builder-tab-json'); | ||
| expect(builderTab).toHaveAttribute('aria-selected', 'true'); | ||
| fireEvent.keyDown(builderTab, { key: 'ArrowRight' }); | ||
| expect(jsonTab).toHaveAttribute('aria-selected', 'true'); | ||
| expect(builderTab).toHaveAttribute('aria-selected', 'false'); | ||
| // Focus must follow selection so keyboard flow isn't stranded on the now | ||
| // tabIndex=-1 button (Gemini review). | ||
| expect(jsonTab).toHaveFocus(); | ||
| expect(jsonTab).toHaveAttribute('tabindex', '0'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('ariaSortForColumn', () => { | ||
| it('maps active direction to the aria-sort token for the matching column', () => { | ||
| expect(ariaSortForColumn('name:asc', 'name')).toBe('ascending'); | ||
| expect(ariaSortForColumn('name:desc', 'name')).toBe('descending'); | ||
| expect(ariaSortForColumn('other:asc', 'name')).toBe('none'); | ||
| expect(ariaSortForColumn(null, 'name')).toBe('none'); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,20 +27,40 @@ export interface ResponsiveLayoutProps { | |
|
|
||
| export function ResponsiveLayout({ builder, textarea }: ResponsiveLayoutProps): React.ReactElement { | ||
| const [activeTab, setActiveTab] = React.useState<'builder' | 'json'>('builder'); | ||
| const builderTabRef = React.useRef<HTMLButtonElement>(null); | ||
| const jsonTabRef = React.useRef<HTMLButtonElement>(null); | ||
|
|
||
| // ArrowLeft/ArrowRight move between the two tabs per the ARIA tablist pattern. | ||
| // Roving tabindex requires FOCUS to follow selection, not just state — else | ||
| // focus is stranded on the now-tabIndex=-1 button and keyboard flow breaks. | ||
| const onTabKeyDown = (e: React.KeyboardEvent<HTMLButtonElement>) => { | ||
| if (e.key === 'ArrowRight' || e.key === 'ArrowLeft') { | ||
| e.preventDefault(); | ||
| const next = activeTab === 'builder' ? 'json' : 'builder'; | ||
| setActiveTab(next); | ||
| (next === 'builder' ? builderTabRef : jsonTabRef).current?.focus(); | ||
| } | ||
| }; | ||
|
Comment on lines
+36
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
|
||
| return ( | ||
| <div className="space-y-3"> | ||
| {/* Tab toggle: only visible <1024px (hidden at lg: breakpoint). */} | ||
| <div | ||
| className="lg:hidden flex gap-2 border-b border-border" | ||
| role="tablist" | ||
| aria-label="Search-space editor view" | ||
| data-testid="cs-builder-tab-toggle" | ||
| > | ||
| <button | ||
| ref={builderTabRef} | ||
| type="button" | ||
| role="tab" | ||
| id="cs-builder-tab-builder" | ||
| aria-selected={activeTab === 'builder'} | ||
| aria-controls="cs-builder-panel-builder" | ||
| tabIndex={activeTab === 'builder' ? 0 : -1} | ||
| onClick={() => setActiveTab('builder')} | ||
| onKeyDown={onTabKeyDown} | ||
| data-testid="cs-builder-tab-builder" | ||
| className={ | ||
| activeTab === 'builder' | ||
|
|
@@ -51,10 +71,15 @@ export function ResponsiveLayout({ builder, textarea }: ResponsiveLayoutProps): | |
| Builder | ||
| </button> | ||
| <button | ||
| ref={jsonTabRef} | ||
| type="button" | ||
| role="tab" | ||
| id="cs-builder-tab-json" | ||
| aria-selected={activeTab === 'json'} | ||
| aria-controls="cs-builder-panel-json" | ||
| tabIndex={activeTab === 'json' ? 0 : -1} | ||
| onClick={() => setActiveTab('json')} | ||
| onKeyDown={onTabKeyDown} | ||
| data-testid="cs-builder-tab-json" | ||
| className={ | ||
| activeTab === 'json' | ||
|
|
@@ -71,12 +96,18 @@ export function ResponsiveLayout({ builder, textarea }: ResponsiveLayoutProps): | |
| RHF register binding + existing test selectors. */} | ||
| <div className="grid gap-4 lg:grid-cols-2"> | ||
| <div | ||
| role="tabpanel" | ||
| id="cs-builder-panel-builder" | ||
| aria-labelledby="cs-builder-tab-builder" | ||
| className={activeTab === 'json' ? 'hidden lg:block' : 'lg:block'} | ||
| data-testid="cs-builder-slot-builder" | ||
| > | ||
| {builder} | ||
| </div> | ||
| <div | ||
| role="tabpanel" | ||
| id="cs-builder-panel-json" | ||
| aria-labelledby="cs-builder-tab-json" | ||
| className={activeTab === 'builder' ? 'hidden lg:block' : 'lg:block'} | ||
| data-testid="cs-builder-slot-json" | ||
| > | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making the entire
MessageStreamcontainer 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-onlywitharia-live="polite") that only announces: