diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 69faff91..b8951711 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -982,6 +982,7 @@ This file is append-only. Never rewrite or delete an existing review record; app | 2026-07-26 | cursor/global-header-scroll-hide-4fd7 (PR #1222) | `a7f6d81f8b1dd5613dda94a3ddef78d480de876e` | Cross-breakpoint header hide/reveal + tablet/desktop scroll coverage | Header now hides on scroll down and returns on scroll up at every breakpoint; bottom search dock stays phone-only. Two root causes fixed: GlobalSearchShell had no scroll source above phones (`#main-content` onScroll never fires there) and its sticky rule sat on `header#search`, which has zero travel inside two header-height parents; ClinicalDashboard's collapse row was `max-sm`-gated so it never hid. Red/green proof captured: with the four source files reverted to base `1aa64e94`, all 12 new Playwright tests and 8/10 static contract assertions fail. | `npm run verify:cheap` pass except pre-existing local `tests/pdf-extractor.test.ts` Python-OCR failure (reproduced identically at base `1aa64e94`); `npm run verify:ui` 284/284 Chromium on the main-synced tree; `check:migration-role`, `check:function-grants`, `check:branch-review-ledger` pass after the #1197 SQL sync; no provider-backed checks. | | 2026-07-25 | PR #1213 / `cursor/pr1188-fix-build-breakers-6ee0` | `c45189c76502050002e579cf2038157416a65f2d` | Deduplicate ledger rows before land | Removed 2 exact duplicate table row(s) introduced by union-merge churn (no unique review content lost). Production UI green after Sheet autofocus upgrade. | check:branch-review-ledger; hosted Production UI success on tip. | | 2026-07-25 | PR #1213 / `cursor/pr1188-fix-build-breakers-6ee0` (supersedes #1188) | merge `8e3a49d0449ec2c1b8e4e10f6cd500c0c8c9550b` | prlanded after safe close+#1213 land | LANDED. #1188 CLOSED (not merged). #1213 squash-merged to main after PR required green (Production UI/Static/Unit/Build/Migration). Content verified on main: dashboard-notices extract, indexing-v3 utils `export async function sha256Hex`, Sheet autofocus defense. Remote feature branch deleted by merge. | Hosted CI run 30165248633 success; gh pr merge squash; git cat-file content checks on origin/main. No provider calls. | +| 2026-07-25 | PR #1217 / `cursor/sheet-focus-hardening-c6d3` | `f7208d52eca6d9059f15908367cebada568b0124` | Sheet focus stress test + CI unblock before land | READY. Stress pass found and fixed two defects in this branch's own work before land: (1) the open-sheet stack guard skipped every close-restore while any sheet was open, so closing a stacked sheet dropped focus to body instead of the sheet below (now `canRestoreFocusTo`: restore proceeds when the target is inside the top-most sheet); (2) a background surface re-focusing itself from its own focus handler could trade `focus()` calls synchronously with the controller (reclaims that do not stick are now bounded; a reclaim that holds resets the count). Resolved the #1213 conflict in favour of the event-driven controller and widened the settle window to cover the lazily imported `data-sheet-autofocus` child main's 200-attempt poll was defending. Repeated `pr-branch-sync` bot merges left hosted CI `action_required`; applying `skip-branch-sync` + this agent commit to re-trigger non-bot CI before land. | `verify:pr-local` green (389 files / 3458 tests, build, client-bundle scan, RAG fixtures); `test:e2e:pr` 285/285 Chromium; `ui-smoke.spec.ts --repeat-each=3` 273/273 no flake; 120-cycle browser soak of the phone mode-menu sheet (zero inert/scroll-lock leaks, focus restored every cycle, median 0 ms / max 10 ms); new `tests/sheet-focus-stress.dom.test.tsx` 13 cases + real-browser inert guard in `ui-accessibility.spec.ts`. No provider-backed checks. | | 2026-07-25 | cursor/eval-matching-distinct-identity-6273 (PR #1204) | `c0db5f5810a37dbe7df9efa3d5479db134c98e10` | prlanded — #030 coverage hardening | LANDED. Squash `eb443dfda0fcfca8181e6839d156d9986dfc52db`. `expectedFileCoverage` now dedupes the citation window by document identity and assigns sources by maximum bipartite matching, closing two re-entry routes into #030 (repeat chunk citations of one document; order-dependent greedy assignment). Content verified by tree comparison against the squash commit (identical). Remote branch deleted at merge; local branch pruned 2026-07-25 after that verification. | `npm run verify:cheap` green pre-merge; hosted `PR required` green; tree-identity check `git diff eb443dfd c0db5f58` empty. No provider-backed checks. | | 2026-07-25 | cursor/alias-slot-disjointness-guard-6273 (PR #1215) | `406cf21eb730809fb06df00b1a9299e3462c728a` | prlanded — generalized #030 contracts + ledger #081 | LANDED. Squash `b2d794c532ea8b7e259751005165f69906fcd784`. Adds two table-independent guards to `tests/eval-document-matching.test.ts`: pairwise alias disjointness across every multi-slot eval case, and the structural rule that one document can never satisfy every slot of a multi-slot case. Also opened ledger item #081 for the then-open PR #1196 alias conflict. Content verified by tree comparison against the squash commit (identical); remote branch deleted at merge, local pruned. | `npm run verify:cheap` green; hosted `PR required` green; tree-identity check `git diff b2d794c5 406cf21e` empty. No provider-backed checks. | | 2026-07-25 | cursor/ledger-081-closeout-6273 (PR #1220) | `84e91194ecca7f74c0d70b9e30e1dbd05ab7853f` | prlanded — archive outstanding item #081 | LANDED. Squash `e7e60c6d02a37c1f5958cb97936bd3533c7a2f46`. #081 moved from Open items to Resolved/archive after PR #1196 was closed 2026-07-25 as superseded by #913 / current main; successor #1198 does not touch `src/lib/eval-document-matching.ts`, and the #1215 contracts fail closed on any re-added dual-listed alias. Merge friction worth recording: a `github-actions[bot]` branch-sync merge landed every 10-20 minutes and every bot-authored head produced `action_required` workflow runs, so the three required checks never reported and both normal and `--admin` merges were refused; runs on agent-pushed heads execute normally, so the resolution was to push an own-authored head and merge on green. Content verified by tree comparison against the squash commit (identical); remote branch deleted at merge, local pruned. | `npm run verify:cheap` on merged main: 387 files / 3431 tests pass; hosted CI, SAST, Secret Scan and PR Policy green on `84e91194`; `npm run docs:check-links` pass. No provider-backed checks. | diff --git a/src/components/ui/sheet-focus.ts b/src/components/ui/sheet-focus.ts new file mode 100644 index 00000000..7c7a99fd --- /dev/null +++ b/src/components/ui/sheet-focus.ts @@ -0,0 +1,307 @@ +"use client"; + +/** + * Stacked-overlay coordination for `Sheet`: open-sheet stack, body scroll lock, + * background inerting, and the open-focus settle controller. + * + * Split out of `sheet.tsx` so the focus rules can be unit-tested directly + * instead of only through a rendered Sheet, and so the stack is unambiguously a + * single module-level singleton shared by every Sheet instance. + * + * Every open Sheet registers a window keydown listener and locks body scroll. + * Without coordination two open Sheets (e.g. an image lightbox or table dialog + * opened over the mobile Evidence sheet) both react to a single Escape — + * closing both — and their independent per-instance scroll-lock save/restore is + * order-dependent (an out-of-order close unlocks the page behind a still-open + * sheet or leaks `overflow:hidden`). The stack lets only the top-most Sheet + * handle Escape/Tab, and its length doubles as a scroll-lock ref count so body + * scroll stays locked until the last Sheet closes and the original overflow is + * restored exactly once. + */ + +/** Marks an element this module deactivated behind the top-most sheet. */ +export const SHEET_INERT_MARKER = "data-sheet-inert"; +/** Opt-in marker for a portaled surface that is logically inside a sheet. */ +export const SHEET_PORTAL_ATTRIBUTE = "data-sheet-portal"; +/** Opt-out marker for background elements that must stay interactive. */ +export const SHEET_INERT_SKIP_ATTRIBUTE = "data-sheet-inert-skip"; + +/** + * How long a newly opened sheet keeps listening for the conditions that steal + * its initial focus: a sibling sheet finishing teardown, `focus=1` hydration + * reclaiming the composer, or a `data-sheet-autofocus` child arriving behind a + * slow dynamic import (the DocumentDrawer Find field). Listening is idle, so + * the window matches the widest case rather than the common one; it ends on the + * first user interaction, when this sheet stops being top-most, or here. + */ +export const SHEET_FOCUS_SETTLE_MS = 10_000; + +/** + * Fallback re-attempt schedule, in milliseconds after the first attempt. Covers + * focus changes that emit neither a `focusin`, a panel mutation, nor a stack + * change. Backoff converges in the common case (target present next frame) + * without the fixed 50ms polling this replaced. + */ +const FALLBACK_ATTEMPT_DELAYS_MS = [16, 32, 64, 128, 256]; + +/** + * Ceiling on reclaims that do not stick. A component that re-focuses itself + * from its own focus handler would otherwise trade `focus()` calls with this + * controller synchronously until the stack blows; giving up leaves focus where + * that surface put it. A reclaim that holds resets the count, so defending the + * panel repeatedly over the settle window is unaffected. + */ +const MAX_CONSECUTIVE_RECLAIMS = 12; + +/** + * Elements that must keep working behind a modal: stylesheets and scripts carry + * no focus, live regions must keep announcing (route changes, status), and the + * Next.js dev overlay/route announcer would otherwise be unreachable. + */ +const NEVER_INERT_TAGS = new Set(["SCRIPT", "STYLE", "LINK", "TEMPLATE", "NOSCRIPT", "NEXT-ROUTE-ANNOUNCER"]); + +type SheetEntry = { id: string; root: HTMLElement | null }; + +const openSheets: SheetEntry[] = []; +const inertedElements = new Set(); +const stackListeners = new Set<() => void>(); +let bodyScrollLockPreviousOverflow = ""; + +export function isTopmostSheet(id: string) { + return openSheets[openSheets.length - 1]?.id === id; +} + +export function hasOpenSheet() { + return openSheets.length > 0; +} + +/** + * Whether a closing sheet may still return focus to `target`. + * + * With nothing open, always. With a sheet still open, only when the target + * lives inside it — that is a stacked sheet handing focus back down to the + * sheet below. A target out in the (now inert) page background belongs to a + * sheet that was replaced rather than stacked on, and restoring it would drag + * focus out of the sheet that took over. + */ +export function canRestoreFocusTo(target: HTMLElement | null) { + if (target == null) return false; + const topRoot = openSheets[openSheets.length - 1]?.root ?? null; + if (topRoot == null) return true; + return topRoot.contains(target); +} + +/** + * Notifies on every push/pop so the top-most sheet can re-take focus the moment + * a sibling sheet finishes tearing down, instead of polling for it. + */ +export function subscribeToSheetStack(listener: () => void) { + stackListeners.add(listener); + return () => { + stackListeners.delete(listener); + }; +} + +function notifyStackListeners() { + for (const listener of Array.from(stackListeners)) listener(); +} + +export function pushSheet(id: string, root: HTMLElement | null = null) { + if (openSheets.length === 0) { + bodyScrollLockPreviousOverflow = document.body.style.overflow; + document.body.style.overflow = "hidden"; + } + openSheets.push({ id, root }); + syncBackgroundInert(); + notifyStackListeners(); +} + +export function popSheet(id: string) { + for (let index = openSheets.length - 1; index >= 0; index -= 1) { + if (openSheets[index].id === id) { + openSheets.splice(index, 1); + break; + } + } + if (openSheets.length === 0) { + document.body.style.overflow = bodyScrollLockPreviousOverflow; + } + syncBackgroundInert(); + notifyStackListeners(); +} + +function canInert(element: Element): element is HTMLElement { + if (!(element instanceof HTMLElement)) return false; + if (NEVER_INERT_TAGS.has(element.tagName) || element.tagName.startsWith("NEXTJS-")) return false; + if (element.hasAttribute(SHEET_INERT_SKIP_ATTRIBUTE)) return false; + if (element.hasAttribute("aria-live")) return false; + // Never clobber an `inert` another owner set: on release we could not tell + // whether removing it was ours to do. + return !element.hasAttribute("inert") || inertedElements.has(element); +} + +/** + * Everything outside the sheet's own subtree: walking to `document.body` and + * taking the siblings at each level covers both portaled sheets (a direct body + * child) and inline sheets nested deep in the page tree. + */ +function collectBackgroundElements(root: HTMLElement) { + const background: HTMLElement[] = []; + let node: HTMLElement = root; + while (node !== document.body) { + const parent: HTMLElement | null = node.parentElement; + if (parent == null) break; + for (const sibling of Array.from(parent.children)) { + if (sibling === node) continue; + if (canInert(sibling)) background.push(sibling); + } + node = parent; + } + return background; +} + +/** + * Containment beats reclamation: while a sheet is open the rest of the page is + * `inert`, so the browser itself refuses to move focus there. Only elements + * present when the sheet opened are marked — a popover portaled out of the + * sheet after it opened stays interactive. + */ +function syncBackgroundInert() { + if (typeof document === "undefined") return; + const topRoot = openSheets[openSheets.length - 1]?.root ?? null; + const next = + topRoot?.isConnected && document.body.contains(topRoot) + ? new Set(collectBackgroundElements(topRoot)) + : new Set(); + + for (const element of Array.from(inertedElements)) { + if (next.has(element)) continue; + element.removeAttribute("inert"); + element.removeAttribute(SHEET_INERT_MARKER); + inertedElements.delete(element); + } + for (const element of next) { + if (inertedElements.has(element)) continue; + element.setAttribute("inert", ""); + element.setAttribute(SHEET_INERT_MARKER, "true"); + inertedElements.add(element); + } +} + +/** + * A sheet may only take focus back from nothing (body) or from background it + * deactivated itself. Anything else — a control inside the panel, a popover + * portaled out of the sheet, a surface that legitimately opened on top — keeps + * the focus the user or the app gave it. + */ +export function shouldReclaimFocus(panel: HTMLElement | null, active: Element | null) { + if (active == null || active === document.body) return true; + if (panel?.contains(active)) return false; + if (active.closest(`[${SHEET_PORTAL_ATTRIBUTE}]`)) return false; + return active.closest(`[${SHEET_INERT_MARKER}="true"]`) != null; +} + +export type SheetFocusController = { cancel: () => void }; + +/** + * Drives a newly opened sheet's initial focus from events rather than a poll: + * a panel `MutationObserver` catches a late-mounted autofocus child, `focusin` + * catches focus landing elsewhere, and the stack subscription catches a sibling + * sheet's teardown. A short backoff chain covers anything none of those emit. + * The window closes on success, on the first user interaction, when this sheet + * is no longer top-most, or at `settleMs`. + */ +export function startSheetOpenFocus({ + sheetId, + getPanel, + resolveTarget, + settleMs = SHEET_FOCUS_SETTLE_MS, +}: { + sheetId: string; + getPanel: () => HTMLElement | null; + resolveTarget: () => HTMLElement | null; + settleMs?: number; +}): SheetFocusController { + let settled = false; + // The element this controller last focused. Focus sitting there is still the + // sheet's own opening focus, so it may be upgraded when a better target (a + // late-mounted autofocus child) appears — unlike focus the user moved. + let lastAppliedTarget: HTMLElement | null = null; + let reclaims = 0; + const timers: number[] = []; + const teardowns: Array<() => void> = []; + + function finish() { + if (settled) return; + settled = true; + for (const timer of timers) window.clearTimeout(timer); + timers.length = 0; + for (const teardown of teardowns.splice(0)) teardown(); + } + + function attempt() { + if (settled) return; + if (!isTopmostSheet(sheetId)) { + finish(); + return; + } + const target = resolveTarget(); + // No target yet: the autofocus child may still be mounting, so stay armed + // rather than burning attempts against a node that does not exist. + if (!target?.isConnected) return; + const active = document.activeElement; + if (active === target) return; + if (active !== lastAppliedTarget && !shouldReclaimFocus(getPanel(), active)) return; + reclaims += 1; + if (reclaims > MAX_CONSECUTIVE_RECLAIMS) { + finish(); + return; + } + target.focus({ preventScroll: true }); + if (document.activeElement === target) { + lastAppliedTarget = target; + // The reclaim held, so this was ordinary defence rather than a fight. + // Only focus that is taken straight back accumulates against the budget. + reclaims = 0; + } + } + + const frame = window.requestAnimationFrame(() => { + if (settled) return; + attempt(); + if (settled) return; + // Stay armed after a successful first focus: the resolved target can still + // improve (a deferred autofocus input mounting into the panel). + + const panel = getPanel(); + if (panel != null && typeof MutationObserver === "function") { + const observer = new MutationObserver(attempt); + observer.observe(panel, { childList: true, subtree: true }); + teardowns.push(() => observer.disconnect()); + } + + document.addEventListener("focusin", attempt); + teardowns.push(() => document.removeEventListener("focusin", attempt)); + + teardowns.push(subscribeToSheetStack(attempt)); + + // A deliberate interaction outranks the sheet's opening focus. + document.addEventListener("pointerdown", finish, true); + document.addEventListener("keydown", finish, true); + teardowns.push(() => { + document.removeEventListener("pointerdown", finish, true); + document.removeEventListener("keydown", finish, true); + }); + + for (const delay of FALLBACK_ATTEMPT_DELAYS_MS) { + timers.push(window.setTimeout(attempt, delay)); + } + timers.push(window.setTimeout(finish, settleMs)); + }); + + teardowns.push(() => window.cancelAnimationFrame(frame)); + + return { + cancel: finish, + }; +} diff --git a/src/components/ui/sheet.tsx b/src/components/ui/sheet.tsx index 596ac9b9..8fa9b449 100644 --- a/src/components/ui/sheet.tsx +++ b/src/components/ui/sheet.tsx @@ -12,41 +12,17 @@ import { import { createPortal } from "react-dom"; import { X } from "lucide-react"; import { cn, toolbarButton } from "@/components/ui-primitives"; +import { + canRestoreFocusTo, + isTopmostSheet, + popSheet, + pushSheet, + startSheetOpenFocus, + type SheetFocusController, +} from "@/components/ui/sheet-focus"; export type SheetMobileSize = "content" | "viewport"; -// Stacked-overlay coordination. Every open Sheet registers a window keydown -// listener and locks body scroll. Without coordination two open Sheets (e.g. an -// image lightbox or table dialog opened over the mobile Evidence sheet) both -// react to a single Escape — closing both — and their independent per-instance -// scroll-lock save/restore is order-dependent (an out-of-order close unlocks the -// page behind a still-open sheet or leaks `overflow:hidden`). This module-level -// stack lets only the top-most Sheet handle Escape/Tab, and the stack length -// doubles as a scroll-lock ref count so body scroll stays locked until the last -// Sheet closes and the original overflow is restored exactly once. -const openSheetStack: string[] = []; -let bodyScrollLockPreviousOverflow = ""; - -function isTopmostSheet(id: string) { - return openSheetStack[openSheetStack.length - 1] === id; -} - -function pushSheet(id: string) { - if (openSheetStack.length === 0) { - bodyScrollLockPreviousOverflow = document.body.style.overflow; - document.body.style.overflow = "hidden"; - } - openSheetStack.push(id); -} - -function popSheet(id: string) { - const index = openSheetStack.lastIndexOf(id); - if (index !== -1) openSheetStack.splice(index, 1); - if (openSheetStack.length === 0) { - document.body.style.overflow = bodyScrollLockPreviousOverflow; - } -} - /** * Responsive overlay: a bottom sheet on mobile (rises from the bottom, safe-area * aware, drag-grip) and a centred dialog from `sm:` up. CSS-only animation, no @@ -109,6 +85,7 @@ export function Sheet({ desktopBackdropClassName?: string; testId?: string; }) { + const backdropRef = useRef(null); const panelRef = useRef(null); const closeRef = useRef(null); const onCloseRef = useRef(onClose); @@ -119,16 +96,13 @@ export function Sheet({ const backdropPointerDownRef = useRef(false); // Pending focus-restore timers from the previous close. Cleared on the next // open and on unmount so a torn-down jsdom environment cannot throw from a - // stale 50ms retry under Vitest coverage workers. - const restoreTimersRef = useRef<{ - frame: number | null; - timeout: number | null; - openFocusRetry: number | null; - }>({ + // stale 50ms retry under Vitest coverage workers, and so a fast + // close-then-reopen cannot restore focus to the opener mid-open. + const restoreTimersRef = useRef<{ frame: number | null; timeout: number | null }>({ frame: null, timeout: null, - openFocusRetry: null, }); + const openFocusRef = useRef(null); const unmountingRef = useRef(false); const titleId = useId(); const descId = useId(); @@ -151,10 +125,8 @@ export function Sheet({ window.clearTimeout(restoreTimers.timeout); restoreTimers.timeout = null; } - if (restoreTimers.openFocusRetry != null) { - window.clearInterval(restoreTimers.openFocusRetry); - restoreTimers.openFocusRetry = null; - } + openFocusRef.current?.cancel(); + openFocusRef.current = null; }; }, []); @@ -198,60 +170,29 @@ export function Sheet({ const explicitReturnElement = returnFocusRef?.current ?? null; const previousActiveElement = document.activeElement instanceof HTMLElement ? document.activeElement : null; const restoreTimers = restoreTimersRef.current; - if (restoreTimers.openFocusRetry != null) { - window.clearInterval(restoreTimers.openFocusRetry); - restoreTimers.openFocusRetry = null; + // A close-then-reopen inside one frame leaves this instance's own restore + // scheduled; left pending it pulls focus back to the opener mid-open. + if (restoreTimers.frame != null) { + window.cancelAnimationFrame(restoreTimers.frame); + restoreTimers.frame = null; } - pushSheet(sheetId); - const focusFrame = window.requestAnimationFrame(() => { - const resolveAutofocusTarget = () => + if (restoreTimers.timeout != null) { + window.clearTimeout(restoreTimers.timeout); + restoreTimers.timeout = null; + } + openFocusRef.current?.cancel(); + + pushSheet(sheetId, backdropRef.current); + openFocusRef.current = startSheetOpenFocus({ + sheetId, + getPanel: () => panelRef.current, + // The close button is only a fallback: the controller upgrades to a + // deferred `data-sheet-autofocus` child (lazy DocumentDrawer Find field / + // UtilityDrawer) as soon as it mounts. + resolveTarget: () => initialFocusRef?.current ?? panelRef.current?.querySelector('[data-sheet-autofocus="true"]') ?? - null; - const focusIfNeeded = () => { - // Never reclaim focus once a newer Sheet is stacked above this one. - if (!isTopmostSheet(sheetId)) return null; - const preferredAutofocus = resolveAutofocusTarget(); - const focusTarget = preferredAutofocus ?? closeRef.current; - if (!focusTarget?.isConnected) return null; - if (document.activeElement === focusTarget) return focusTarget; - // Reclaim when focus is outside the panel (or still on body after a - // remount). Also upgrade from the close-button fallback once a deferred - // `data-sheet-autofocus` child mounts (lazy DocumentDrawer / UtilityDrawer). - // Do not steal from another in-panel control the user tabbed to. - const active = document.activeElement; - const activeIsCloseFallback = - preferredAutofocus != null && closeRef.current != null && active === closeRef.current; - if ( - active == null || - active === document.body || - activeIsCloseFallback || - (panelRef.current != null && !panelRef.current.contains(active)) - ) { - focusTarget.focus({ preventScroll: true }); - } - return focusTarget; - }; - focusIfNeeded(); - // Retries cover sibling-sheet teardown and late-mounted autofocus inputs - // (UtilityDrawer media sync / deferred drawer children). Keep retrying long - // enough to outlast focus=1 hydration (rAF + ~300ms), composer reclaim, and - // a slow dynamic import of DocumentDrawer before the Find field mounts. - let attempts = 0; - const retryTimer = window.setInterval(() => { - attempts += 1; - // Keep defending preferred autofocus for the full window. An early stop - // after the first successful focus lets focus=1 / composer reclaim steal - // the Find field and leave UI smoke stuck on a non-focused dialog input. - focusIfNeeded(); - if (attempts >= 200 || !isTopmostSheet(sheetId)) { - window.clearInterval(retryTimer); - if (restoreTimers.openFocusRetry === retryTimer) { - restoreTimers.openFocusRetry = null; - } - } - }, 50); - restoreTimers.openFocusRetry = retryTimer; + closeRef.current, }); function onKeyDown(event: KeyboardEvent) { @@ -293,19 +234,18 @@ export function Sheet({ window.addEventListener("keydown", onKeyDown); return () => { - window.cancelAnimationFrame(focusFrame); window.removeEventListener("keydown", onKeyDown); + openFocusRef.current?.cancel(); + openFocusRef.current = null; popSheet(sheetId); const restoreTarget = explicitReturnElement ?? previousActiveElement; if (restoreTimers.frame != null) { window.cancelAnimationFrame(restoreTimers.frame); + restoreTimers.frame = null; } if (restoreTimers.timeout != null) { window.clearTimeout(restoreTimers.timeout); - } - if (restoreTimers.openFocusRetry != null) { - window.clearInterval(restoreTimers.openFocusRetry); - restoreTimers.openFocusRetry = null; + restoreTimers.timeout = null; } if (unmountingRef.current) return; // Focus restore is best-effort. Under Vitest coverage workers the jsdom @@ -315,6 +255,12 @@ export function Sheet({ restoreTimers.frame = window.requestAnimationFrame(() => { restoreTimers.frame = null; if (typeof document === "undefined" || !restoreTarget?.isConnected) return; + // A sheet opened while this one was closing (switching between two + // sheets in one tick) now owns focus. Instances cannot cancel each + // other's timers, so the stack is the only place this is knowable — + // without it the new sheet has to fight the restore back. Handing + // focus back down to a sheet this one was stacked on still restores. + if (!canRestoreFocusTo(restoreTarget)) return; restoreTarget.focus({ preventScroll: true }); restoreTimers.timeout = window.setTimeout(() => { restoreTimers.timeout = null; @@ -324,6 +270,7 @@ export function Sheet({ // took focus, do not steal it back. if ( !restoreTarget.isConnected || + !canRestoreFocusTo(restoreTarget) || document.activeElement === restoreTarget || (document.activeElement !== document.body && document.activeElement != null) ) { @@ -344,6 +291,7 @@ export function Sheet({ const sheet = (
{ + if (typeof document !== "undefined" && document.body) { + document.body.style.overflow = ""; + } +}); + +function markerCount() { + return document.querySelectorAll(`[${SHEET_INERT_MARKER}="true"]`).length; +} + +function inertCount() { + return document.querySelectorAll("[inert]").length; +} + +function panelOf(title: string) { + const heading = Array.from(document.body.querySelectorAll("h2")).find((node) => node.textContent === title); + return heading?.closest('[role="dialog"]') as HTMLElement | undefined; +} + +function Stack({ depth }: { depth: 0 | 1 | 2 | 3 }) { + return ( + <> + {([1, 2, 3] as const).map((level) => ( + = level} onClose={() => {}} title={`Level ${level}`} portal> +

Level {level} body

+ + ))} + + ); +} + +describe("Sheet focus stress", () => { + it("survives rapid open/close cycling without leaking scroll lock, inert, or timers", async () => { + document.body.style.overflow = "scroll"; + const background = document.createElement("div"); + background.append(document.createElement("button")); + document.body.append(background); + + const { rerender, unmount } = render( + {}} title="Cycler" portal> +

Body

+ , + ); + + for (let cycle = 0; cycle < 25; cycle += 1) { + rerender( + {}} title="Cycler" portal> +

Body

+ , + ); + rerender( + {}} title="Cycler" portal> +

Body

+ , + ); + } + + expect(document.body.style.overflow).toBe("scroll"); + expect(markerCount()).toBe(0); + expect(background).not.toHaveAttribute("inert"); + + unmount(); + await new Promise((resolve) => setTimeout(resolve, 60)); + expect(document.body.style.overflow).toBe("scroll"); + expect(markerCount()).toBe(0); + background.remove(); + document.body.style.overflow = ""; + }); + + it("keeps three stacked sheets consistent through reverse-order teardown", () => { + const background = document.createElement("div"); + document.body.append(background); + + const { rerender } = render(); + rerender(); + const level1 = panelOf("Level 1")?.parentElement as HTMLElement; + expect(background).toHaveAttribute("inert"); + expect(level1).not.toHaveAttribute("inert"); + + rerender(); + const level2 = panelOf("Level 2")?.parentElement as HTMLElement; + expect(level1).toHaveAttribute("inert"); + expect(level2).not.toHaveAttribute("inert"); + + rerender(); + const level3 = panelOf("Level 3")?.parentElement as HTMLElement; + expect(level2).toHaveAttribute("inert"); + expect(level3).not.toHaveAttribute("inert"); + + rerender(); + expect(level2).not.toHaveAttribute("inert"); + expect(level1).toHaveAttribute("inert"); + + rerender(); + expect(background).not.toHaveAttribute("inert"); + expect(markerCount()).toBe(0); + background.remove(); + }); + + it("returns focus to the sheet below when a stacked sheet closes", async () => { + const { rerender } = render(); + await waitFor(() => { + expect(panelOf("Level 1")?.contains(document.activeElement)).toBe(true); + }); + const lowerOpener = document.activeElement as HTMLElement; + + rerender(); + await waitFor(() => { + expect(panelOf("Level 2")?.contains(document.activeElement)).toBe(true); + }); + + rerender(); + await waitFor(() => { + expect(panelOf("Level 1")?.contains(document.activeElement)).toBe(true); + }); + expect(document.activeElement).toBe(lowerOpener); + }); + + it("closes a middle sheet without disturbing the sheet above it", async () => { + function Pair({ lower, upper }: { lower: boolean; upper: boolean }) { + return ( + <> + {}} title="Lower" portal> +

Lower body

+ + {}} title="Upper" portal> +

Upper body

+ + + ); + } + + const { rerender } = render(); + await waitFor(() => { + expect(panelOf("Upper")?.contains(document.activeElement)).toBe(true); + }); + const upperFocus = document.activeElement; + + // The lower sheet closes underneath the open one. + rerender(); + await new Promise((resolve) => setTimeout(resolve, 80)); + + expect(document.activeElement).toBe(upperFocus); + expect(panelOf("Upper")?.contains(document.activeElement)).toBe(true); + }); + + it("stays balanced under StrictMode double-invoked effects", async () => { + document.body.style.overflow = ""; + const background = document.createElement("div"); + document.body.append(background); + + const { unmount } = render( + + {}} title="Strict" portal> +

