Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions src/lib/actions/modalDialog.svelte.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/**
* Shared controller for the native modal `<dialog>` overlays (Dialog, Sheet).
*
* Drives a `<dialog>` imperatively from a two-way `open` binding: `showModal()`
* reflects it into the top layer with a focus trap + inert background, and every
* dismissal path (Escape, backdrop click, `open=false`) routes through the native
* `close` event so `handleClose` fires exactly once. Also saves the previously
* focused element on open and restores it on close or on teardown-while-open
* (where the native `close` event never fires).
*
* Params are getters so the caller stays the single source of truth (mirrors the
* `useOverlayTheme(invert)` convention). Call during component init.
*/
export function useModalDialog(opts: {
/** The bound `<dialog>` element, or null before mount. */
dialog: () => HTMLDialogElement | null;
/** Current open state. */
open: () => boolean;
/** Sync the two-way `open` binding back to the caller. */
setOpen: (value: boolean) => void;
/** Optional close callback, read lazily so a late-bound prop is honored. */
onclose: () => (() => void) | undefined;
}): { handleClose: () => void } {
let previouslyFocused: HTMLElement | null = null;

// Never set the `open` attribute in markup; showModal()/close() are the only
// drivers so the native `close` event stays the single dismissal path.
$effect(() => {
const d = opts.dialog();
if (!d) return;
if (opts.open() && !d.open) {
previouslyFocused = document.activeElement as HTMLElement | null;
d.showModal();
} else if (!opts.open() && d.open) {
d.close();
}
});

// If unmounted while still open, the native `close` event never fires, so
// handleClose() never runs — restore focus on teardown so it isn't stranded
// on a now-removed element.
$effect(() => () => {
if (previouslyFocused?.isConnected) previouslyFocused.focus({ preventScroll: true });
});

// The single source of truth for closing is the native `close` event; sync the
// binding, notify, and restore focus.
function handleClose() {
opts.setOpen(false);
opts.onclose()?.();
if (previouslyFocused?.isConnected) previouslyFocused.focus({ preventScroll: true });
previouslyFocused = null;
}

return { handleClose };
}
38 changes: 8 additions & 30 deletions src/lib/components/Dialog.svelte
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<script lang="ts">
import type { Snippet } from 'svelte';
import { useOverlayTheme } from '../theme/context.js';
import { useModalDialog } from '../actions/modalDialog.svelte.js';

