Skip to content

Fix accessible mobile table dialog semantics #495

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.

Sign up for GitHub

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

Merged
BigSimmo merged 2 commits into from
Jul 11, 2026
Merged

Fix accessible mobile table dialog semantics #495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68a06ac
fix: preserve accessible table dialog semantics
BigSimmo Jul 11, 2026
de533ac
Merge remote-tracking branch 'origin/main' into codex/responsive-acce…
BigSimmo Jul 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/branch-review-ledger.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD
| 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 | 96dbd0394888d5a52c916dba52b94d0f83e4507e | open-PR review and CI | Integrated the all-viewport hero composer, retained the compact hero scale, made composer width continuous across 1024px, and restored a mobile centering height floor. Review ledger SHAs were expanded to full IDs and source guards cover the layout invariants. | Focused source guards (30/30); TypeScript; full Vitest (1,594 passed, 1 skipped); required and advisory UI, build, static, unit, CodeRabbit, Semgrep, Gitleaks, GitGuardian, and post-merge main CI passed. |
| 2026-07-11 | PR #481 / claude/perf-r2-auth-roundtrip | 24fad070fb9834510309e74e1dc0e216cd08646b | main-integration follow-up | The stacked PR had merged into an already-merged feature base, so its reviewed delta was not present on `main`. Replayed only PR #481's first-parent patch onto current `main`, preserving current answer-route behavior while adding client payload trimming and cookie-authenticated proxy refresh coverage. | `npm run verify:cheap`; focused proxy/payload/clinical-safety Vitest (18/18); `npm run check:production-readiness:ci`; focused Prettier; `git diff --check`. |
| 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. |
71 changes: 34 additions & 37 deletions src/components/AccessibleTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -336,6 +326,7 @@ export function AccessibleTable({
lowConfidenceFallback?: ReactNode;
}) {
const dialogId = useId();
const dialogRef = useRef<HTMLDivElement>(null);
const closeButtonRef = useRef<HTMLButtonElement>(null);
const restoreFocusRef = useRef<HTMLElement | null>(null);
const [open, setOpen] = useState(false);
Expand Down Expand Up @@ -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<HTMLElement>(
'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 () => {
Expand Down Expand Up @@ -419,32 +435,10 @@ export function AccessibleTable({
setOpen(true);
}

function handleSurfaceKeyDown(event: KeyboardEvent<HTMLDivElement>) {
if (!canExpand) return;
if (event.key !== "Enter" && event.key !== " ") return;
event.preventDefault();
openDialog(event.currentTarget);
}

return (
<>
<div className="relative min-w-0">
<div
data-testid="accessible-table-surface"
onClick={(event) => 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",
)}
>
<div data-testid="accessible-table-surface" className="min-w-0">
{table}
</div>
{canExpand ? (
Expand All @@ -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();
Expand All @@ -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"
>
<span>Expand table</span>
<Maximize2 className="h-4 w-4" />
<Maximize2 className="h-4 w-4" aria-hidden />
</button>
) : null}
</div>
{dialogOpen ? (
<div
ref={dialogRef}
id={dialogId}
data-testid="table-fullscreen-dialog"
role="dialog"
aria-modal="true"
Expand All @@ -492,7 +489,7 @@ export function AccessibleTable({
onClick={() => setOpen(false)}
className="grid h-11 w-11 shrink-0 place-items-center rounded-lg border border-[color:var(--border-lux)] bg-[color:var(--surface)] text-[color:var(--text)] shadow-[var(--shadow-inset)] transition hover:border-[color:var(--border-strong)] focus-visible:outline-none focus-visible:ring-4 focus-visible:ring-[color:var(--focus)]/25"
>
<X className="h-5 w-5" />
<X className="h-5 w-5" aria-hidden />
</button>
</div>
<div className="min-h-0 flex-1 overflow-auto p-3 pb-[max(1rem,env(safe-area-inset-bottom))]">
Expand Down
11 changes: 11 additions & 0 deletions tests/ui-smoke.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down