refactor: shared Modal primitive + replace native dialogs with in-app feedback

Behavior-preserving consolidation of dialog accessibility and feedback. No modal
field/validation/save changes; YAML output unchanged (golden baselines byte-identical).

- Add a shared <Modal> primitive (src/components/Modal/) owning ESC close, optional
  overlay-click close, body-scroll lock, focus trap, and focus return to the opener;
  renders nothing when closed. Add ConfirmDialog built on it. Honors prefers-reduced
  -motion; unique per-instance ids via useId().
- Migrate CameraModal and ElectrodeGroupModal onto Modal: lift each form into a child
  rendered only while open, so initial state computes once (fixes the prior unstable
  init effect). ElectrodeGroupModal now gains a focus trap + focus return it never had,
  drops the interim trap, and sources device types from the single canonical
  valueList.deviceTypes() (was a hardcoded list missing one entry).
- Rebase AlertModal onto Modal so it inherits the trap/focus-return; keep its public
  props and visual classes.
- Replace all blocking native dialogs in the new UI: window.confirm() -> ConfirmDialog
  (electrode-group delete, behavioral-event delete); alert() -> AlertModal with titles
  (navigating success alerts defer navigation to dismissal) and inline role=alert
  regions (channel-map validation errors, calendar day-creation failures). Legacy form
  left untouched.
- Correct DataAcqSection's misleading "debounced" docstring (no behavior change).
- Tests: Modal, ConfirmDialog, migrated-modal trap/return, device-source parity, the
  delete/alert dialog flows, and a source-scan test asserting zero native dialogs in
  the new-UI surfaces.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: edeno

.claude/docs/plans/v3-workspace-cutover/PLAN.md

@@ -1,6 +1,6 @@
 # v3.0.0 Workspace Cutover — Implementation Plan
 
-**Status:** Phases 0–2 complete (merged to `modern`). Phase 3 next.
+**Status:** Phases 0–3 complete (merged to `modern`). Phase 4 next.
 
 Carries the `modern` branch from its mid-build state (M8a complete; new multi-page workspace exists
 but cannot export YAML and loses data on refresh) through to **v3.0.0**, where the workspace UI is a

.claude/docs/plans/v3-workspace-cutover/overview.md

@@ -159,6 +159,16 @@ not executed here.
   navigation race in `src/pages/Home/index.jsx` and the stale-`days`-snapshot guard in batch day
   creation in `src/pages/AnimalWorkspace/index.jsx` are fixed in
   [Phase 2](phase-2-navigation-stub-honesty.md), not silently dropped.
+- **Remaining dialogs not yet on the shared `<Modal>` primitive.** Phase 3 created the primitive and
+  migrated `CameraModal`, `ElectrodeGroupModal`, and `AlertModal`. Three other overlay surfaces still
+  use bespoke markup and lack the full dialog a11y contract (focus trap / focus return / ESC / proper
+  ARIA): `src/pages/AnimalEditor/ChannelMapEditor.jsx`, `src/pages/AnimalEditor/CopyFromAnimalDialog.jsx`
+  (uses `<dialog open>` without `showModal()`), and `src/components/CalendarDayCreator/CalendarDayCreator.jsx`
+  (inline card carrying `role="dialog"`). These were **out of scope for Phase 3** (its task list named
+  only the three migrated modals). **Follow-up:** migrate them onto `<Modal>` (or fix in place) — a
+  natural fit for the [Phase 9](phase-9-a11y-keyboard.md) accessibility pass, which should also sweep
+  the pre-existing color-contrast issues (inline-warning text, channel-map select focus rings, calendar
+  day-number contrast) flagged in the Phase 3 UX review.
 
 ## Estimated Effort
 

src/__tests__/unit/no-native-dialogs.test.js