Body

+ + , + ); + + await waitFor(() => { + expect(panelOf("Strict")?.contains(document.activeElement)).toBe(true); + }); + expect(document.body.style.overflow).toBe("hidden"); + expect(background).toHaveAttribute("inert"); + + unmount(); + expect(document.body.style.overflow).toBe(""); + expect(markerCount()).toBe(0); + expect(inertCount()).toBe(0); + background.remove(); + }); + + it("does not crash or leak when the sheet has no focusable target", async () => { + const timeoutSpy = vi.spyOn(window, "setTimeout"); + const { unmount } = render( + {}} portal> +

Untitled body with no controls

+ , + ); + + await new Promise((resolve) => setTimeout(resolve, 60)); + expect(document.querySelector('[role="dialog"]')).not.toBeNull(); + + unmount(); + await new Promise((resolve) => setTimeout(resolve, 40)); + expect(markerCount()).toBe(0); + timeoutSpy.mockRestore(); + }); + + it("releases every document listener it registered", async () => { + const added = new Map(); + const removed = new Map(); + const bump = (map: Map, key: string) => map.set(key, (map.get(key) ?? 0) + 1); + const addSpy = vi.spyOn(document, "addEventListener").mockImplementation(function ( + this: Document, + ...args: Parameters + ) { + bump(added, String(args[0])); + return Document.prototype.addEventListener.apply(this, args); + }); + const removeSpy = vi.spyOn(document, "removeEventListener").mockImplementation(function ( + this: Document, + ...args: Parameters + ) { + bump(removed, String(args[0])); + return Document.prototype.removeEventListener.apply(this, args); + }); + + const { unmount } = render( + {}} title="Listeners" portal> +

Body

+ , + ); + await new Promise((resolve) => setTimeout(resolve, 40)); + unmount(); + await new Promise((resolve) => setTimeout(resolve, 40)); + + for (const type of ["focusin", "pointerdown", "keydown"]) { + expect(removed.get(type) ?? 0).toBeGreaterThanOrEqual(added.get(type) ?? 0); + } + addSpy.mockRestore(); + removeSpy.mockRestore(); + }); + + it("deactivates ancestor siblings for an inline (non-portal) sheet", () => { + function InlineHost({ open }: { open: boolean }) { + return ( +
+ +
+ {}} title="Inline"> +

Body

+ +
+
+ ); + } + + const { rerender } = render(); + const sibling = document.querySelector('[data-testid="inline-sibling"]') as HTMLElement; + expect(sibling).toHaveAttribute("inert"); + // The sheet's own ancestors must stay interactive. + expect(sibling.parentElement).not.toHaveAttribute("inert"); + + rerender(); + expect(sibling).not.toHaveAttribute("inert"); + expect(markerCount()).toBe(0); + }); + + it("gives up instead of recursing when a background surface fights for focus", async () => { + const background = document.createElement("div"); + const greedy = document.createElement("button"); + background.append(greedy); + document.body.append(background); + + let steals = 0; + // Models a component that re-focuses itself from its own focus handler. + const steal = () => { + steals += 1; + if (steals > 200) return; + greedy.focus(); + }; + greedy.addEventListener("focusout", steal); + + render( + {}} title="Contested" portal> +

Body

+ , + ); + await waitFor(() => { + expect(panelOf("Contested")?.contains(document.activeElement)).toBe(true); + }); + + act(() => { + greedy.focus(); + }); + await new Promise((resolve) => setTimeout(resolve, 120)); + + // The war ends: bounded reclaims, no stack overflow, focus left alone. + expect(steals).toBeLessThan(200); + greedy.removeEventListener("focusout", steal); + background.remove(); + }); + + it("marks nothing for a detached root and still releases cleanly", () => { + const background = document.createElement("div"); + document.body.append(background); + const detachedRoot = document.createElement("div"); + + pushSheet("detached-root", detachedRoot); + expect(background).not.toHaveAttribute("inert"); + expect(markerCount()).toBe(0); + + popSheet("detached-root"); + expect(markerCount()).toBe(0); + expect(document.body.style.overflow).toBe(""); + background.remove(); + }); + + it("re-evaluates background when a sheet with a detached root is stacked on", () => { + const background = document.createElement("div"); + document.body.append(background); + const attachedRoot = document.createElement("div"); + document.body.append(attachedRoot); + + pushSheet("attached", attachedRoot); + expect(background).toHaveAttribute("inert"); + + // A top-most sheet whose root never attached must not leave the lower + // sheet's marks stranded. + pushSheet("detached", document.createElement("div")); + expect(background).not.toHaveAttribute("inert"); + expect(markerCount()).toBe(0); + + popSheet("detached"); + expect(background).toHaveAttribute("inert"); + + popSheet("attached"); + expect(markerCount()).toBe(0); + expect(document.body.style.overflow).toBe(""); + attachedRoot.remove(); + background.remove(); + }); + + it("keeps focus in a sheet opened from inside another sheet's panel", async () => { + function Nested({ inner }: { inner: boolean }) { + return ( + {}} title="Outer" portal> + + {}} title="Inner" portal> +

