refactor(builder): extract shared Modal component for the three editor modals
New src/ui/Modal.tsx ({ open, onClose, title?, children, width?,
closeOnEscape?, closeOnBackdropClick?, backdropStyle?, backdropProps? })
owns the backdrop, Escape-to-close, backdrop-click-to-close, and a new
body-scroll-lock while open. Adopted by TemplateModal, HeadCodeModal, and
SitesmithModal; each keeps its own panel styling/header/footer as children
since those differ per modal.
- TemplateModal: Escape/backdrop-click still close the confirm-template
sub-dialog first via a wrapped onClose passed to Modal; the header's X
button keeps using the raw onClose prop (always fully closes), matching
prior asymmetric behavior.
- HeadCodeModal: straightforward adoption, no prior custom close logic.
- SitesmithModal: previously had no `open` prop, no Escape-to-close, and no
backdrop-click-to-close. Preserved via open (always mounted-open by its
parent already), closeOnEscape={false}, closeOnBackdropClick={false}; role
and aria-modal are passed through via backdropProps to keep them on the
same backdrop element as before.
Body scroll-lock while a modal is open is a small new addition (requested by
the task) applied uniformly; it has no visible effect since each modal's
opaque fixed-position backdrop already fully covers the viewport.
Added a light Modal.test.tsx (renders children, Escape/backdrop-click close
behavior, scroll-lock) using the existing react-dom/client + act harness.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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(<Modal open onClose={vi.fn()}><div data-testid="body">hello</div></Modal>);
|
||||
expect(container.querySelector('[data-testid="body"]')?.textContent).toBe('hello');
|
||||
});
|
||||
|
||||
test('renders nothing when closed', () => {
|
||||
render(<Modal open={false} onClose={vi.fn()}><div data-testid="body">hello</div></Modal>);
|
||||
expect(container.querySelector('[data-testid="body"]')).toBeNull();
|
||||
});
|
||||
|
||||
test('Escape calls onClose by default', () => {
|
||||
const onClose = vi.fn();
|
||||
render(<Modal open onClose={onClose}><div>body</div></Modal>);
|
||||
pressEscape();
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test('Escape does not call onClose when closeOnEscape is false', () => {
|
||||
const onClose = vi.fn();
|
||||
render(<Modal open onClose={onClose} closeOnEscape={false}><div>body</div></Modal>);
|
||||
pressEscape();
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('clicking the backdrop calls onClose by default', () => {
|
||||
const onClose = vi.fn();
|
||||
render(<Modal open onClose={onClose}><div data-testid="body">body</div></Modal>);
|
||||
// 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(<Modal open onClose={onClose}><div data-testid="body">body</div></Modal>);
|
||||
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(<Modal open onClose={onClose} closeOnBackdropClick={false}><div>body</div></Modal>);
|
||||
click(container.firstElementChild);
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('locks body scroll while open and restores it on close', () => {
|
||||
render(<Modal open onClose={vi.fn()}><div>body</div></Modal>);
|
||||
expect(document.body.style.overflow).toBe('hidden');
|
||||
act(() => { root.render(<Modal open={false} onClose={vi.fn()}><div>body</div></Modal>); });
|
||||
expect(document.body.style.overflow).toBe('');
|
||||
});
|
||||
});
|
||||
@@ -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<HTMLDivElement>;
|
||||
}
|
||||
|
||||
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<ModalProps> = ({
|
||||
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 (
|
||||
<div
|
||||
{...backdropProps}
|
||||
style={backdropStyle ? { ...defaultBackdropStyle, ...backdropStyle } : defaultBackdropStyle}
|
||||
onClick={(e) => {
|
||||
if (closeOnBackdropClick && e.target === e.currentTarget) onClose();
|
||||
}}
|
||||
>
|
||||
<div style={width !== undefined ? { width } : undefined}>
|
||||
{title !== undefined && (
|
||||
<div style={{ fontSize: 15, fontWeight: 600, marginBottom: 8 }}>{title}</div>
|
||||
)}
|
||||
{children}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
Reference in New Issue
Block a user