@@ -0,0 +1,72 @@
+import { describe, it, expect } from 'vitest';
+import fs from 'fs';
+import path from 'path';
+import { fileURLToPath } from 'url';
+
+const __filename = fileURLToPath(import.meta.url);
+const __dirname = path.dirname(__filename);
+const SRC = path.resolve(__dirname, '../..');
+
+/**
+ * Directories whose new-UI surfaces must use the in-app Modal/AlertModal/ConfirmDialog
+ * components instead of blocking native dialogs.
+ */
+const SCOPED_DIRS = [
+  path.join(SRC, 'pages'),
+  path.join(SRC, 'components', 'CalendarDayCreator'),
+];
+
+// The legacy form is the frozen safety net and is intentionally exempt.
+const EXEMPT = [path.join(SRC, 'pages', 'LegacyFormView.jsx')];
+
+/** Native blocking-dialog calls: bare alert()/confirm() or window.alert/confirm(). */
+const NATIVE_DIALOG = /(^|[^.\w])(alert|confirm)\s*\(|window\.(alert|confirm)\s*\(/;
+
+/**
+ * Recursively collect .js/.jsx source files (excluding tests) under a directory.
+ * @param {string} dir Directory to walk.
+ * @returns {string[]} Absolute file paths.
+ */
+function collectSourceFiles(dir) {
+  if (!fs.existsSync(dir)) return [];
+  return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
+    const full = path.join(dir, entry.name);
+    if (entry.isDirectory()) {
+      if (entry.name === '__tests__') return [];
+      return collectSourceFiles(full);
+    }
+    if (!/\.(js|jsx)$/.test(entry.name)) return [];
+    if (/\.(test|spec)\.(js|jsx)$/.test(entry.name)) return [];
+    return [full];
+  });
+}
+
+/**
+ * Find native-dialog call sites in a file, ignoring comment lines/segments so that
+ * doc comments mentioning "alert()" or "window.confirm()" don't trip the scan.
+ * @param {string} file Absolute file path.
+ * @returns {string[]} Offending "relpath:line" entries.
+ */
+function findNativeDialogs(file) {
+  const offenders = [];
+  const lines = fs.readFileSync(file, 'utf-8').split('\n');
+  lines.forEach((line, i) => {
+    const trimmed = line.trim();
+    if (trimmed.startsWith('//') || trimmed.startsWith('*') || trimmed.startsWith('/*')) return;
+    const code = line.split('//')[0]; // drop trailing line comments
+    if (NATIVE_DIALOG.test(code)) {
+      offenders.push(`${path.relative(SRC, file)}:${i + 1}`);
+    }
+  });
+  return offenders;
+}
+
+describe('no native dialogs in the new UI', () => {
+  it('uses Modal/AlertModal/ConfirmDialog instead of alert()/window.confirm()', () => {
+    const offenders = SCOPED_DIRS.flatMap(collectSourceFiles)
+      .filter((file) => !EXEMPT.includes(file))
+      .flatMap(findNativeDialogs);
+
+    expect(offenders, `Native dialog calls found:\n${offenders.join('\n')}`).toEqual([]);
+  });
+});

src/components/AlertModal.jsx

@@ -1,5 +1,6 @@
-import React, { useEffect, useRef } from 'react';
+import React, { useId } from 'react';
 import PropTypes from 'prop-types';