Inner body

+ + + ); + } + + const { rerender } = render(); + const opener = document.querySelector('[data-testid="open-inner"]') as HTMLElement; + act(() => { + opener.focus(); + }); + + rerender(); + await waitFor(() => { + expect(panelOf("Inner")?.contains(document.activeElement)).toBe(true); + }); + + rerender(); + await waitFor(() => { + expect(document.activeElement).toBe(opener); + }); + }); + + it("survives a sheet whose open state flips while another mounts and unmounts", async () => { + function Churn() { + const [tick, setTick] = useState(0); + return ( + <> + + {}} title="Even" portal> +

Even body

+ + {}} title="Odd" portal> +

Odd body

+ + + ); + } + + render(); + const churn = document.querySelector('[data-testid="churn"]') as HTMLElement; + + for (let index = 0; index < 12; index += 1) { + act(() => { + churn.click(); + }); + } + + await waitFor(() => { + expect(panelOf("Even")?.contains(document.activeElement)).toBe(true); + }); + expect(markerCount()).toBeGreaterThan(0); + expect(document.body.style.overflow).toBe("hidden"); + }); +}); diff --git a/tests/sheet-focus.dom.test.tsx b/tests/sheet-focus.dom.test.tsx new file mode 100644 index 00000000..3380e433 --- /dev/null +++ b/tests/sheet-focus.dom.test.tsx @@ -0,0 +1,257 @@ +import { act, render, waitFor } from "@testing-library/react"; +import { useEffect, useState } from "react"; +import { afterEach, describe, expect, it } from "vitest"; + +import { Sheet } from "@/components/ui/sheet"; +import { + SHEET_INERT_MARKER, + SHEET_INERT_SKIP_ATTRIBUTE, + SHEET_PORTAL_ATTRIBUTE, + shouldReclaimFocus, +} from "@/components/ui/sheet-focus"; + +afterEach(() => { + if (typeof document !== "undefined" && document.body) { + document.body.style.overflow = ""; + } +}); + +function TwoSheets({ shown }: { shown: "a" | "b" | "both" | "none" }) { + return ( + <> + {}} title="Sheet A" portal> +

A body

+ + {}} title="Sheet B" portal> +

