From 68a06ac154588561d12d6ceaa3e8477434141b31 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 11 Jul 2026 22:00:02 +0800 Subject: [PATCH] fix: preserve accessible table dialog semantics --- docs/branch-review-ledger.md | 1 + src/components/AccessibleTable.tsx | 71 ++++++++++++++---------------- tests/ui-smoke.spec.ts | 11 +++++ 3 files changed, 46 insertions(+), 37 deletions(-) diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index c467335dc..014b6d9e0 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -46,3 +46,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-11 | PR #473 / claude/mobile-search-bar-popup-bx163m | 7cef01852a9713ec51184578868212df5805adbf | open-PR review, unresolved comments, and CI | P1 merge-conflict markers removed from the shared search header while retaining the all-viewport hero portal and inline fallback. P2 fixed: phone-hidden command results can no longer open, report expanded state, receive keyboard navigation, or execute an invisible selection. The launcher and global-shell conflict findings were already resolved at the reviewed head. | No conflict markers; TypeScript; focused Prettier; app-mode/search/universal-search Vitest (37/37); `git diff --check`. Browser proof delegated to hosted CI because Turbopack rejects the isolated worktree's external node_modules junction. | | 2026-07-11 | PR #461 / claude/differentials-search-ux-polish-f2ff06 | 8bf455325b0915898417dd66aa61d419080c5528 | open-PR review, unresolved comments, and CI | Preserved diagnosis selections through workflow-aware comparison routing, constrained cross-workflow IDs to supported candidates, and removed comparison controls from presentation rows. Restored all four required core UI smoke markers and hardened answer/search mocks against invalid payloads and stale-response races. | Focused differential Vitest (22/22); TypeScript; full required CI, advisory Chromium, CodeRabbit, Semgrep, Gitleaks, and GitGuardian passed on the final head. | | 2026-07-11 | PR #485 / claude/home-answer-page-layout-rtx10n | 162085d5177f01e503b498a04efe4597478163ef | open-PR review and CI | No unresolved review threads or additional high-confidence defects were found in the compact home-layout diff. Integrated the all-viewport hero composer and retained the branch's deliberate 44px icon and 24px mobile heading scale. | TypeScript; focused app-mode/search Vitest (28/28); full Vitest (1,594 passed, 1 skipped); runtime/action/sitemap/type-scale/lint checks passed; hosted browser verification pending on the final integrated head. | +| 2026-07-11 | codex/responsive-accessibility-audit | 66883b7c86f606e617db4bee2bab6f85fff59bdc | responsive and accessibility audit | P2 fixed: the mobile expandable clinical table no longer wraps semantic table content in a duplicate ARIA button, and its full-screen dialog now traps keyboard focus while preserving Escape dismissal and focus return. Added responsive ARIA and focus regression coverage. No additional high-confidence responsive or accessibility defect was reproduced across audited primary app modes and 320px-1440px widths. | Multi-width DOM/geometry/contrast audit; a11y media (2/2); overlap (12/12); table Vitest (6/6); TypeScript; lint/static checks; full Vitest (1,598 passed, 1 skipped); `npm run verify:ui` (132/132); Prettier; `git diff --check`. Provider checks skipped. | diff --git a/src/components/AccessibleTable.tsx b/src/components/AccessibleTable.tsx index 0b745c16e..b9e1a44fe 100644 --- a/src/components/AccessibleTable.tsx +++ b/src/components/AccessibleTable.tsx @@ -1,17 +1,7 @@ "use client"; import { Maximize2, X } from "lucide-react"; -import { - type KeyboardEvent, - type ReactNode, - useCallback, - useEffect, - useId, - useMemo, - useRef, - useState, - useSyncExternalStore, -} from "react"; +import { type ReactNode, useCallback, useEffect, useId, useMemo, useRef, useState, useSyncExternalStore } from "react"; import { cn, textMuted } from "@/components/ui-primitives"; import { normalizeAccessibleTable, type NormalizedAccessibleTable } from "@/lib/accessible-table-normalization"; import { normalizeExtractedGlyphs } from "@/lib/source-text-sanitizer"; @@ -336,6 +326,7 @@ export function AccessibleTable({ lowConfidenceFallback?: ReactNode; }) { const dialogId = useId(); + const dialogRef = useRef(null); const closeButtonRef = useRef(null); const restoreFocusRef = useRef(null); const [open, setOpen] = useState(false); @@ -366,7 +357,32 @@ export function AccessibleTable({ document.body.style.overflow = "hidden"; const focusTimer = window.setTimeout(() => closeButtonRef.current?.focus(), 0); const handleKeyDown = (event: globalThis.KeyboardEvent) => { - if (event.key === "Escape") setOpen(false); + if (event.key === "Escape") { + event.preventDefault(); + setOpen(false); + return; + } + if (event.key !== "Tab") return; + + const focusable = Array.from( + dialogRef.current?.querySelectorAll( + 'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), summary, [tabindex]:not([tabindex="-1"])', + ) ?? [], + ).filter((element) => element.getAttribute("aria-hidden") !== "true"); + if (focusable.length === 0) return; + + const first = focusable[0]; + const last = focusable[focusable.length - 1]; + if (!dialogRef.current?.contains(document.activeElement)) { + event.preventDefault(); + (event.shiftKey ? last : first).focus(); + } else if (event.shiftKey && document.activeElement === first) { + event.preventDefault(); + last.focus(); + } else if (!event.shiftKey && document.activeElement === last) { + event.preventDefault(); + first.focus(); + } }; document.addEventListener("keydown", handleKeyDown); return () => { @@ -419,32 +435,10 @@ export function AccessibleTable({ setOpen(true); } - function handleSurfaceKeyDown(event: KeyboardEvent) { - if (!canExpand) return; - if (event.key !== "Enter" && event.key !== " ") return; - event.preventDefault(); - openDialog(event.currentTarget); - } - return ( <>
-
openDialog(event.currentTarget)} - onKeyDown={handleSurfaceKeyDown} - role={canExpand ? "button" : undefined} - tabIndex={canExpand ? 0 : -1} - aria-label={canExpand ? `Open ${title} full table` : undefined} - aria-haspopup={canExpand ? "dialog" : undefined} - aria-expanded={canExpand ? dialogOpen : undefined} - aria-controls={dialogOpen ? dialogId : undefined} - className={cn( - "min-w-0", - canExpand && - "relative z-50 cursor-zoom-in rounded-lg outline-none ring-offset-2 ring-offset-[color:var(--surface)] transition focus-within:ring-4 focus-within:ring-[color:var(--focus)]/25", - )} - > +
{table}
{canExpand ? ( @@ -453,6 +447,7 @@ export function AccessibleTable({ data-testid="table-expand-button" aria-label={`Open ${title} full screen`} aria-haspopup="dialog" + aria-expanded={dialogOpen} aria-controls={dialogOpen ? dialogId : undefined} onClick={(event) => { event.stopPropagation(); @@ -461,12 +456,14 @@ export function AccessibleTable({ className="relative z-50 mt-2 inline-flex min-h-11 w-full items-center justify-center gap-2 scroll-mb-[calc(18rem+env(safe-area-inset-bottom))] rounded-lg border border-[color:var(--border-lux)] bg-[color:var(--surface-raised)] px-3 text-xs font-semibold text-[color:var(--text)] shadow-[var(--shadow-tight)] transition hover:border-[color:var(--border-strong)] focus-visible:outline-none focus-visible:ring-4 focus-visible:ring-[color:var(--focus)]/25" > Expand table - + ) : null}
{dialogOpen ? (
diff --git a/tests/ui-smoke.spec.ts b/tests/ui-smoke.spec.ts index 5877ba930..a7aa3152b 100644 --- a/tests/ui-smoke.spec.ts +++ b/tests/ui-smoke.spec.ts @@ -1792,6 +1792,7 @@ test.describe("Clinical KB UI smoke coverage", () => { await expect(clinicalTable).not.toContainText(/page|p\.|chunk|Synthetic clozapine monitoring protocol/i); const expandButton = clinicalTable.getByTestId("table-expand-button"); + const tableSurface = clinicalTable.getByTestId("accessible-table-surface"); if (!viewport.expands) { await expect(page.getByRole("button", { name: "Open answer sources" })).toContainText(/sources?/i); await expect(page.getByTestId("table-specific-answer-layout")).toHaveAttribute( @@ -1822,11 +1823,21 @@ test.describe("Clinical KB UI smoke coverage", () => { return; } + await expect(tableSurface).not.toHaveAttribute("role", "button"); + await expect(tableSurface).not.toHaveAttribute("tabindex"); + await expect(expandButton).toHaveAttribute("aria-expanded", "false"); await page.keyboard.press("Escape"); const surfaceDialog = await openMobileTableFullscreen(page, clinicalTable); + await expect(expandButton).toHaveAttribute("aria-expanded", "true"); + await expect(surfaceDialog.getByRole("button", { name: "Close full-screen table" })).toBeFocused(); + await page.keyboard.press("Shift+Tab"); + expect(await surfaceDialog.evaluate((element) => element.contains(document.activeElement))).toBe(true); + await page.keyboard.press("Tab"); + expect(await surfaceDialog.evaluate((element) => element.contains(document.activeElement))).toBe(true); await expect(surfaceDialog).toContainText("FBC/ANC"); await page.keyboard.press("Escape"); await expect(surfaceDialog).toBeHidden(); + await expect(expandButton).toHaveAttribute("aria-expanded", "false"); await expect(expandButton).toBeVisible(); await scrollMobileTableExpandClearOfFooter(page, clinicalTable);