+import Modal from './Modal/Modal';
 import './AlertModal.scss';
 
 /**
@@ -24,54 +25,10 @@ import './AlertModal.scss';
  * @returns {JSX.Element|null} Modal component or null if closed
  */
 const AlertModal = ({ isOpen, message, title = 'Alert', onClose, type = 'info' }) => {
-  const closeButtonRef = useRef(null);
-
-  // Handle ESC key
-  useEffect(() => {
-    if (!isOpen) return;
-
-    const handleEscape = (e) => {
-      if (e.key === 'Escape') {
-        onClose();
-      }
-    };
-
-    document.addEventListener('keydown', handleEscape);
-    return () => document.removeEventListener('keydown', handleEscape);
-  }, [isOpen, onClose]);
-
-  // Focus close button when modal opens
-  useEffect(() => {
-    if (isOpen && closeButtonRef.current) {
-      closeButtonRef.current.focus();
-    }
-  }, [isOpen]);
-
-  // Lock body scroll when modal is open
-  useEffect(() => {
-    if (isOpen) {
-      document.body.style.overflow = 'hidden';
-    } else {
-      document.body.style.overflow = '';
-    }
-
-    return () => {
-      document.body.style.overflow = '';
-    };
-  }, [isOpen]);
-
-  // Handle click on overlay (outside modal)
-  const handleOverlayClick = (e) => {
-    if (e.target.classList.contains('alert-modal-overlay')) {
-      onClose();
-    }
-  };
-
-  // Don't render if not open
-  if (!isOpen) return null;
-
-  const titleId = 'alert-modal-title';
-  const messageId = 'alert-modal-message';
+  // Unique per instance so multiple alerts in the tree can't collide on ids.
+  const baseId = useId();
+  const titleId = `${baseId}-alert-title`;
+  const messageId = `${baseId}-alert-message`;
 
   // Icon mapping for accessibility (not relying on color alone)
   const iconMap = {
@@ -81,39 +38,33 @@ const AlertModal = ({ isOpen, message, title = 'Alert', onClose, type = 'info' }
     error: '❌',
   };
 
+  // Built on the shared Modal primitive, which provides the focus trap, focus
+  // return, ESC/overlay close, and scroll lock. AlertModal keeps its own visual
+  // classes (alert-modal-content + type variant, message, close button).
   return (
-    <div
-      className="alert-modal-overlay"
-      onClick={handleOverlayClick}
-      role="presentation"
-    >
-      <div
-        className={`alert-modal-content alert-modal-${type}`}
-        role="alertdialog"
-        aria-modal="true"
-        aria-labelledby={titleId}
-        aria-describedby={messageId}
-        onClick={(e) => e.stopPropagation()} // Prevent overlay click when clicking content
-      >
-        <h2 id={titleId} className="alert-modal-title">
+    <Modal
+      isOpen={isOpen}
+      onClose={onClose}
+      role="alertdialog"
+      titleId={titleId}
+      describedById={messageId}
+      className={`alert-modal-content alert-modal-${type}`}
+      title={
+        <>
           <span className="alert-modal-icon" aria-hidden="true">
             {iconMap[type]}
           </span>
           {title}
-        </h2>
-        <p id={messageId} className="alert-modal-message">
-          {message}
-        </p>
-        <button
-          ref={closeButtonRef}
-          onClick={onClose}
-          className="alert-modal-close"
-          aria-label="Close alert"
-        >
-          Close
-        </button>
-      </div>
-    </div>
+        </>
+      }
+    >
+      <p id={messageId} className="alert-modal-message">
+        {message}
+      </p>
+      <button type="button" onClick={onClose} className="alert-modal-close" aria-label="Close alert">
+        Close
+      </button>
+    </Modal>
   );
 };
 

src/components/CalendarDayCreator/CalendarDayCreator.css

@@ -313,3 +313,13 @@
     gap: 0.5rem;
   }
 }
+
+/* Inline error shown when day creation fails (replaces a blocking dialog). */
+.calendar-create-error {
+  margin: 0.75rem 0;
+  padding: 0.75rem 1rem;
+  background-color: #ffebee;
+  color: #b71c1c;
+  border-left: 4px solid #d32f2f;
+  border-radius: 4px;
+}

src/components/CalendarDayCreator/CalendarDayCreator.jsx

