From 106d61d0cd78f64a5d74fafcfd34a436bb033432 Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Wed, 17 Jun 2026 03:25:20 -0700 Subject: [PATCH 1/2] feat(a11y): modal focus-trap + focus-restore + role=dialog (useModalA11y) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds hooks/useModalA11y: marks a modal as role="dialog" aria-modal with an accessible name, moves focus into it on open, traps Tab/Shift+Tab within it, and restores focus to the trigger on close. Applied to CreateWorkItemModal, CreateGraphModal, and WorkItemDetailsModal (the details modal keeps its own initial container focus; fixed its aria-labelledby→aria-label so the name isn't the title input's value). Pairs with useDialog (Escape/click-outside). New @a11y suite (tests/e2e/a11y-focus.spec.ts) asserts role=dialog, focus enters the modal, Tab is trapped, and focus returns to the trigger on close — behavior that was previously untested and mostly absent (you could Tab out of a modal into the graph behind it). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../web/src/components/CreateGraphModal.tsx | 5 +- .../src/components/CreateWorkItemModal.tsx | 5 +- .../src/components/WorkItemDetailsModal.tsx | 8 +- packages/web/src/hooks/useModalA11y.ts | 107 ++++++++++++++++ tests/e2e/a11y-focus.spec.ts | 117 ++++++++++++++++++ 5 files changed, 239 insertions(+), 3 deletions(-) create mode 100644 packages/web/src/hooks/useModalA11y.ts create mode 100644 tests/e2e/a11y-focus.spec.ts diff --git a/packages/web/src/components/CreateGraphModal.tsx b/packages/web/src/components/CreateGraphModal.tsx index 22dd012c..8fecd862 100644 --- a/packages/web/src/components/CreateGraphModal.tsx +++ b/packages/web/src/components/CreateGraphModal.tsx @@ -1,5 +1,6 @@ import { useState, useRef, useEffect } from 'react'; import { useDialog } from '../hooks/useDialogManager'; +import { useModalA11y } from '../hooks/useModalA11y'; import { X, Folder, FolderOpen, Plus, Copy, FileText } from 'lucide-react'; import { useGraph } from '../contexts/GraphContext'; import { useAuth } from '../contexts/AuthContext'; @@ -14,6 +15,8 @@ interface CreateGraphModalProps { export function CreateGraphModal({ isOpen, onClose, parentGraphId }: CreateGraphModalProps) { useDialog(isOpen, onClose); + const panelRef = useRef(null); + useModalA11y(panelRef, { isOpen, label: 'Create new graph' }); const { currentTeam, currentUser } = useAuth(); const { createGraph, duplicateGraph, availableGraphs, isCreating } = useGraph(); const { showSuccess, showError } = useNotifications(); @@ -219,7 +222,7 @@ export function CreateGraphModal({ isOpen, onClose, parentGraphId }: CreateGraph /> {/* Modern eye-catching modal */} -
+
{/* Animated gradient border */}
diff --git a/packages/web/src/components/CreateWorkItemModal.tsx b/packages/web/src/components/CreateWorkItemModal.tsx index ab742ab0..01d4bc82 100644 --- a/packages/web/src/components/CreateWorkItemModal.tsx +++ b/packages/web/src/components/CreateWorkItemModal.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { createPortal } from 'react-dom'; import { useDialog } from '../hooks/useDialogManager'; +import { useModalA11y } from '../hooks/useModalA11y'; import { useMutation, useQuery } from '@apollo/client'; import { X, Link, ChevronDown, Plus } from 'lucide-react'; import { CREATE_WORK_ITEM, GET_WORK_ITEMS, GET_EDGES, CREATE_EDGE } from '../lib/queries'; @@ -50,6 +51,8 @@ interface CreateWorkItemModalProps { export function CreateWorkItemModal({ isOpen, onClose, parentWorkItemId, position, onSubmit }: CreateWorkItemModalProps) { useDialog(isOpen, onClose); + const panelRef = React.useRef(null); + useModalA11y(panelRef, { isOpen, label: parentWorkItemId ? 'Create and connect work item' : 'Create new work item' }); const { currentUser, currentTeam } = useAuth(); const { currentGraph } = useGraph(); const { showSuccess, showError } = useNotifications(); @@ -345,7 +348,7 @@ export function CreateWorkItemModal({ isOpen, onClose, parentWorkItemId, positio onClick={onClose} /> -
e.stopPropagation()}> +
e.stopPropagation()}>
diff --git a/packages/web/src/components/WorkItemDetailsModal.tsx b/packages/web/src/components/WorkItemDetailsModal.tsx index f38841c2..813e25dd 100644 --- a/packages/web/src/components/WorkItemDetailsModal.tsx +++ b/packages/web/src/components/WorkItemDetailsModal.tsx @@ -11,6 +11,7 @@ import { useAuth } from '../contexts/AuthContext'; import { useGraph } from '../contexts/GraphContext'; import { useNotifications } from '../contexts/NotificationContext'; import { useDialog } from '../hooks/useDialogManager'; +import { useModalA11y } from '../hooks/useModalA11y'; import { Calendar, Clock, Layers, Trophy, Target, ListTodo, AlertTriangle, Lightbulb, Microscope, @@ -80,8 +81,12 @@ export function WorkItemDetailsModal({ const disconnectDropdownRef = useRef(null); const datePickerRef = useRef(null); const modalRef = useRef(null); + const dialogRef = useRef(null); useDialog(isOpen, onClose); + // Container already manages its own initial focus (modalRef) below; this adds + // the Tab focus-trap + focus-restore-to-trigger on top. + useModalA11y(dialogRef, { isOpen, initialFocus: false }); useEffect(() => { if (node) { @@ -516,10 +521,11 @@ export function WorkItemDetailsModal({ return createPortal((
0 || el.offsetHeight > 0 || el === document.activeElement; +} + +export function useModalA11y(ref: RefObject, opts: ModalA11yOptions): void { + const { isOpen, label, labelledBy, initialFocus = true } = opts; + + useEffect(() => { + const el = ref.current; + if (!isOpen || !el) return; + + if (!el.getAttribute('role')) el.setAttribute('role', 'dialog'); + el.setAttribute('aria-modal', 'true'); + if (labelledBy) el.setAttribute('aria-labelledby', labelledBy); + else if (label && !el.getAttribute('aria-labelledby')) el.setAttribute('aria-label', label); + + const previouslyFocused = document.activeElement as HTMLElement | null; + + const focusables = () => + Array.from(el.querySelectorAll(FOCUSABLE)).filter(isVisible); + + let raf = 0; + if (initialFocus) { + // Defer past paint so portaled inputs/buttons exist before we grab focus. + raf = requestAnimationFrame(() => { + const items = focusables(); + if (items[0]) { + items[0].focus({ preventScroll: true }); + } else { + if (!el.getAttribute('tabindex')) el.setAttribute('tabindex', '-1'); + el.focus({ preventScroll: true }); + } + }); + } + + const onKeyDown = (e: KeyboardEvent) => { + if (e.key !== 'Tab') return; + const items = focusables(); + if (items.length === 0) { + e.preventDefault(); + el.focus({ preventScroll: true }); + return; + } + const first = items[0]; + const last = items[items.length - 1]; + const active = document.activeElement as HTMLElement | null; + if (e.shiftKey) { + if (active === first || !el.contains(active)) { + e.preventDefault(); + last.focus({ preventScroll: true }); + } + } else if (active === last || !el.contains(active)) { + e.preventDefault(); + first.focus({ preventScroll: true }); + } + }; + el.addEventListener('keydown', onKeyDown); + + return () => { + if (raf) cancelAnimationFrame(raf); + el.removeEventListener('keydown', onKeyDown); + if ( + previouslyFocused && + previouslyFocused !== document.body && + document.contains(previouslyFocused) && + typeof previouslyFocused.focus === 'function' + ) { + previouslyFocused.focus({ preventScroll: true }); + } + }; + }, [isOpen, label, labelledBy, initialFocus, ref]); +} diff --git a/tests/e2e/a11y-focus.spec.ts b/tests/e2e/a11y-focus.spec.ts new file mode 100644 index 00000000..ee96cede --- /dev/null +++ b/tests/e2e/a11y-focus.spec.ts @@ -0,0 +1,117 @@ +import { test, expect, Page } from '@playwright/test'; +import { login, TEST_USERS, getBaseURL } from '../helpers/auth'; + +/** + * Modal accessibility / keyboard-focus gate (@a11y). Asserts the contract added + * by useModalA11y: a modal is exposed as role="dialog" aria-modal with an + * accessible name, keyboard focus MOVES INTO it on open, Tab/Shift+Tab is + * TRAPPED within it (never leaks to the page behind), and focus is RESTORED to + * the trigger on close. These were entirely untested and mostly unimplemented; + * a modal you can Tab out of (into the graph behind it) is real keyboard friction. + */ + +const FOCUSABLE = + 'a[href],button:not([disabled]),input:not([disabled]),select:not([disabled]),textarea:not([disabled]),[tabindex]:not([tabindex="-1"])'; + +async function openWorkspace(page: Page, viewMode = 'cards') { + await login(page, TEST_USERS.ADMIN); + await page.addInitScript((m) => localStorage.setItem('graphdone:viewMode', m), viewMode); + await page.goto(`${getBaseURL()}/`, { waitUntil: 'domcontentloaded' }); + await page.waitForTimeout(3000); +} + +const focusWithinDialog = (page: Page) => + page.evaluate(() => { + const d = document.querySelector('[role="dialog"][aria-modal="true"]'); + return !!d && !!document.activeElement && d.contains(document.activeElement); + }); + +/** Focus the first focusable in the dialog, then Tab `presses` times; focus must + * stay inside the dialog the whole time (the trap). Returns true if it never escaped. */ +async function tabStaysTrapped(page: Page, presses = 14): Promise { + await page.evaluate((sel) => { + const d = document.querySelector('[role="dialog"][aria-modal="true"]'); + const f = d?.querySelector(sel) as HTMLElement | null; + f?.focus(); + }, FOCUSABLE); + for (let i = 0; i < presses; i++) { + await page.keyboard.press('Tab'); + if (!(await focusWithinDialog(page))) return false; + } + // and a few back-tabs + for (let i = 0; i < 4; i++) { + await page.keyboard.press('Shift+Tab'); + if (!(await focusWithinDialog(page))) return false; + } + return true; +} + +test.describe('modal a11y: role + focus trap + restore @a11y', () => { + test.describe.configure({ timeout: 90_000 }); + + test.describe('desktop', () => { + test.use({ viewport: { width: 1440, height: 900 } }); + + test('work-item details modal: role=dialog, accessible name, focus trapped', async ({ page }) => { + await openWorkspace(page, 'cards'); + await page.locator('[data-testid="view-content"] .grid > div').first().click().catch(() => {}); + await page.waitForTimeout(1200); + const badge = page.locator('[data-testid="details-type-badge"]'); + if (!(await badge.isVisible().catch(() => false))) test.skip(true, 'details modal did not open'); + + const dialog = page.locator('[role="dialog"][aria-modal="true"]'); + await expect(dialog, 'exposed as a modal dialog').toBeVisible(); + await expect(dialog, 'has an accessible name').toHaveAttribute('aria-label', /.+/); + expect(await tabStaysTrapped(page), 'Tab focus stays within the details modal').toBe(true); + }); + + test('create-graph modal: role=dialog and focus trapped', async ({ page }) => { + await openWorkspace(page, 'cards'); + const sel = page.locator('[data-testid="graph-selector"]'); + let trigger = null as any; + for (let i = 0; i < (await sel.count()); i++) { + if (await sel.nth(i).isVisible().catch(() => false)) { trigger = sel.nth(i); break; } + } + if (!trigger) test.skip(true, 'no graph selector'); + await trigger.click(); + await page.waitForTimeout(400); + const create = page.locator('[title="Create New Graph"]').first(); + if (!(await create.isVisible().catch(() => false))) test.skip(true, 'no create-graph affordance'); + await create.click(); + await page.waitForTimeout(700); + + const dialog = page.locator('[role="dialog"][aria-modal="true"]'); + await expect(dialog, 'create-graph exposed as a modal dialog').toBeVisible(); + expect(await tabStaysTrapped(page), 'Tab focus stays within the create-graph modal').toBe(true); + }); + }); + + test.describe('phone', () => { + test.use({ viewport: { width: 390, height: 844 } }); + + test('create-work-item modal: focus enters, is trapped, and restores to the trigger', async ({ page }) => { + await openWorkspace(page, 'cards'); + const fab = page.locator('[aria-label="New work item"]'); + if (!(await fab.isVisible().catch(() => false))) test.skip(true, 'no create FAB'); + await fab.click(); + await page.waitForTimeout(1000); + + const dialog = page.locator('[role="dialog"][aria-modal="true"]'); + await expect(dialog, 'create-work-item exposed as a modal dialog').toBeVisible(); + // Focus moved into the modal on open. + expect(await focusWithinDialog(page), 'focus moved into the modal on open').toBe(true); + // Tab is trapped within it. + expect(await tabStaysTrapped(page), 'Tab focus stays within the create modal').toBe(true); + + // Close via the backdrop (the dialog-manager defers Escape while a text field + // is focused, by design). Focus-restore fires regardless of how it closes. + await page.mouse.click(5, 5); + await page.waitForTimeout(600); + await expect(dialog, 'modal closed').toHaveCount(0); + const restored = await page.evaluate( + () => document.activeElement?.getAttribute('aria-label') === 'New work item' + ); + expect(restored, 'focus restored to the New-work-item trigger').toBe(true); + }); + }); +}); From fd524cd87f8d02b30fb35fe030b02622cbb5da8b Mon Sep 17 00:00:00 2001 From: Matthew Valancy Date: Wed, 17 Jun 2026 03:42:27 -0700 Subject: [PATCH 2/2] refactor(a11y): harden useModalA11y + make the trap test prove the trap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review fixes: - useModalA11y: prefer the first form field for initial focus (not the icon-only Close button); restore focus only when it dropped to so a deliberate post-close focus move isn't overridden; sharper isVisible (getClientRects + computed visibility); honest docstring (Tab-trap + aria-modal hint, not a background-inert); include [contenteditable] in the focusable set. - CreateWorkItemModal / CreateGraphModal: aria-label="Close" on the X buttons. - WorkItemDetailsModal: drop the bespoke capture-phase Escape handler so Escape is owned by the dialog-manager (defers while typing, keeps the stack coherent); keep only Ctrl/Cmd+S. - a11y-focus.spec: test the trap AT THE BOUNDARY (Tab from the last focusable must wrap inside; Shift+Tab from the first) — the previous fixed 14 Tabs never reached the edge on modals with many focusables, so it passed even without the trap. Restore test now focuses the trigger first and asserts focus left it while open. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../web/src/components/CreateGraphModal.tsx | 1 + .../src/components/CreateWorkItemModal.tsx | 1 + .../src/components/WorkItemDetailsModal.tsx | 9 ++-- packages/web/src/hooks/useModalA11y.ts | 42 +++++++++++----- tests/e2e/a11y-focus.spec.ts | 49 ++++++++++++------- 5 files changed, 67 insertions(+), 35 deletions(-) diff --git a/packages/web/src/components/CreateGraphModal.tsx b/packages/web/src/components/CreateGraphModal.tsx index 8fecd862..b6ea98e9 100644 --- a/packages/web/src/components/CreateGraphModal.tsx +++ b/packages/web/src/components/CreateGraphModal.tsx @@ -249,6 +249,7 @@ export function CreateGraphModal({ isOpen, onClose, parentGraphId }: CreateGraph