From 393d595984f66df08504e1659997e12ad551bce9 Mon Sep 17 00:00:00 2001 From: blinding-pixels Date: Sun, 27 Sep 2026 01:03:04 +0530 Subject: [PATCH] fix(accessibility): correct control announcements and sidebar traversal (#13491) Co-authored-by: eshaan Sood Co-authored-by: Gabriel De Andrade <30420087+gabrielelpidio@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- .../connection/ConnectionFormField.test.tsx | 42 ++++++++++ .../connection/ConnectionFormField.tsx | 10 ++- apps/web/src/components/AppSidebarLayout.tsx | 2 + apps/web/src/components/Sidebar.logic.test.ts | 30 +++++++ apps/web/src/components/Sidebar.logic.ts | 15 ++++ apps/web/src/components/Sidebar.tsx | 51 ++++++++++-- apps/web/src/components/ui/scroll-area.tsx | 4 + apps/web/src/components/ui/sidebar.tsx | 7 +- apps/web/src/components/ui/switch.test.tsx | 82 ++++++++++++++++--- 9 files changed, 219 insertions(+), 24 deletions(-) create mode 100644 apps/mobile/src/features/connection/ConnectionFormField.test.tsx diff --git a/apps/mobile/src/features/connection/ConnectionFormField.test.tsx b/apps/mobile/src/features/connection/ConnectionFormField.test.tsx new file mode 100644 index 0000000000000000000000000000000000000000..343d44cf376bc633dd0825d781b339a1570bd88a --- /dev/null +++ b/apps/mobile/src/features/connection/ConnectionFormField.test.tsx @@ -0,0 +1,42 @@ +import type { ReactNode } from "react"; +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it, vi } from "vite-plus/test"; + +vi.mock("react-native", () => ({ + View: ({ children }: { children: ReactNode }) =>
{children}
, +})); +vi.mock("../../components/AppText", () => ({ + AppText: ({ + accessibilityElementsHidden, + importantForAccessibility, + children, + }: { + accessibilityElementsHidden?: boolean; + importantForAccessibility?: string; + children: ReactNode; + }) => ( + + {children} + + ), + AppTextInput: ({ accessibilityLabel }: { accessibilityLabel?: string }) => ( + + ), +})); + +import { ConnectionFormField } from "./ConnectionFormField"; + +describe("ConnectionFormField accessibility", () => { + it.each(["Host", "Pairing code"])("exposes %s once as the input name", (label) => { + const markup = renderToStaticMarkup( + , + ); + + expect(markup).toContain(``); + expect(markup).toContain(``); + }); +}); diff --git a/apps/mobile/src/features/connection/ConnectionFormField.tsx b/apps/mobile/src/features/connection/ConnectionFormField.tsx index 0d5e21148e54d034c74ef54afa871ab4567b5e8f..b492dd9ce43cb37e72bfbdae0f4664281aa98acd 100644 --- a/apps/mobile/src/features/connection/ConnectionFormField.tsx +++ b/apps/mobile/src/features/connection/ConnectionFormField.tsx @@ -3,7 +3,7 @@ import { View } from "react-native"; import { AppText, AppTextInput, type AppTextInputProps } from "../../components/AppText"; import { cn } from "../../lib/cn"; -type ConnectionFormFieldProps = Omit & { +type ConnectionFormFieldProps = Omit & { readonly label: string; readonly className?: string; }; @@ -12,12 +12,16 @@ type ConnectionFormFieldProps = Omit & { export function ConnectionFormField({ label, className, ...inputProps }: ConnectionFormFieldProps) { return ( - + {label} diff --git a/apps/web/src/components/AppSidebarLayout.tsx b/apps/web/src/components/AppSidebarLayout.tsx index 7e3eaad1125ba681a057fb958c318a7d91c8005f..05d965d95d067312f54a1f518bc89dd1e043d52f 100644 --- a/apps/web/src/components/AppSidebarLayout.tsx +++ b/apps/web/src/components/AppSidebarLayout.tsx @@ -304,6 +304,8 @@ export function AppSidebarLayout({ children }: { children: ReactNode }) { side="left" collapsible="offcanvas" data-app-sidebar="" + role="navigation" + aria-label={isOnSettings ? "Settings" : "Threads"} resizable={{ maxWidth: sidebarMaximumWidth, minWidth: THREAD_SIDEBAR_MIN_WIDTH, diff --git a/apps/web/src/components/Sidebar.logic.test.ts b/apps/web/src/components/Sidebar.logic.test.ts index b4b07805596dc7ef6d1b2c9226c4280469d6508c..fede3c1834485cb9f6e31abfde5208fe06b65911 100644 --- a/apps/web/src/components/Sidebar.logic.test.ts +++ b/apps/web/src/components/Sidebar.logic.test.ts @@ -23,6 +23,7 @@ import { isTrailingDoubleClick, orderItemsByPreferredIds, resolveProjectStatusIndicator, + resolveSidebarRowAccessibility, resolveSidebarThreadStatus, resolveThreadStatusPill, resolveWorkingStartedAt, @@ -69,6 +70,35 @@ import { const localEnvironmentId = EnvironmentId.make("environment-local"); +describe("resolveSidebarRowAccessibility", () => { + it.each([ + { + title: "Can you audit the UI?", + statusLabel: "Working", + projectDisplayName: "T3 Code", + isActive: true, + expected: { label: "Can you audit the UI?, Working, T3 Code", current: "page" }, + }, + { + title: "The audit is done", + statusLabel: null, + projectDisplayName: "T3 Code", + isActive: false, + expected: { label: "The audit is done, T3 Code", current: undefined }, + }, + { + title: "Untitled task", + statusLabel: null, + projectDisplayName: null, + isActive: false, + expected: { label: "Untitled task", current: undefined }, + }, + ])("leads with the title without folding row actions into its name: %j", (input) => { + const { expected, ...state } = input; + expect(resolveSidebarRowAccessibility(state)).toEqual(expected); + }); +}); + describe("animateSidebarLayoutChanges", () => { const baseArgs: Parameters[0] = { active: null, diff --git a/apps/web/src/components/Sidebar.logic.ts b/apps/web/src/components/Sidebar.logic.ts index 3e12090f32285d3926b179676e871b1733389270..2796b2f7f885d868442bb68ee376837cedcc70ed 100644 --- a/apps/web/src/components/Sidebar.logic.ts +++ b/apps/web/src/components/Sidebar.logic.ts @@ -42,6 +42,21 @@ export function shouldNavigateAfterThreadPark(input: { const THREAD_SELECTION_SAFE_SELECTOR = "[data-thread-item], [data-thread-selection-safe]"; export const THREAD_JUMP_HINT_SHOW_DELAY_MS = 200; + +export function resolveSidebarRowAccessibility(input: { + readonly title: string; + readonly statusLabel: string | null; + readonly projectDisplayName: string | null; + readonly isActive: boolean; +}): { readonly label: string; readonly current: "page" | undefined } { + return { + // The title is the row's identity and must lead when users scan tasks. + // Only static context belongs here; nested action labels remain separate controls. + label: [input.title, input.statusLabel, input.projectDisplayName].filter(Boolean).join(", "), + current: input.isActive ? "page" : undefined, + }; +} + // Visible sidebar rows are prewarmed into the thread-detail cache so opening a // nearby thread usually reuses an already-hot subscription. Each prewarmed // thread holds a live, fully hydrated detail subscription (all messages and diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 4926df4948ae423521c896dedb52c96e239b0c40..90f65ea2b941788d9be7c6fdcabd26865952cb42 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -173,6 +173,7 @@ import { resolveAdjacentThreadId, resolveSidebarDropTarget, resolveSidebarDropVerb, + resolveSidebarRowAccessibility, type SidebarDropVerb, resolveSidebarThreadStatus, searchSidebarThreads, @@ -728,6 +729,12 @@ const SidebarDraftRow = memo(function SidebarDraftRow(props: { promptPreview.length > 0 ? promptPreview : `${attachmentCount} attachment${attachmentCount === 1 ? "" : "s"}`; + const accessibility = resolveSidebarRowAccessibility({ + title: preview, + statusLabel: "Unsent draft", + projectDisplayName: props.projectDisplayName, + isActive: props.isActive, + }); const handleActivate = useCallback(() => onNavigate(draftId), [draftId, onNavigate]); const handleKeyDown = useCallback( (event: ReactKeyboardEvent) => { @@ -755,6 +762,8 @@ const SidebarDraftRow = memo(function SidebarDraftRow(props: {
+ {preview}
@@ -790,7 +800,9 @@ const SidebarDraftRow = memo(function SidebarDraftRow(props: {
-
{preview}
+
+ {preview} +
@@ -1447,6 +1459,13 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: { ) : null; + const accessibility = resolveSidebarRowAccessibility({ + title: thread.title, + statusLabel: topStatus?.label ?? null, + projectDisplayName: props.projectDisplayName, + isActive: props.isActive, + }); + const title = isRenaming ? ( ) : ( ); + const accessibleTitle = isRenaming ? null : {thread.title}; // Stacks show their layer count; multiple unrelated links show their total count. // Plain clicks open T3; individual PR links also support opening the host in a new tab. @@ -1593,6 +1614,8 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: { ref={rowRef} role="button" tabIndex={0} + aria-label={accessibility.label} + aria-current={accessibility.current} data-testid="sidebar-row-slim" aria-busy={isRegeneratingTitle || undefined} className={cn(rowSurfaceClassName, "flex h-9 items-center gap-2.5 px-2.5")} @@ -1603,6 +1626,7 @@ const SidebarThreadRow = memo(function SidebarThreadRow(props: { /> } > + {accessibleTitle} {/* Settled history recedes: dimmed favicon at rest, restored on hover so the tail stays scannable when you're hunting. */} } > + {accessibleTitle}
{draftIndicator} @@ -2008,6 +2035,12 @@ const SidebarSearchResultRow = memo(function SidebarSearchResultRow(props: { onFileDropThreads: (threadRef: ScopedThreadRef, files: File[]) => void; }) { const { thread } = props; + const accessibility = resolveSidebarRowAccessibility({ + title: thread.title, + statusLabel: null, + projectDisplayName: props.projectDisplayName, + isActive: props.isRouteActive, + }); const threadRef = useMemo( () => scopeThreadRef(thread.environmentId, thread.id), [thread.environmentId, thread.id], @@ -2084,12 +2117,8 @@ const SidebarSearchResultRow = memo(function SidebarSearchResultRow(props: { // which owns all keyboard interaction for the listbox. tabIndex={-1} aria-selected={props.isHighlighted} - aria-current={props.isRouteActive ? "page" : undefined} - aria-label={ - props.projectDisplayName - ? `${thread.title}, ${props.projectDisplayName}` - : thread.title - } + aria-current={accessibility.current} + aria-label={accessibility.label} onMouseMove={props.onHighlight} onClick={props.onSelect} className={cn( @@ -4573,7 +4602,7 @@ export default function Sidebar() { } > - + {isSearchingThreads ? ( threadSearchResults.length > 0 ? (
    0 && "flex-1", diff --git a/apps/web/src/components/ui/scroll-area.tsx b/apps/web/src/components/ui/scroll-area.tsx index 7792b9606e41c49381e44daee611d17c1b9e8778..ce9d41edaa5d0a5079425a1424b4e953b8c42512 100644 --- a/apps/web/src/components/ui/scroll-area.tsx +++ b/apps/web/src/components/ui/scroll-area.tsx @@ -30,6 +30,7 @@ function ScrollArea({ hideScrollbars = false, chainVerticalScroll = false, radius = "inherit", + viewportTabIndex, ...props }: ScrollAreaPrimitive.Root.Props & { scrollFade?: boolean; @@ -41,6 +42,8 @@ function ScrollArea({ chainVerticalScroll?: boolean; /** The viewport clips to the parent's radius; "none" for a region flush to an edge. */ radius?: "inherit" | "none"; + /** Override Base UI's focusable viewport when focusable descendants provide scroll access. */ + viewportTabIndex?: number; }) { return ( - + {children}
@@ -597,6 +598,10 @@ function SidebarContent({ hideScrollbars scrollFade scrollFadePadding={false} + // Thread rows provide keyboard access to this scroll region. Keeping + // Base UI's viewport out of the tab order lets its presentational role + // flatten in WebKit instead of becoming a VoiceOver interaction group. + viewportTabIndex={-1} className="h-auto min-h-0 flex-1 [&>[data-slot=scroll-area-viewport]]:[--fade-size:0.75rem]" >
[0]>) { - const html = renderToStaticMarkup(); - return html.match(/]*role="switch"[^>]*>/)?.[0] ?? ""; +let root: Root; +let container: HTMLDivElement; + +beforeEach(() => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); +}); + +afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + vi.unstubAllGlobals(); +}); + +function ControlledSwitch({ + initialChecked, + initialMixed = false, +}: { + initialChecked: boolean; + initialMixed?: boolean; +}) { + const [checked, setChecked] = useState(initialChecked); + const [mixed, setMixed] = useState(initialMixed); + + return ( + { + setMixed(false); + setChecked(nextChecked); + }} + /> + ); +} + +function switchRoot() { + const element = container.querySelector('[role="switch"]'); + if (!element) throw new Error("Switch was not rendered"); + return element; +} + +async function clickSwitch() { + await act(async () => switchRoot().click()); +} + +async function renderSwitch(initialChecked: boolean, initialMixed = false) { + await act(async () => { + root.render(); + }); } describe("Switch accessibility", () => { - it("exposes the checked state to assistive tech", () => { - expect(renderSwitchRoot({ checked: true })).toContain('aria-checked="true"'); - expect(renderSwitchRoot({ checked: false })).toContain('aria-checked="false"'); - }); + it.each([ + { initialChecked: false, initialMixed: false, before: false, after: true }, + { initialChecked: true, initialMixed: false, before: true, after: false }, + { initialChecked: false, initialMixed: true, before: "mixed", after: true }, + ])("exposes $before before activation and $after after activation", async (state) => { + await renderSwitch(state.initialChecked, state.initialMixed); + expect(switchRoot().getAttribute("aria-checked")).toBe(String(state.before)); - it("exposes the mixed state", () => { - expect(renderSwitchRoot({ checked: false, mixed: true })).toContain('aria-checked="mixed"'); + await clickSwitch(); + + expect(switchRoot().getAttribute("aria-checked")).toBe(String(state.after)); + expect(container.querySelector('input[type="checkbox"]')?.checked).toBe( + state.after, + ); }); }); -- 2.54.0