diff --git a/craft/src/panels/sitesmith/SitesmithModal.tsx b/craft/src/panels/sitesmith/SitesmithModal.tsx index 6f3ea80..6bb9ac7 100644 --- a/craft/src/panels/sitesmith/SitesmithModal.tsx +++ b/craft/src/panels/sitesmith/SitesmithModal.tsx @@ -11,6 +11,7 @@ import { MessageList } from './MessageList'; import { ChatInput } from './ChatInput'; import { WorkingIndicator } from './WorkingIndicator'; import { SitesmithResponse } from '../../types/sitesmith'; +import { Modal } from '../../ui/Modal'; interface Props { onClose: () => void; @@ -39,7 +40,6 @@ export const SitesmithModal: React.FC = ({ onClose, target }) => { const canChat = summary && (summary.status === 'OK_BONUS' || summary.status === 'OK_MONTHLY'); - const overlay: React.CSSProperties = { position: 'fixed', inset: 0, background: 'rgba(0,0,0,0.8)', zIndex: 9000, display: 'flex', alignItems: 'center', justifyContent: 'center' }; const panel: React.CSSProperties = { background: '#0f0f17', border: '1px solid #27272a', borderRadius: 12, width: 'min(720px, 90vw)', maxHeight: '90vh', display: 'flex', flexDirection: 'column' }; const header: React.CSSProperties = { display: 'flex', alignItems: 'center', padding: '14px 18px', borderBottom: '1px solid #27272a', gap: 8 }; const body: React.CSSProperties = { flex: 1, padding: '14px 18px', overflowY: 'auto', display: 'flex', flexDirection: 'column' }; @@ -93,7 +93,14 @@ export const SitesmithModal: React.FC = ({ onClose, target }) => { }; return ( -
+
✨ Sitesmith
@@ -166,6 +173,6 @@ export const SitesmithModal: React.FC = ({ onClose, target }) => { onCancel={() => setPendingReplace(null)} />
-
+
); }; diff --git a/craft/src/panels/topbar/HeadCodeModal.tsx b/craft/src/panels/topbar/HeadCodeModal.tsx index 7030649..d84f528 100644 --- a/craft/src/panels/topbar/HeadCodeModal.tsx +++ b/craft/src/panels/topbar/HeadCodeModal.tsx @@ -1,5 +1,6 @@ -import React, { useEffect } from 'react'; +import React from 'react'; import { useSiteDesign } from '../../state/SiteDesignContext'; +import { Modal } from '../../ui/Modal'; interface HeadCodeModalProps { open: boolean; @@ -9,19 +10,8 @@ interface HeadCodeModalProps { export const HeadCodeModal: React.FC = ({ open, onClose }) => { const { design, updateDesign } = useSiteDesign(); - useEffect(() => { - if (!open) return; - const handler = (e: KeyboardEvent) => { - if (e.key === 'Escape') onClose(); - }; - window.addEventListener('keydown', handler); - return () => window.removeEventListener('keydown', handler); - }, [open, onClose]); - - if (!open) return null; - return ( -
+
e.stopPropagation()}> {/* Header */}
@@ -92,25 +82,12 @@ export const HeadCodeModal: React.FC = ({ open, onClose }) =
-
+ ); }; /* ---------- Styles ---------- */ -const backdropStyle: React.CSSProperties = { - position: 'fixed', - top: 0, - left: 0, - right: 0, - bottom: 0, - backgroundColor: 'rgba(0, 0, 0, 0.65)', - display: 'flex', - alignItems: 'center', - justifyContent: 'center', - zIndex: 10000, -}; - const modalStyle: React.CSSProperties = { width: '90vw', maxWidth: 700, diff --git a/craft/src/panels/topbar/TemplateModal.tsx b/craft/src/panels/topbar/TemplateModal.tsx index 0577843..93b2227 100644 --- a/craft/src/panels/topbar/TemplateModal.tsx +++ b/craft/src/panels/topbar/TemplateModal.tsx @@ -10,6 +10,7 @@ import { } from '../../templates'; import { componentResolver } from '../../components/resolver'; import { clickableProps } from '../../utils/a11y'; +import { Modal } from '../../ui/Modal'; // --------------------------------------------------------------------------- // Types @@ -60,18 +61,15 @@ export const TemplateModal: React.FC = ({ open, onClose }) = }; }, []); - // Close on Escape key - useEffect(() => { - if (!open) return; - const handler = (e: KeyboardEvent) => { - if (e.key === 'Escape') { - if (confirmTemplate) setConfirmTemplate(null); - else onClose(); - } - }; - document.addEventListener('keydown', handler); - return () => document.removeEventListener('keydown', handler); - }, [open, confirmTemplate, onClose]); + // Escape / backdrop-click close the confirmation dialog first if it's open, + // otherwise close the modal. Passed to below, which handles the + // Escape listener and the backdrop-click detection itself; the header's own + // close button (X) below still uses the raw `onClose` prop directly, so it + // always fully closes the modal even mid-confirmation (unchanged behavior). + const handleModalClose = useCallback(() => { + if (confirmTemplate) setConfirmTemplate(null); + else onClose(); + }, [confirmTemplate, onClose]); // Filter templates by category const filtered = useMemo(() => { @@ -236,24 +234,8 @@ export const TemplateModal: React.FC = ({ open, onClose }) = } }, [confirmTemplate, applyDesign, pages, addPage, switchPage, deletePage, updateDesign, addTemplateComponents, clearCanvas, applyHeaderFooter, wait, onClose]); - // Close on backdrop click - const handleBackdropClick = useCallback( - (e: React.MouseEvent) => { - if (e.target === e.currentTarget) { - if (confirmTemplate) { - setConfirmTemplate(null); - } else { - onClose(); - } - } - }, - [confirmTemplate, onClose], - ); - - if (!open) return null; - return ( -
+
{/* Header */}
@@ -377,7 +359,7 @@ export const TemplateModal: React.FC = ({ open, onClose }) =
)}
-
+
); }; @@ -473,19 +455,6 @@ const TemplateCard: React.FC<{ // Styles // --------------------------------------------------------------------------- -const backdropStyle: React.CSSProperties = { - position: 'fixed', - top: 0, - left: 0, - right: 0, - bottom: 0, - backgroundColor: 'rgba(0, 0, 0, 0.65)', - display: 'flex', - alignItems: 'center', - justifyContent: 'center', - zIndex: 10000, -}; - const modalStyle: React.CSSProperties = { width: '90vw', maxWidth: 900, diff --git a/craft/src/ui/Modal.test.tsx b/craft/src/ui/Modal.test.tsx new file mode 100644 index 0000000..47fa5b7 --- /dev/null +++ b/craft/src/ui/Modal.test.tsx @@ -0,0 +1,96 @@ +import { describe, test, expect, vi, afterEach } from 'vitest'; +import React from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; +import { Modal } from './Modal'; + +/* ---------- DOM test harness (no @testing-library/react in this repo, see + src/ui/AssetPicker.test.tsx for the same pattern: react-dom/client + + react-dom/test-utils `act`, both transitive deps of react-dom already). ---------- */ +let container: HTMLDivElement; +let root: Root; + +function render(ui: React.ReactElement) { + container = document.createElement('div'); + document.body.appendChild(container); + act(() => { + root = createRoot(container); + root.render(ui); + }); +} + +function unmount() { + act(() => { root.unmount(); }); + container.remove(); +} + +function pressEscape() { + act(() => { + window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true })); + }); +} + +function click(el: Element | null) { + if (!el) throw new Error('element not found'); + act(() => { (el as HTMLElement).dispatchEvent(new MouseEvent('click', { bubbles: true })); }); +} + +afterEach(() => { + if (container) unmount(); + document.body.style.overflow = ''; +}); + +describe('Modal', () => { + test('renders children when open', () => { + render(
hello
); + expect(container.querySelector('[data-testid="body"]')?.textContent).toBe('hello'); + }); + + test('renders nothing when closed', () => { + render(
hello
); + expect(container.querySelector('[data-testid="body"]')).toBeNull(); + }); + + test('Escape calls onClose by default', () => { + const onClose = vi.fn(); + render(
body
); + pressEscape(); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + test('Escape does not call onClose when closeOnEscape is false', () => { + const onClose = vi.fn(); + render(
body
); + pressEscape(); + expect(onClose).not.toHaveBeenCalled(); + }); + + test('clicking the backdrop calls onClose by default', () => { + const onClose = vi.fn(); + render(
body
); + // The backdrop is the outermost rendered element. + click(container.firstElementChild); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + test('clicking inside the content does not call onClose', () => { + const onClose = vi.fn(); + render(
body
); + click(container.querySelector('[data-testid="body"]')); + expect(onClose).not.toHaveBeenCalled(); + }); + + test('clicking the backdrop does not call onClose when closeOnBackdropClick is false', () => { + const onClose = vi.fn(); + render(
body
); + click(container.firstElementChild); + expect(onClose).not.toHaveBeenCalled(); + }); + + test('locks body scroll while open and restores it on close', () => { + render(
body
); + expect(document.body.style.overflow).toBe('hidden'); + act(() => { root.render(
body
); }); + expect(document.body.style.overflow).toBe(''); + }); +}); diff --git a/craft/src/ui/Modal.tsx b/craft/src/ui/Modal.tsx new file mode 100644 index 0000000..9c4be30 --- /dev/null +++ b/craft/src/ui/Modal.tsx @@ -0,0 +1,94 @@ +import React, { useEffect } from 'react'; + +export interface ModalProps { + open: boolean; + onClose: () => void; + title?: string; + children: React.ReactNode; + width?: number | string; + /** Override the backdrop's own inline styles (e.g. a different overlay + * opacity/z-index). Merged on top of the shared defaults. */ + backdropStyle?: React.CSSProperties; + /** Escape key closes the modal. Default true — set false to preserve a + * modal that never handled Escape before this component existed. */ + closeOnEscape?: boolean; + /** Clicking the backdrop (outside the modal content) closes the modal. + * Default true — set false to preserve a modal that never handled + * backdrop clicks before this component existed. */ + closeOnBackdropClick?: boolean; + /** Extra attributes spread onto the backdrop div (e.g. role="dialog", + * aria-modal) for a modal that previously set those directly. */ + backdropProps?: React.HTMLAttributes; +} + +const defaultBackdropStyle: React.CSSProperties = { + position: 'fixed', + top: 0, + left: 0, + right: 0, + bottom: 0, + backgroundColor: 'rgba(0, 0, 0, 0.65)', + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + zIndex: 10000, +}; + +/* ---------- Shared modal chrome ---------- + Backdrop + centered panel wrapper + Escape-to-close + scroll-lock, extracted + from TemplateModal / HeadCodeModal / SitesmithModal (task E4.3). Each + modal's own content — including its own header, close button, and panel + styling (background, radius, shadow, dimensions) — stays in that modal's + own file and is passed in as `children`, since those visuals differ across + the three. `closeOnEscape` / `closeOnBackdropClick` default to true (the + behavior TemplateModal and HeadCodeModal already had); SitesmithModal had + neither before this extraction, so it opts out of both to keep its exact + prior behavior. */ +export const Modal: React.FC = ({ + open, + onClose, + title, + children, + width, + backdropStyle, + closeOnEscape = true, + closeOnBackdropClick = true, + backdropProps, +}) => { + useEffect(() => { + if (!open || !closeOnEscape) return; + const handler = (e: KeyboardEvent) => { + if (e.key === 'Escape') onClose(); + }; + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, [open, closeOnEscape, onClose]); + + useEffect(() => { + if (!open) return; + const prevOverflow = document.body.style.overflow; + document.body.style.overflow = 'hidden'; + return () => { + document.body.style.overflow = prevOverflow; + }; + }, [open]); + + if (!open) return null; + + return ( +
{ + if (closeOnBackdropClick && e.target === e.currentTarget) onClose(); + }} + > +
+ {title !== undefined && ( +
{title}
+ )} + {children} +
+
+ ); +};