@@ -88,6 +88,8 @@ export function CalendarDayCreator({ animalId, existingDays = [], onCreateDays,
 
   // Selected dates (Set for O(1) lookup)
   const [selectedDates, setSelectedDates] = useState(new Set());
+  // Inline error shown if day creation fails (replaces a blocking alert()).
+  const [createError, setCreateError] = useState(null);
 
   /**
    * Navigate to previous month
@@ -173,6 +175,7 @@ export function CalendarDayCreator({ animalId, existingDays = [], onCreateDays,
     const dates = Array.from(selectedDates).sort();
 
     try {
+      setCreateError(null);
       // Call parent callback with all dates
       await onCreateDays(dates);
 
@@ -181,7 +184,7 @@ export function CalendarDayCreator({ animalId, existingDays = [], onCreateDays,
       if (onClose) onClose();
     } catch (error) {
       console.error('Failed to create days:', error);
-      alert(`Failed to create days: ${error.message}`);
+      setCreateError(`Failed to create days: ${error.message}`);
     }
   }, [selectedDates, onCreateDays, onClose]);
 
@@ -203,6 +206,12 @@ export function CalendarDayCreator({ animalId, existingDays = [], onCreateDays,
 
       <CalendarLegend />
 
+      {createError && (
+        <div className="calendar-create-error" role="alert">
+          {createError}
+        </div>
+      )}
+
       <div className="calendar-actions">
         <button
           type="button"

src/components/CalendarDayCreator/__tests__/CalendarDayCreator.test.jsx

@@ -341,7 +341,6 @@ describe('CalendarDayCreator Component', () => {
     it('handles creation errors', async () => {
       const user = userEvent.setup();
       const onCreateDays = vi.fn().mockRejectedValue(new Error('Creation failed'));
-      const alertSpy = vi.spyOn(window, 'alert').mockImplementation(() => {});
 
       render(
         <CalendarDayCreator
@@ -368,13 +367,11 @@ describe('CalendarDayCreator Component', () => {
         const createButton = screen.getByRole('button', { name: /create 1 recording day/i });
         await user.click(createButton);
 
-        // Verify error alert shown
+        // The failure is shown inline (role=alert), not via a native alert().
         await waitFor(() => {
-          expect(alertSpy).toHaveBeenCalledWith(expect.stringContaining('Failed to create days'));
+          expect(screen.getByRole('alert')).toHaveTextContent(/Failed to create days/i);
         });
       }
-
-      alertSpy.mockRestore();
     });
   });
 

src/components/Modal/ConfirmDialog.jsx

@@ -0,0 +1,76 @@
+import React, { useId } from 'react';
+import PropTypes from 'prop-types';
+import Modal from './Modal';
+import './ConfirmDialog.scss';
+
+/**
+ * Accessible confirm/cancel dialog built on the shared Modal primitive. Replaces
+ * blocking window.confirm() prompts. Cancel, ESC, and overlay click all invoke
+ * onCancel; only the Confirm button invokes onConfirm.
+ *
+ * @param {object} props
+ * @param {boolean} props.isOpen Whether the dialog is shown.
+ * @param {string} props.title Dialog heading.
+ * @param {React.ReactNode} props.message Body / question.
+ * @param {string} [props.confirmLabel] Confirm button text.
+ * @param {string} [props.cancelLabel] Cancel button text.
+ * @param {boolean} [props.destructive] Style the confirm action as destructive.
+ * @param {Function} props.onConfirm Called when the user confirms.
+ * @param {Function} props.onCancel Called for cancel / ESC / overlay close.
+ * @returns {JSX.Element|null}
+ */
+const ConfirmDialog = ({
+  isOpen,
+  title,
+  message,
+  confirmLabel = 'Confirm',
+  cancelLabel = 'Cancel',
+  destructive = false,
+  onConfirm,
+  onCancel,
+}) => {
+  // Unique per instance so multiple dialogs in the tree can't collide on ids.
+  const baseId = useId();
+  const titleId = `${baseId}-title`;
+  const messageId = `${baseId}-message`;
+
+  return (
+    <Modal
+      isOpen={isOpen}
+      onClose={onCancel}
+      title={title}
+      titleId={titleId}
+      describedById={messageId}
+      className="confirm-dialog"
+    >
+      <p id={messageId} className="confirm-dialog-message">
+        {message}
+      </p>
+      <div className="form-actions">
+        <button type="button" className="btn-cancel" onClick={onCancel}>
+          {cancelLabel}
+        </button>
+        <button
+          type="button"
+          className={destructive ? 'btn-danger' : 'btn-save'}
+          onClick={onConfirm}
+        >
+          {confirmLabel}
+        </button>
+      </div>
+    </Modal>
+  );
+};
+
+ConfirmDialog.propTypes = {
+  isOpen: PropTypes.bool.isRequired,
+  title: PropTypes.node.isRequired,
+  message: PropTypes.node.isRequired,
+  confirmLabel: PropTypes.string,
+  cancelLabel: PropTypes.string,
+  destructive: PropTypes.bool,
+  onConfirm: PropTypes.func.isRequired,
+  onCancel: PropTypes.func.isRequired,
+};
+
+export default ConfirmDialog;