B body

+ + + ); +} + +/** Mounts an autofocus input one tick after the sheet body first paints. */ +function DeferredAutofocusChild() { + const [ready, setReady] = useState(false); + useEffect(() => { + const timer = setTimeout(() => setReady(true), 20); + return () => clearTimeout(timer); + }, []); + return ready ? : null; +} + +function panelOf(title: string) { + const heading = Array.from(document.body.querySelectorAll("h2")).find((node) => node.textContent === title); + expect(heading, `no open sheet titled "${title}"`).toBeTruthy(); + return heading?.closest('[role="dialog"]') as HTMLElement; +} + +describe("shouldReclaimFocus", () => { + it("reclaims from nothing at all", () => { + expect(shouldReclaimFocus(null, null)).toBe(true); + expect(shouldReclaimFocus(null, document.body)).toBe(true); + }); + + it("leaves focus alone inside the panel", () => { + const panel = document.createElement("div"); + const control = document.createElement("button"); + panel.append(control); + document.body.append(panel); + + expect(shouldReclaimFocus(panel, control)).toBe(false); + panel.remove(); + }); + + it("leaves focus alone in a surface portaled out of the sheet", () => { + const panel = document.createElement("div"); + const portalHost = document.createElement("div"); + portalHost.setAttribute(SHEET_PORTAL_ATTRIBUTE, "true"); + const control = document.createElement("button"); + portalHost.append(control); + document.body.append(panel, portalHost); + + expect(shouldReclaimFocus(panel, control)).toBe(false); + panel.remove(); + portalHost.remove(); + }); + + it("reclaims only from background this module deactivated", () => { + const panel = document.createElement("div"); + const deactivated = document.createElement("div"); + deactivated.setAttribute(SHEET_INERT_MARKER, "true"); + const backgroundControl = document.createElement("button"); + deactivated.append(backgroundControl); + const untouched = document.createElement("button"); + document.body.append(panel, deactivated, untouched); + + expect(shouldReclaimFocus(panel, backgroundControl)).toBe(true); + expect(shouldReclaimFocus(panel, untouched)).toBe(false); + + panel.remove(); + deactivated.remove(); + untouched.remove(); + }); +}); + +describe("Sheet background inerting", () => { + it("deactivates page background while open and restores it on close", () => { + const background = document.createElement("div"); + background.append(document.createElement("button")); + document.body.append(background); + + const { rerender } = render( + {}} title="Solo" portal> +

Body

+ , + ); + + expect(background).toHaveAttribute("inert"); + expect(background).toHaveAttribute(SHEET_INERT_MARKER, "true"); + + rerender( + {}} title="Solo" portal> +

Body

+ , + ); + + expect(background).not.toHaveAttribute("inert"); + expect(background).not.toHaveAttribute(SHEET_INERT_MARKER); + background.remove(); + }); + + it("honours the opt-out marker and never clobbers another owner's inert", () => { + const optedOut = document.createElement("div"); + optedOut.setAttribute(SHEET_INERT_SKIP_ATTRIBUTE, "true"); + const alreadyInert = document.createElement("div"); + alreadyInert.setAttribute("inert", ""); + document.body.append(optedOut, alreadyInert); + + const { rerender } = render( + {}} title="Solo" portal> +

Body

+ , + ); + + expect(optedOut).not.toHaveAttribute("inert"); + expect(alreadyInert).not.toHaveAttribute(SHEET_INERT_MARKER); + + rerender( + {}} title="Solo" portal> +

Body

+ , + ); + + // The pre-existing inert belongs to its own owner and must survive. + expect(alreadyInert).toHaveAttribute("inert"); + optedOut.remove(); + alreadyInert.remove(); + }); + + it("deactivates a lower sheet while one is stacked above it, then revives it", () => { + const { rerender } = render(); + const lowerBackdrop = panelOf("Sheet A").parentElement as HTMLElement; + expect(lowerBackdrop).not.toHaveAttribute("inert"); + + rerender(); + expect(lowerBackdrop).toHaveAttribute("inert"); + + rerender(); + expect(lowerBackdrop).not.toHaveAttribute("inert"); + }); +}); + +describe("Sheet open focus", () => { + it("does not restore focus to the opener when another sheet takes over", async () => { + const opener = document.createElement("button"); + document.body.append(opener); + opener.focus(); + + const { rerender } = render(); + // Swap sheets in one commit: A's restore and B's open focus race. + rerender(); + + await waitFor(() => { + expect(panelOf("Sheet B").contains(document.activeElement)).toBe(true); + }); + expect(document.activeElement).not.toBe(opener); + opener.remove(); + }); + + it("keeps focus in the panel across a close-then-reopen of the same sheet", async () => { + const opener = document.createElement("button"); + document.body.append(opener); + opener.focus(); + + const { rerender } = render(); + rerender(); + rerender(); + + await waitFor(() => { + expect(panelOf("Sheet A").contains(document.activeElement)).toBe(true); + }); + opener.remove(); + }); + + it("upgrades to a late-mounted autofocus child", async () => { + render( + {}} title="Deferred" portal> + + , + ); + + await waitFor(() => { + expect(document.activeElement).toBe(document.querySelector('[data-testid="deferred-input"]')); + }); + }); + + it("takes focus back when it lands on deactivated background", async () => { + const background = document.createElement("div"); + const backgroundControl = document.createElement("button"); + background.append(backgroundControl); + document.body.append(background); + + render( + {}} title="Solo" portal> +

