From 332cf5e2cc86806cc1e08d5b8dd1044f7f71d66b Mon Sep 17 00:00:00 2001
From: cavidelizade <elizadecavidnice@gmail.com>
Date: Fri, 17 Jul 2026 01:27:21 +0400
Subject: [PATCH] fix(ui): add focus management to the shared Modal

The shared Modal had dialog roles and Escape, but it never moved focus into the
dialog, didn't trap Tab (so keyboard focus wandered onto the page behind it),
and didn't return focus to the trigger on close. Since most modals reuse this
primitive, that gap repeated everywhere.

Move focus to the first field on open, trap Tab/Shift+Tab within the dialog,
restore focus to the previously focused element on close, and give the title a
unique id for aria-labelledby (instead of a shared static one).

Closes #347

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 apps/web/src/components/ui/Modal.tsx | 59 ++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 8 deletions(-)

diff --git a/apps/web/src/components/ui/Modal.tsx b/apps/web/src/components/ui/Modal.tsx
index de42ec47..ed0c87e5 100644
--- a/apps/web/src/components/ui/Modal.tsx
+++ b/apps/web/src/components/ui/Modal.tsx
@@ -1,4 +1,4 @@
-import { useEffect, type ReactNode } from 'react';
+import { useEffect, useId, useRef, type ReactNode } from 'react';
 import { createPortal } from 'react-dom';
 import { cn } from '../../lib/utils';
 
@@ -11,17 +11,58 @@ export interface ModalProps {
   className?: string;
 }
 
+const FOCUSABLE_SELECTOR =
+  'a[href], button:not([disabled]), textarea:not([disabled]), input:not([disabled]), select:not([disabled]), [tabindex]:not([tabindex="-1"])';
+
 export function Modal({ open, onClose, title, children, footer, className }: ModalProps) {
+  const titleId = useId();
+  const dialogRef = useRef<HTMLDivElement>(null);
+
   useEffect(() => {
     if (!open) return;
-    const handleEscape = (e: KeyboardEvent) => {
-      if (e.key === 'Escape') onClose();
+    // Remember what had focus so we can restore it when the dialog closes.
+    const previouslyFocused = document.activeElement as HTMLElement | null;
+
+    const handleKeyDown = (e: KeyboardEvent) => {
+      if (e.key === 'Escape') {
+        onClose();
+        return;
+      }
+      if (e.key !== 'Tab') return;
+      // Trap focus inside the dialog so Tab can't reach the page behind it.
+      const root = dialogRef.current;
+      if (!root) return;
+      const focusable = Array.from(root.querySelectorAll<HTMLElement>(FOCUSABLE_SELECTOR)).filter(
+        (el) => el.offsetParent !== null,
+      );
+      if (focusable.length === 0) {
+        e.preventDefault();
+        root.focus();
+        return;
+      }
+      const first = focusable[0]!;
+      const last = focusable[focusable.length - 1]!;
+      const active = document.activeElement;
+      if (e.shiftKey && (active === first || !root.contains(active))) {
+        e.preventDefault();
+        last.focus();
+      } else if (!e.shiftKey && active === last) {
+        e.preventDefault();
+        first.focus();
+      }
     };
-    document.addEventListener('keydown', handleEscape);
+
+    document.addEventListener('keydown', handleKeyDown);
     document.body.style.overflow = 'hidden';
+
+    // Move focus into the dialog (first field, or the dialog itself).
+    const root = dialogRef.current;
+    (root?.querySelector<HTMLElement>(FOCUSABLE_SELECTOR) ?? root)?.focus();
+
     return () => {
-      document.removeEventListener('keydown', handleEscape);
+      document.removeEventListener('keydown', handleKeyDown);
       document.body.style.overflow = '';
+      previouslyFocused?.focus?.();
     };
   }, [open, onClose]);
 
@@ -32,18 +73,20 @@ export function Modal({ open, onClose, title, children, footer, className }: Mod
       className="fixed inset-0 z-10050 flex items-center justify-center p-4"
       role="dialog"
       aria-modal="true"
-      aria-labelledby="modal-title"
+      aria-labelledby={titleId}
     >
       <div className="absolute inset-0 bg-(--bg-backdrop)" onClick={onClose} aria-hidden />
       <div
+        ref={dialogRef}
+        tabIndex={-1}
         className={cn(
-          'relative z-10 w-full max-w-md rounded-(--radius-lg) border border-(--border-subtle) bg-(--bg-surface-1) shadow-(--shadow-overlay)',
+          'relative z-10 w-full max-w-md rounded-(--radius-lg) border border-(--border-subtle) bg-(--bg-surface-1) shadow-(--shadow-overlay) outline-none',
           className,
         )}
         onClick={(e) => e.stopPropagation()}
       >
         <div className="border-b border-(--border-subtle) px-5 py-4">
-          <h2 id="modal-title" className="text-lg font-semibold text-(--txt-primary)">
+          <h2 id={titleId} className="text-lg font-semibold text-(--txt-primary)">
             {title}
           </h2>
         </div>