From 45691918a9b6ea8fde7ffe34a9f4791e18077d2d Mon Sep 17 00:00:00 2001
From: "Yoshiaki Ueda (bootjp)" <contact@bootjp.me>
Date: Sun, 26 Apr 2026 18:17:10 +0900
Subject: [PATCH 1/2] fix(admin/spa): drop redundant abort guard and add Modal
 a11y
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two follow-ups from the review of #649:

1. useApi.ts: removed the `if (ctrl.signal.aborted) return;` check
   inside the catch handler. The cleanup function sets
   `cancelled = true` *before* calling `ctrl.abort()`, so any abort
   path is already covered by the preceding `cancelled` check. The
   second guard was harmless belt-and-suspenders dead code; the
   comment now records why one check is enough.

2. Modal.tsx: added the WAI-ARIA dialog contract (`role="dialog"`,
   `aria-modal="true"`, `aria-labelledby` pointing at the title)
   and a focus trap:
   - On open: store the previously-focused element, then focus the
     first focusable inside the dialog so keyboard users do not stay
     stranded on the page underneath. Empty dialogs fall back to
     focusing the dialog container itself with tabindex=-1.
   - Tab / Shift+Tab cycle within the dialog instead of escaping to
     the page underneath.
   - On close: restore focus to whoever opened the dialog (skipped
     when that element has been unmounted, e.g. the dialog deleted
     the row whose button opened it).
   - Escape and backdrop click stay wired through the existing
     `busy` gate so a half-applied operation cannot be dismissed.

   Implementation is vanilla DOM querying — focus-trap-react /
   react-focus-lock would have added 5–7 KB gzip for what is a
   ~40 line vanilla solution. Bundle delta from this PR is +0.4 KB
   gzip (60.23 -> 60.59 KB).

Verified with `npm run build` and `npm run lint` (tsc --strict
--noUnusedLocals --noUncheckedSideEffectImports), and `go build ./...`.

Stacked on top of #649 (feat/admin-dashboard-spa); merge that one
first, then this rebases cleanly onto main.
---
 web/admin/src/components/Modal.tsx | 103 +++++++++++++++++++++++++++--
 web/admin/src/lib/useApi.ts        |   4 +-
 2 files changed, 101 insertions(+), 6 deletions(-)

