fix(admin/spa): split Modal effect to stop focus flicker on busy toggle

Claude review on b36cfed 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).

Author: bootjp

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;