Body

+ , + ); + await waitFor(() => { + expect(panelOf("Solo").contains(document.activeElement)).toBe(true); + }); + + // jsdom does not enforce `inert`, so background focus is still reachable + // here; the marker is what tells the sheet this focus is stale. + act(() => { + backgroundControl.focus(); + }); + + await waitFor(() => { + expect(panelOf("Solo").contains(document.activeElement)).toBe(true); + }); + background.remove(); + }); + + it("stops reclaiming focus once the user interacts", async () => { + const background = document.createElement("div"); + const backgroundControl = document.createElement("button"); + background.append(backgroundControl); + document.body.append(background); + + render( + {}} title="Solo" portal> +

Body

+ , + ); + await waitFor(() => { + expect(panelOf("Solo").contains(document.activeElement)).toBe(true); + }); + + act(() => { + document.dispatchEvent(new Event("pointerdown", { bubbles: true })); + backgroundControl.focus(); + }); + + await new Promise((resolve) => setTimeout(resolve, 60)); + expect(document.activeElement).toBe(backgroundControl); + background.remove(); + }); +}); diff --git a/tests/ui-accessibility.spec.ts b/tests/ui-accessibility.spec.ts index 7d1f133f..37a4182a 100644 --- a/tests/ui-accessibility.spec.ts +++ b/tests/ui-accessibility.spec.ts @@ -268,6 +268,59 @@ test.describe("Clinical KB accessibility coverage", () => { await expect(modeButton).toHaveAttribute("aria-expanded", "false"); }); + test("an open sheet deactivates the page behind it and releases it on close", async ({ page }) => { + // jsdom cannot enforce `inert`, so the browser is the only place the modal + // containment contract (and its release) can actually be proven. + await page.setViewportSize({ width: 390, height: 844 }); + await mockMinimalDashboardApi(page); + await gotoApp(page); + await expectDashboardUsable(page); + + const readState = () => + page.evaluate(() => { + const marked = Array.from(document.querySelectorAll('[data-sheet-inert="true"]')); + const dialog = document.querySelector('[role="dialog"]'); + const background = marked.flatMap((element) => Array.from(element.querySelectorAll("button, a[href]"))).at(0); + let focusRefused: boolean | null = null; + if (background instanceof HTMLElement) { + const before = document.activeElement; + background.focus(); + focusRefused = document.activeElement !== background; + if (before instanceof HTMLElement) before.focus(); + } + return { + marked: marked.length, + allInert: marked.every((element) => element.hasAttribute("inert")), + dialogIsMarked: dialog != null && dialog.closest('[data-sheet-inert="true"]') != null, + focusInDialog: dialog != null && document.activeElement != null && dialog.contains(document.activeElement), + focusRefused, + overflow: document.body.style.overflow, + }; + }); + + const modeButton = page.getByRole("button", { name: "Mode Answer", exact: true }); + await modeButton.click(); + await expect(page.locator('[role="dialog"]').first()).toBeVisible(); + + const open = await readState(); + expect(open.marked, "background should be deactivated behind the sheet").toBeGreaterThan(0); + expect(open.allInert).toBe(true); + expect(open.dialogIsMarked, "the sheet itself must stay interactive").toBe(false); + expect(open.focusInDialog).toBe(true); + expect(open.focusRefused, "the browser must refuse focus into the deactivated background").toBe(true); + expect(open.overflow).toBe("hidden"); + + await page.keyboard.press("Escape"); + await expect(page.locator('[role="dialog"]').first()).toBeHidden(); + + await expect + .poll(async () => (await readState()).marked, { message: "inert markers must be released on close" }) + .toBe(0); + expect(await page.locator("[inert]").count()).toBe(0); + await expect(modeButton).toBeFocused(); + expect((await readState()).overflow).toBe(""); + }); + test("phone privacy link meets the tap target and guests do not poll private ingestion routes", async ({ page }) => { const privateIngestionRequests: string[] = []; page.on("request", (request) => { diff --git a/tests/ui-overlay-css-contract.test.ts b/tests/ui-overlay-css-contract.test.ts index a6ed1ff7..e30f6390 100644 --- a/tests/ui-overlay-css-contract.test.ts +++ b/tests/ui-overlay-css-contract.test.ts @@ -5,6 +5,7 @@ import { describe, expect, it } from "vitest"; const read = (relativePath: string) => readFileSync(new URL(`../${relativePath}`, import.meta.url), "utf8"); const answerResultSurfaceSource = read("src/components/clinical-dashboard/answer-result-surface.tsx"); const sheetSource = read("src/components/ui/sheet.tsx"); +const sheetFocusSource = read("src/components/ui/sheet-focus.ts"); const globalStylesSource = read("src/app/globals.css"); const agentsSource = read("AGENTS.md"); const searchChromeBehaviourSource = read("docs/search-chrome-behaviour.md"); @@ -29,6 +30,21 @@ describe("overlay and global CSS contracts", () => { ); }); + it("settles Sheet open focus from events rather than a polling interval", () => { + expect(sheetSource).not.toContain("setInterval"); + expect(sheetFocusSource).not.toContain("setInterval"); + expect(sheetFocusSource).toContain("new MutationObserver(attempt)"); + expect(sheetFocusSource).toContain('document.addEventListener("focusin", attempt)'); + }); + + it("holds the close-restore behind the open-sheet stack", () => { + // A sheet opened while another was closing must keep focus; without this + // guard the new sheet has to fight the old sheet's restore back. Closing a + // stacked sheet must still hand focus back down to the sheet below. + expect(sheetSource).toContain("if (!canRestoreFocusTo(restoreTarget)) return;"); + expect(sheetFocusSource).toContain("return topRoot.contains(target);"); + }); + it("defines the shared easing tokens only once", () => { expect(occurrenceCount(globalStylesSource, "--ease-standard:")).toBe(1); expect(occurrenceCount(globalStylesSource, "--ease-emphasized:")).toBe(1);