interface Props {
/** Open state (two-way bindable). */
Expand Down Expand Up @@ -37,38 +38,15 @@

const titleId = $props.id();
let dialog = $state<HTMLDialogElement | null>(null);
let previouslyFocused: HTMLElement | null = null;

// Drive the native dialog imperatively. We never set the `open` attribute in
// markup; showModal() reflects it, puts the dialog in the top layer, and adds
// a focus trap + inert background.
$effect(() => {
const d = dialog;
if (!d) return;
if (open && !d.open) {
previouslyFocused = document.activeElement as HTMLElement | null;
d.showModal();
} else if (!open && d.open) {
d.close();
}
// Native modal wiring (showModal/close, single-dismissal `close` event, focus
// save/restore) is shared with <Sheet> — see useModalDialog.
const { handleClose } = useModalDialog({
dialog: () => dialog,
open: () => open,
setOpen: (v) => (open = v),
onclose: () => onclose
});

// If the dialog is unmounted while still open, the native `close` event never
// fires, so handleClose() never runs — restore focus on teardown so it isn't
// stranded on a now-removed element.
$effect(() => () => {
if (previouslyFocused?.isConnected) previouslyFocused.focus({ preventScroll: true });
});

// The single source of truth for closing is the native `close` event; every
// dismissal path (Escape, backdrop click, open=false) routes through d.close()
// so this fires exactly once. Sync the binding, notify, and restore focus.
function handleClose() {
open = false;
onclose?.();
if (previouslyFocused?.isConnected) previouslyFocused.focus({ preventScroll: true });
previouslyFocused = null;
}
</script>

<dialog
Expand Down
46 changes: 46 additions & 0 deletions src/lib/components/Dialog.svelte.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,52 @@ describe('Dialog', () => {
expect(document.querySelector('dialog.pn-dialog')!.getAttribute('data-theme')).toBeNull();
});

// --- Focus save/restore (owned by useModalDialog, shared with Sheet) ---
// Pinned here per POS-75: the a11y-critical focus logic was centralized so a
// future fix can't land in one component and not the other — that guarantee
// only holds if the centralized behavior is test-pinned. Render closed and open
// via rerender so the controller captures a real opener as document.activeElement
// (rendering open from the start would capture <body>, not an opener).

test('restores focus to the opener when closed', async () => {
const opener = document.createElement('button');
document.body.appendChild(opener);
opener.focus();
expect(document.activeElement).toBe(opener);

const screen = render(Dialog, { open: false, title: 'T', children: body });
await screen.rerender({ open: true, title: 'T', children: body });

const el = document.querySelector('dialog.pn-dialog') as HTMLDialogElement;
await expect.poll(() => el.open).toBe(true);
// showModal() pulls focus into the top layer, off the opener…
expect(document.activeElement).not.toBe(opener);

el.close(); // native close — the single dismissal path
// …and the close handler returns focus to whoever opened the overlay.
await expect.poll(() => document.activeElement).toBe(opener);
opener.remove();
});

test('restores focus on teardown while still open (native close never fires)', async () => {
const opener = document.createElement('button');
document.body.appendChild(opener);
opener.focus();

const screen = render(Dialog, { open: false, title: 'T', children: body });
await screen.rerender({ open: true, title: 'T', children: body });

const el = document.querySelector('dialog.pn-dialog') as HTMLDialogElement;
await expect.poll(() => el.open).toBe(true);
expect(document.activeElement).not.toBe(opener);

// Unmounting an open <dialog> never fires the native `close` event, so focus
// restore must come from the $effect cleanup branch — assert it still runs.
await screen.unmount();
await expect.poll(() => document.activeElement).toBe(opener);
opener.remove();
});

// Polarity inversion alone is the dialog's figure-ground cue — no neon glow,
// even when the inverted surface is Machine-themed.
test('no glow in either polarity', () => {
Expand Down
31 changes: 8 additions & 23 deletions src/lib/components/Sheet.svelte
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<script lang="ts">
import type { Snippet } from 'svelte';
import { useOverlayTheme } from '../theme/context.js';
import { useModalDialog } from '../actions/modalDialog.svelte.js';

interface Props {
/** Open state (two-way bindable). */
Expand Down Expand Up @@ -37,31 +38,15 @@

const titleId = $props.id();
let dialog = $state<HTMLDialogElement | null>(null);
let previouslyFocused: HTMLElement | null = null;

$effect(() => {
const d = dialog;
if (!d) return;
if (open && !d.open) {
previouslyFocused = document.activeElement as HTMLElement | null;
d.showModal();
} else if (!open && d.open) {
d.close();
}
});

// Restore focus on teardown if unmounted while open (no native `close` fires).
$effect(() => () => {
if (previouslyFocused?.isConnected) previouslyFocused.focus({ preventScroll: true });
// Native modal wiring (showModal/close, single-dismissal `close` event, focus
// save/restore) is shared with <Dialog> — see useModalDialog.
const { handleClose } = useModalDialog({
dialog: () => dialog,
open: () => open,
setOpen: (v) => (open = v),
onclose: () => onclose
});

// Closing always routes through the native `close` event so this fires once.
function handleClose() {
open = false;
onclose?.();
if (previouslyFocused?.isConnected) previouslyFocused.focus({ preventScroll: true });
previouslyFocused = null;
}
</script>

<dialog
Expand Down