diff --git a/web/admin/src/components/Modal.tsx b/web/admin/src/components/Modal.tsx
index d83bef603..f5cad1269 100644
--- a/web/admin/src/components/Modal.tsx
+++ b/web/admin/src/components/Modal.tsx
@@ -1,4 +1,4 @@
-import { useEffect } from "react";
+import { useEffect, useId, useRef } from "react";
 import type { ReactNode } from "react";
 
 interface ModalProps {
@@ -15,14 +15,69 @@ interface ModalProps {
 // Lightweight modal: no portal, no animation library. shadcn/ui's
 // Dialog primitive would pull in @radix-ui/react-dialog (~10KB gzip)
 // and we only need a single confirmation/edit surface per page.
+//
+// Accessibility (per the WAI-ARIA Authoring Practices for dialogs):
+//   - role="dialog" + aria-modal="true" so AT announce the dialog
+//     and treat the rest of the page as inert.
+//   - aria-labelledby on the title <div> so the dialog is named.
+//   - Focus is moved into the dialog on open and restored to the
+//     previously-focused element on close.
+//   - Tab and Shift+Tab are wrapped to keep focus inside the dialog
+//     until it closes, so keyboard users cannot accidentally tab to
+//     the page underneath.
 export function Modal({ title, open, onClose, children, busy }: ModalProps) {
+  const dialogRef = useRef<HTMLDivElement>(null);
+  const previouslyFocusedRef = useRef<HTMLElement | null>(null);
+  const titleId = useId();
+
   useEffect(() => {
     if (!open) return;
+
+    previouslyFocusedRef.current = (document.activeElement as HTMLElement | null) ?? null;
+    // Focus the first focusable element so keyboard users land
+    // inside the dialog instead of the page underneath. Run on the
+    // next tick so the dialog DOM exists and any autofocus has had
+    // a chance to settle.
+    queueMicrotask(() => focusFirstFocusable(dialogRef.current));
+
     const onKey = (e: KeyboardEvent) => {
-      if (e.key === "Escape" && !busy) onClose();
+      if (e.key === "Escape" && !busy) {
+        onClose();
+        return;
+      }
+      if (e.key !== "Tab") return;
+      const root = dialogRef.current;
+      if (!root) return;
+      const focusables = focusableElements(root);
+      if (focusables.length === 0) {
+        // Empty dialog: keep Tab from leaving via the page.
+        e.preventDefault();
+        return;
+      }
+      const first = focusables[0];
+      const last = focusables[focusables.length - 1];
+      const active = document.activeElement;
+      if (e.shiftKey && active === first) {
+        e.preventDefault();
+        last.focus();
+      } else if (!e.shiftKey && active === last) {
+        e.preventDefault();
+        first.focus();
+      }
     };
+
     window.addEventListener("keydown", onKey);
-    return () => window.removeEventListener("keydown", onKey);
+    return () => {
+      window.removeEventListener("keydown", onKey);
+      // Restore focus to whoever opened the dialog. Guard against
+      // the trigger being unmounted (e.g. the dialog deleted the
+      // row whose button opened it) by checking isConnected.
+      const restore = previouslyFocusedRef.current;
+      previouslyFocusedRef.current = null;
+      if (restore && restore.isConnected) {
+        restore.focus();
+      }
+    };
   }, [open, busy, onClose]);
 
   if (!open) return null;
@@ -33,9 +88,15 @@ export function Modal({ title, open, onClose, children, busy }: ModalProps) {
         if (e.target === e.currentTarget && !busy) onClose();
       }}
     >
-      <div className="w-full max-w-md rounded-lg border border-border bg-surface shadow-xl">
+      <div
+        ref={dialogRef}
+        role="dialog"
+        aria-modal="true"
+        aria-labelledby={titleId}
+        className="w-full max-w-md rounded-lg border border-border bg-surface shadow-xl"
+      >
         <div className="border-b border-border px-4 py-3 flex items-center">
-          <div className="font-semibold text-sm">{title}</div>
+          <div id={titleId} className="font-semibold text-sm">{title}</div>
           <button
             type="button"
             onClick={onClose}
@@ -51,3 +112,35 @@ export function Modal({ title, open, onClose, children, busy }: ModalProps) {
     </div>
   );
 }
+
+// focusableSelector targets the elements an end user can tab to.
+// Excludes [tabindex="-1"] (programmatic-only focus targets) and
+// disabled / hidden inputs. Kept narrow on purpose: the modal only
+// hosts buttons / form fields / links, not embedded media or
+// content-editable surfaces.
+const focusableSelector = [
+  "a[href]",
+  "button:not([disabled])",
+  "input:not([disabled]):not([type='hidden'])",
+  "select:not([disabled])",
+  "textarea:not([disabled])",
+  '[tabindex]:not([tabindex="-1"])',
+].join(",");
+
+function focusableElements(root: HTMLElement): HTMLElement[] {
+  return Array.from(root.querySelectorAll<HTMLElement>(focusableSelector));
+}
+
+function focusFirstFocusable(root: HTMLElement | null): void {
+  if (!root) return;
+  const focusables = focusableElements(root);
+  if (focusables.length > 0) {
+    focusables[0].focus();
+    return;
+  }
+  // Fallback: focus the dialog container itself so screen readers
+  // still announce it. Add tabindex=-1 dynamically so the browser
+  // accepts the focus call without making the dialog tab-stop bait.
+  root.setAttribute("tabindex", "-1");
+  root.focus();
+}
diff --git a/web/admin/src/lib/useApi.ts b/web/admin/src/lib/useApi.ts
index 741edf165..369152e9b 100644
--- a/web/admin/src/lib/useApi.ts
+++ b/web/admin/src/lib/useApi.ts
@@ -44,8 +44,10 @@ export function useApiQuery<T>(
         setLoading(false);
       })
       .catch((err: unknown) => {
+        // The cleanup function (below) sets `cancelled = true` *before*
+        // calling `ctrl.abort()`, so any abort path is already covered
+        // by the `cancelled` check above. No second guard needed.
         if (cancelled) return;
-        if (ctrl.signal.aborted) return;
         if (err instanceof ApiError) {
           if (err.status === 401) markUnauthorizedRef.current();
           setError(err);

From 086480c790e9ee83063e83f2f7a7f61744906006 Mon Sep 17 00:00:00 2001
From: "Yoshiaki Ueda (bootjp)" <contact@bootjp.me>
Date: Sun, 26 Apr 2026 18:35:58 +0900
Subject: [PATCH 2/2] fix(admin/spa): split Modal effect to stop focus flicker
 on busy toggle
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Claude review on b36cfed7 caught a real focus-management bug: the
single useEffect in Modal.tsx mixed two invariants — focus
capture/restore (open) and the keyboard handler (open + busy +
onClose) — into one effect with deps [open, busy, onClose].

Concrete failure: user clicks Save, parent flips busy=true. React
runs the cleanup, which removes the keydown listener AND restores
focus to the previously-focused element (the trigger button, outside
the dialog). The effect then re-runs, captures the trigger button
as the NEW restore target, and snaps focus back. Net: focus
flickers out and back on every busy toggle, previouslyFocusedRef
gets clobbered with the wrong element, and screen readers announce
the dialog as briefly "exited" mid-operation.

Fix: split into two effects.
- Focus capture/restore — deps [open] only. Runs once on open, once
  on close. previouslyFocusedRef stays valid for the whole dialog
  lifetime.
- Keyboard handler — deps [open, busy, onClose]. Re-binds when
  any of those change; only observable side effect is one window
  listener swap.

All three automated reviewers (Claude, Gemini, Codex) flagged this.
The split is the standard React idiom — each invariant owns its own
state slice.

The dist/ placeholder index.html is intentionally NOT regenerated
(per .gitignore: assets are built at deploy time, not committed).
---
 web/admin/src/components/Modal.tsx | 48 +++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/web/admin/src/components/Modal.tsx b/web/admin/src/components/Modal.tsx
index f5cad1269..9f78eb676 100644
--- a/web/admin/src/components/Modal.tsx
+++ b/web/admin/src/components/Modal.tsx
@@ -30,16 +30,41 @@ export function Modal({ title, open, onClose, children, busy }: ModalProps) {
   const previouslyFocusedRef = useRef<HTMLElement | null>(null);
   const titleId = useId();
 
+  // Focus capture / restore is tied to `open` ONLY. Folding `busy`
+  // into the same effect as the previous version did caused the
+  // cleanup to fire on every busy toggle (e.g. the user clicks Save
+  // and the parent flips busy=true): focus would briefly leave the
+  // dialog, previouslyFocusedRef would get clobbered with the
+  // trigger button, then the next run would snap focus back. Real
+  // bug for screen-reader users — the dialog became "exited" mid-
+  // operation. Splitting the effects keeps each invariant tied to
+  // its own state slice (Claude review on #650).
   useEffect(() => {
     if (!open) return;
-
     previouslyFocusedRef.current = (document.activeElement as HTMLElement | null) ?? null;
-    // Focus the first focusable element so keyboard users land
-    // inside the dialog instead of the page underneath. Run on the
-    // next tick so the dialog DOM exists and any autofocus has had
-    // a chance to settle.
+    // queueMicrotask defers focus until after sibling useEffect
+    // hooks (notably any autofocus on form fields inside the
+    // dialog children) have settled, so we focus the truly-first
+    // tab stop rather than racing with autofocus.
     queueMicrotask(() => focusFirstFocusable(dialogRef.current));
+    return () => {
+      // Restore focus to whoever opened the dialog. Guard against
+      // the trigger being unmounted (e.g. the dialog deleted the
+      // row whose button opened it) by checking isConnected.
+      const restore = previouslyFocusedRef.current;
+      previouslyFocusedRef.current = null;
+      if (restore && restore.isConnected) {
+        restore.focus();
+      }
+    };
+  }, [open]);
 
+  // Keyboard handler legitimately needs `busy` (Esc gates on it) and
+  // `onClose`, so it re-binds when either changes. Re-binding is
+  // cheap — one window listener swap per change — and unlike the
+  // focus effect it has no observable side effect on the user.
+  useEffect(() => {
+    if (!open) return;
     const onKey = (e: KeyboardEvent) => {
       if (e.key === "Escape" && !busy) {
         onClose();
@@ -65,19 +90,8 @@ export function Modal({ title, open, onClose, children, busy }: ModalProps) {
         first.focus();
       }
     };
-
     window.addEventListener("keydown", onKey);
-    return () => {
-      window.removeEventListener("keydown", onKey);
-      // Restore focus to whoever opened the dialog. Guard against
-      // the trigger being unmounted (e.g. the dialog deleted the
-      // row whose button opened it) by checking isConnected.
-      const restore = previouslyFocusedRef.current;
-      previouslyFocusedRef.current = null;
-      if (restore && restore.isConnected) {
-        restore.focus();
-      }
-    };
+    return () => window.removeEventListener("keydown", onKey);
   }, [open, busy, onClose]);
 
   if (!open) return null;