From 2a60e1813600fa4df32ad53bd3d5320aa3e2e80b Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Thu, 27 Aug 2026 19:12:55 +0200 Subject: [PATCH 1/2] fix(dialog): non-modal coexistence, drag-safe outside presses, name warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three dialog fixes, decided once in dom-dialog and inherited by both DOM substrates: - A non-modal Viewport no longer swallows presses aimed at the page: it renders pointer-events: none (the window stays auto), and outside presses move to a document-level watch — topmost-only, window and trigger excepted — since a transparent Viewport can't detect them itself. - A text-selection drag no longer dismisses: trackPressOrigin captures where a press began at pointerdown, before the browser collapses the click target to the common ancestor, and the press gates refuse a gesture that started inside the window. - A dialog that resolves no accessible name warns on open — the core contract (Title, or aria-label/aria-labelledby, never neither) was documented but unenforced. Deferred a macrotask so a rendered Title's registration commits first; cancelled by close. Co-Authored-By: Claude Fable 5 --- .changeset/dialog-accessible-name-warning.md | 13 ++ .changeset/dialog-drag-never-dismisses.md | 19 ++ .../dialog-nonmodal-pointer-passthrough.md | 22 +++ packages/core/dialog/SPEC.md | 10 +- packages/dom/components/dialog/SPEC.md | 55 +++++- packages/dom/components/dialog/src/index.ts | 3 + .../dom/components/dialog/src/open-layer.ts | 19 ++ .../dom/components/dialog/src/press-origin.ts | 28 +++ packages/dom/components/dialog/src/press.ts | 19 +- .../components/dialog/src/viewport-style.ts | 15 ++ .../dialog/src/watch-outside-press.ts | 37 ++++ .../components/dialog/tests/dialog.test.ts | 175 +++++++++++++++++- packages/react/dialog/SPEC.md | 12 +- packages/react/dialog/src/context.ts | 12 ++ packages/react/dialog/src/dialog.tsx | 87 +++++++-- .../react/dialog/stories/dialog.stories.tsx | 3 +- packages/react/dialog/tests/dialog.test.tsx | 119 ++++++++++++ packages/solid/dialog/SPEC.md | 12 +- packages/solid/dialog/src/context.ts | 11 ++ packages/solid/dialog/src/dialog.tsx | 124 +++++++++++-- .../solid/dialog/stories/dialog.stories.tsx | 3 +- packages/solid/dialog/tests/dialog.test.tsx | 121 ++++++++++++ 22 files changed, 861 insertions(+), 58 deletions(-) create mode 100644 .changeset/dialog-accessible-name-warning.md create mode 100644 .changeset/dialog-drag-never-dismisses.md create mode 100644 .changeset/dialog-nonmodal-pointer-passthrough.md create mode 100644 packages/dom/components/dialog/src/press-origin.ts create mode 100644 packages/dom/components/dialog/src/viewport-style.ts create mode 100644 packages/dom/components/dialog/src/watch-outside-press.ts diff --git a/.changeset/dialog-accessible-name-warning.md b/.changeset/dialog-accessible-name-warning.md new file mode 100644 index 0000000..21845cc --- /dev/null +++ b/.changeset/dialog-accessible-name-warning.md @@ -0,0 +1,13 @@ +--- +'@dunky.dev/dom-dialog': patch +--- + +The dialog warns when it resolves no accessible name. The core contract — +a rendered Title, or an `aria-label` / `aria-labelledby` on Content, never +neither — was documented but unenforced, so a nameless dialog shipped +silently. `openDialogLayer` now checks the window's own attributes on open +and warns with the fix, the same loud treatment the stranded-focus miss +already gets. The check reads the DOM rather than the machine's +Title-presence flag (a label may arrive through a prop spread), defers a +macrotask so a rendered Title's registration commits first, and is cancelled +by the close. diff --git a/.changeset/dialog-drag-never-dismisses.md b/.changeset/dialog-drag-never-dismisses.md new file mode 100644 index 0000000..f028beb --- /dev/null +++ b/.changeset/dialog-drag-never-dismisses.md @@ -0,0 +1,19 @@ +--- +'@dunky.dev/dom-dialog': minor +'@dunky.dev/react-dialog': patch +'@dunky.dev/solid-dialog': patch +--- + +A text-selection drag no longer dismisses the dialog. A mousedown inside the +window with a mouseup outside it makes the browser fire `click` on their +common ancestor — the viewport or the backdrop — which was indistinguishable +from a genuine outside press by the click's target alone, so selecting text +across the window's edge closed the dialog and lost the form state with it. + +Where a press *began* now decides, not where it ended: `@dunky.dev/dom-dialog` +gains `trackPressOrigin(content)`, which captures each press's origin at +`pointerdown` (before the browser collapses the click target), and +`acceptsBackdropPress` / `acceptsViewportPress` take that as a new +`startedInside` argument and refuse the press when the gesture started +inside the window. A press that starts and ends outside still dismisses as +before. diff --git a/.changeset/dialog-nonmodal-pointer-passthrough.md b/.changeset/dialog-nonmodal-pointer-passthrough.md new file mode 100644 index 0000000..0296b74 --- /dev/null +++ b/.changeset/dialog-nonmodal-pointer-passthrough.md @@ -0,0 +1,22 @@ +--- +'@dunky.dev/dom-dialog': minor +'@dunky.dev/react-dialog': patch +'@dunky.dev/solid-dialog': patch +--- + +A non-modal dialog no longer blocks the page around it. `Dialog.Viewport` is +a full-coverage layer, so even with no backdrop rendered it swallowed every +press aimed at the page beneath — the page element never received it, and +the dialog dismissed instead. Now, while `modal={false}`, the Viewport +renders with `pointer-events: none` and the window with +`pointer-events: auto` (the consumer's own `style` wins over both), so +presses on the empty area reach the page and the window stays interactive. + +Outside-press detection moves with it: a transparent Viewport never receives +the press it used to detect, so `@dunky.dev/dom-dialog` gains +`watchOutsidePress` — a document-level watch (the only vantage point that +sees both the portaled dialog and the page) with the same refusals as the +element path: topmost dialog only, the window excepted, and the trigger +excepted so its own press stays a plain toggle instead of a +close-and-reopen. `viewportPointerEvents(modal)` and `contentPointerEvents` +carry the two style values so every DOM substrate applies the same pair. diff --git a/packages/core/dialog/SPEC.md b/packages/core/dialog/SPEC.md index 2ca4129..9a1b109 100644 --- a/packages/core/dialog/SPEC.md +++ b/packages/core/dialog/SPEC.md @@ -50,7 +50,15 @@ Using the dialog is a walkthrough of intent, not a prop list: - Pressing the **backdrop** — or the **viewport** area around the dialog window — counts as an "outside interaction"; whether that dismisses follows the dialog's dismissal settings, whichever layer was pressed. Presses inside the - content never count as outside. + content never count as outside — including a press that starts inside and + releases outside (a text-selection drag): where a press ends doesn't decide, + where it began does. The trigger's own press is never an outside interaction + either; it stays a plain toggle. +- A **non-modal** dialog (`modal={false}`) coexists with the page: no backdrop, + no focus trap, no scroll lock, and the page around it stays fully + interactive — a press on the empty area around the window reaches whatever + the page has there. That press still counts as the dialog's outside + interaction (the trigger excepted), following the same dismissal settings. - The **content** is the dialog window, labelled and described by the Title and Description parts when they are rendered. - **Title / Description** name and describe the dialog. The dialog's ARIA name diff --git a/packages/dom/components/dialog/SPEC.md b/packages/dom/components/dialog/SPEC.md index a2321b1..cb39fc5 100644 --- a/packages/dom/components/dialog/SPEC.md +++ b/packages/dom/components/dialog/SPEC.md @@ -69,6 +69,17 @@ typically because it lacks `tabindex="-1"` — focus is stranded outside the layer, against the modal dialog pattern. That miss is loud: a `console.warn` names the fix instead of failing silently. +The window's accessible name gets the same loud treatment: neither a +rendered Title (`aria-labelledby`) nor an explicit `aria-label` on the +window warns, rather than shipping a dialog assistive tech can't name. +Reading the window's own attributes — not the machine's Title-registered +flag — is deliberate: a consumer's label can arrive through a prop spread +rather than a typed prop, and the DOM is where it lands either way. The +check defers a macrotask so a rendered Title's registration commits first +(it arrives through the substrate's own re-render, not this call's turn), +and the close cancels it — a dialog gone before the check fires must not +warn about a window that no longer exists. + The disposer releases the stack **before** restoring focus. Both orders are load-bearing: the stack must exist before focus moves in, and the layers beneath must be un-inerted before focus can land on one of them. @@ -119,6 +130,28 @@ dialog's to answer: press must have started on the viewport itself, and then the same topmost rule applies. +Both also refuse a press whose gesture began inside the window, from +`trackPressOrigin`. A `click`'s own target can't answer that once the +browser has collapsed it: a mousedown inside the window and a mouseup +outside it fires `click` on their common ancestor — the backdrop or the +viewport, not where the press actually started — so a text-selection drag +that starts inside and releases outside would otherwise read as a genuine +outside press and lose whatever the user was selecting. `trackPressOrigin` +captures the press's origin at `pointerdown`, before that collapse, the +only point it's still recoverable. + +A non-modal dialog has no Backdrop, and its Viewport lets a press on the +empty area fall through to the page rather than swallow it — +`viewportPointerEvents` is `none` there, `contentPointerEvents` on the +window stays `auto` regardless. That fall-through means Viewport never +receives those presses to detect them, so `watchOutsidePress` is the +non-modal substitute: a document-level listener, because the document is +the only vantage point that can see both the dialog (portaled out of the +page's own subtree) and the page. It carries the same three refusals — +topmost-only, the window excepted, the trigger excepted (its own press +stays a plain toggle; counting it as outside would close and immediately +reopen) — plus the same started-inside guard as the click-based path. + ### Focus trap `dialogTrapOptions` is the trap configuration the substrate hands to its @@ -127,15 +160,19 @@ part is the cycle's last stop wherever it renders. ## API -| Export | Description | -| ------------------------------------- | --------------------------------------------------------------------------- | -| `domDialogEffects` | Core effects + the document Escape listener, as `DialogEffect` tuples. | -| `openDialogLayer(content, options)` | The open sequence; returns the close sequence. | -| `startExitWindow(content, options)` | Hides and watches the still-painting layer; returns the undo. | -| `guardBackNavigation(options)` | The history guard: report the open state as it changes, release at the end. | -| `acceptsBackdropPress(id)` | Whether a backdrop press is this dialog's outside interaction. | -| `acceptsViewportPress(id, event)` | Same for the viewport, ignoring presses that bubbled from the content. | -| `dialogTrapOptions(machine, closeId)` | `TrapFocusOptions` for the dialog window. | +| Export | Description | +| ------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------- | +| `domDialogEffects` | Core effects + the document Escape listener, as `DialogEffect` tuples. | +| `openDialogLayer(content, options)` | The open sequence; returns the close sequence. | +| `startExitWindow(content, options)` | Hides and watches the still-painting layer; returns the undo. | +| `guardBackNavigation(options)` | The history guard: report the open state as it changes, release at the end. | +| `acceptsBackdropPress(id, startedInside)` | Whether a backdrop press is this dialog's outside interaction. | +| `acceptsViewportPress(id, event, startedInside)` | Same for the viewport, ignoring presses that bubbled from the content. | +| `trackPressOrigin(content)` | Tracks whether the most recent press began inside `content`; returns `{ startedInside, dispose }`. | +| `watchOutsidePress(id, options)` | The non-modal outside-press surface — a document-level substitute for the Viewport's own click. Returns the undo. | +| `viewportPointerEvents(modal)` | The Viewport's `pointer-events` value: `undefined` while modal, `'none'` otherwise. | +| `contentPointerEvents` | The window's `pointer-events` value — always `'auto'`. | +| `dialogTrapOptions(machine, closeId)` | `TrapFocusOptions` for the dialog window. | ## Constraints diff --git a/packages/dom/components/dialog/src/index.ts b/packages/dom/components/dialog/src/index.ts index cecf6bd..16ffbe8 100644 --- a/packages/dom/components/dialog/src/index.ts +++ b/packages/dom/components/dialog/src/index.ts @@ -7,4 +7,7 @@ export { type BackNavigationGuardOptions, } from './back-navigation' export { acceptsBackdropPress, acceptsViewportPress } from './press' +export { trackPressOrigin, type PressOriginTracker } from './press-origin' +export { watchOutsidePress, type WatchOutsidePressOptions } from './watch-outside-press' +export { viewportPointerEvents, contentPointerEvents } from './viewport-style' export { dialogTrapOptions } from './focus-trap' diff --git a/packages/dom/components/dialog/src/open-layer.ts b/packages/dom/components/dialog/src/open-layer.ts index eba0860..25bd01d 100644 --- a/packages/dom/components/dialog/src/open-layer.ts +++ b/packages/dom/components/dialog/src/open-layer.ts @@ -58,7 +58,26 @@ export function openDialogLayer(content: HTMLElement, options: OpenDialogLayerOp } } + // The window's own attributes are the reliable read: a consumer's label may + // arrive through a prop spread rather than a typed prop, and the DOM is + // where it lands either way. Deferred a macrotask: a rendered Title + // registers presence through its own effect, and the canonical nesting + // (Title inside Content) settles before this call's own effect runs — but + // nothing requires that arrangement, so a Title rendered as a later + // sibling still gets a chance to register first. Cleared on close: a + // dialog that closes before the check fires must not warn about a window + // that no longer exists. + const nameCheck = setTimeout(() => { + if (!content.hasAttribute('aria-label') && !content.hasAttribute('aria-labelledby')) { + console.warn( + '[openDialogLayer] the dialog has no accessible name: render a , ' + + 'or pass aria-label / aria-labelledby to Content.', + ) + } + }) + return () => { + clearTimeout(nameCheck) unregister() if (previous instanceof HTMLElement) previous.focus({ preventScroll: true }) } diff --git a/packages/dom/components/dialog/src/press-origin.ts b/packages/dom/components/dialog/src/press-origin.ts new file mode 100644 index 0000000..36dcdce --- /dev/null +++ b/packages/dom/components/dialog/src/press-origin.ts @@ -0,0 +1,28 @@ +export interface PressOriginTracker { + /** Whether the most recently observed press began inside the tracked + * element. */ + startedInside: () => boolean + /** Stops tracking. */ + dispose: () => void +} + +/** + * Tracks whether the most recent pointer press began inside `content`. A + * click's own target can't answer that once the browser has collapsed it: a + * mousedown inside the window and a mouseup outside it fires `click` on + * their common ancestor, not on where the press actually started — a + * text-selection drag then reads as a fresh press on whatever sits above + * that ancestor. Capturing at pointerdown, before the collapse, is the only + * way to recover it. + */ +export function trackPressOrigin(content: HTMLElement): PressOriginTracker { + let insideContent = false + const onPointerDown = (event: PointerEvent): void => { + insideContent = event.target instanceof Node && content.contains(event.target) + } + document.addEventListener('pointerdown', onPointerDown, true) + return { + startedInside: () => insideContent, + dispose: () => document.removeEventListener('pointerdown', onPointerDown, true), + } +} diff --git a/packages/dom/components/dialog/src/press.ts b/packages/dom/components/dialog/src/press.ts index 5301984..1799220 100644 --- a/packages/dom/components/dialog/src/press.ts +++ b/packages/dom/components/dialog/src/press.ts @@ -10,9 +10,13 @@ interface PressTarget { /** * Whether a backdrop press is this dialog's outside interaction. Only the * topmost dialog of a stack answers one — a nested stack dismisses one layer - * at a time, the same rule Escape follows. + * at a time, the same rule Escape follows. `startedInside` (see + * `trackPressOrigin`) refuses a press whose gesture began inside the + * window: a text-selection drag that starts inside and releases on the + * backdrop must not read as an outside press. */ -export function acceptsBackdropPress(id: string): boolean { +export function acceptsBackdropPress(id: string, startedInside: boolean): boolean { + if (startedInside) return false return isTopmostLayer(id) } @@ -20,8 +24,17 @@ export function acceptsBackdropPress(id: string): boolean { * Whether a viewport press is this dialog's outside interaction. Content * presses bubble up to the viewport, so only a press that started on the * viewport itself counts — and then only for the topmost dialog. + * `startedInside` catches what the bubble check can't: a text-selection drag + * starting inside the window and releasing on the viewport's own background + * collapses the click's target to the viewport, passing the bubble check — + * see `trackPressOrigin`. */ -export function acceptsViewportPress(id: string, event: PressTarget): boolean { +export function acceptsViewportPress( + id: string, + event: PressTarget, + startedInside: boolean, +): boolean { + if (startedInside) return false if (event.target !== event.currentTarget) return false return isTopmostLayer(id) } diff --git a/packages/dom/components/dialog/src/viewport-style.ts b/packages/dom/components/dialog/src/viewport-style.ts new file mode 100644 index 0000000..0b0ebd7 --- /dev/null +++ b/packages/dom/components/dialog/src/viewport-style.ts @@ -0,0 +1,15 @@ +/** + * The Viewport's pointer-events reach. Modal: unset — it is the outside- + * press surface for whatever gutter sits around the window. Non-modal: the + * page coexists with the dialog, so the Viewport's own full-coverage box + * must not swallow presses meant for whatever sits beneath it — `none` lets + * them fall through; the window stays reachable regardless (see + * `contentPointerEvents`). + */ +export function viewportPointerEvents(modal: boolean): 'none' | undefined { + return modal ? undefined : 'none' +} + +/** The dialog window always stays interactive, even where its Viewport + * disables pointer events to let non-modal presses fall through around it. */ +export const contentPointerEvents = 'auto' as const diff --git a/packages/dom/components/dialog/src/watch-outside-press.ts b/packages/dom/components/dialog/src/watch-outside-press.ts new file mode 100644 index 0000000..88b2651 --- /dev/null +++ b/packages/dom/components/dialog/src/watch-outside-press.ts @@ -0,0 +1,37 @@ +import { isTopmostLayer } from '@dunky.dev/dom-overlay' + +export interface WatchOutsidePressOptions { + /** The dialog window; a press landing inside it is never outside. */ + content: HTMLElement + /** The trigger that opened this dialog, excepted so its own press stays a + * plain toggle — counting it as outside would close and immediately + * reopen. */ + trigger: Element | null + /** Whether the gesture that produced this click began inside `content` — + * see `trackPressOrigin`. A text-selection drag starting inside and + * releasing outside must not read as an outside press either. */ + startedInside: () => boolean + onOutsidePress: (event: MouseEvent) => void +} + +/** + * The non-modal outside-press surface. A non-modal dialog has no Backdrop, + * and its Viewport lets a press on the empty area fall through to the page + * rather than swallow it (see the pointer-events split the substrate + * bindings apply) — which means Viewport itself never receives that press + * to detect it. The document is the only vantage point left that can see + * both the dialog and the page. + */ +export function watchOutsidePress(id: string, options: WatchOutsidePressOptions): () => void { + const onClick = (event: MouseEvent): void => { + if (!isTopmostLayer(id)) return + if (options.startedInside()) return + const target = event.target + if (!(target instanceof Node)) return + if (options.content.contains(target)) return + if (options.trigger?.contains(target) === true) return + options.onOutsidePress(event) + } + document.addEventListener('click', onClick, true) + return () => document.removeEventListener('click', onClick, true) +} diff --git a/packages/dom/components/dialog/tests/dialog.test.ts b/packages/dom/components/dialog/tests/dialog.test.ts index 94377e0..67ff627 100644 --- a/packages/dom/components/dialog/tests/dialog.test.ts +++ b/packages/dom/components/dialog/tests/dialog.test.ts @@ -18,6 +18,8 @@ import { guardBackNavigation, openDialogLayer, startExitWindow, + trackPressOrigin, + watchOutsidePress, } from '@dunky.dev/dom-dialog' type DialogService = Machine @@ -48,6 +50,7 @@ const registered: (() => void)[] = [] const mountLayer = (id: string, depth: number, html = '', dismiss?: () => void): HTMLElement => { const content = document.createElement('div') content.tabIndex = -1 + content.setAttribute('aria-label', 'Layer') content.innerHTML = html document.body.append(content) registered.push( @@ -137,6 +140,7 @@ describe('openDialogLayer', () => { ): { content: HTMLElement; close: () => void } => { const content = document.createElement('div') content.tabIndex = -1 + content.setAttribute('aria-label', 'Dialog') content.innerHTML = html document.body.append(content) @@ -154,6 +158,7 @@ describe('openDialogLayer', () => { it('moves focus to the first form field, without scrolling the locked surface', () => { const content = document.createElement('div') content.tabIndex = -1 + content.setAttribute('aria-label', 'Dialog') content.innerHTML = '' document.body.append(content) const field = document.getElementById('field') as HTMLInputElement @@ -168,6 +173,7 @@ describe('openDialogLayer', () => { it('honors an explicit initialFocus over the overlay default', () => { const content = document.createElement('div') content.tabIndex = -1 + content.setAttribute('aria-label', 'Dialog') content.innerHTML = '' document.body.append(content) const pick = content.querySelector('#pick') as HTMLButtonElement @@ -180,6 +186,7 @@ describe('openDialogLayer', () => { it('falls back to the dialog window when the target refuses focus', () => { const content = document.createElement('div') content.tabIndex = -1 + content.setAttribute('aria-label', 'Dialog') content.innerHTML = '' document.body.append(content) const field = content.querySelector('#field') as HTMLInputElement @@ -201,6 +208,32 @@ describe('openDialogLayer', () => { expect(warn).toHaveBeenCalledWith(expect.stringContaining('tabindex="-1"')) }) + it('warns when the dialog has neither a Title nor an accessible label', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const content = document.createElement('div') + content.tabIndex = -1 + document.body.append(content) + + registered.push(openDialogLayer(content, options)) + await new Promise(resolve => setTimeout(resolve, 0)) + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('no accessible name')) + }) + + it('does not warn when the window carries aria-labelledby', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const content = document.createElement('div') + content.tabIndex = -1 + content.setAttribute('aria-labelledby', 'dlg-title') + content.innerHTML = '

Title

' + document.body.append(content) + + registered.push(openDialogLayer(content, options)) + await new Promise(resolve => setTimeout(resolve, 0)) + + expect(warn).not.toHaveBeenCalledWith(expect.stringContaining('no accessible name')) + }) + it('restores focus to whatever held it before the dialog opened', () => { const trigger = document.createElement('button') document.body.append(trigger) @@ -357,10 +390,10 @@ describe('guardBackNavigation', () => { describe('outside-press gating', () => { it('lets only the topmost dialog answer a backdrop press', () => { mountLayer('dlg', 1) - expect(acceptsBackdropPress('dlg')).toBe(true) + expect(acceptsBackdropPress('dlg', false)).toBe(true) mountLayer('above', 2) - expect(acceptsBackdropPress('dlg')).toBe(false) + expect(acceptsBackdropPress('dlg', false)).toBe(false) }) it('ignores a viewport press that bubbled up from the content', () => { @@ -368,8 +401,142 @@ describe('outside-press gating', () => { const viewport = document.createElement('div') const content = document.createElement('div') - expect(acceptsViewportPress('dlg', { target: viewport, currentTarget: viewport })).toBe(true) - expect(acceptsViewportPress('dlg', { target: content, currentTarget: viewport })).toBe(false) + expect(acceptsViewportPress('dlg', { target: viewport, currentTarget: viewport }, false)).toBe( + true, + ) + expect(acceptsViewportPress('dlg', { target: content, currentTarget: viewport }, false)).toBe( + false, + ) + }) + + it('refuses a backdrop or viewport press whose gesture started inside the window', () => { + mountLayer('dlg', 1) + const viewport = document.createElement('div') + + expect(acceptsBackdropPress('dlg', true)).toBe(false) + expect(acceptsViewportPress('dlg', { target: viewport, currentTarget: viewport }, true)).toBe( + false, + ) + }) +}) + +describe('trackPressOrigin', () => { + const dispatchPointerDown = (target: Element): void => { + target.dispatchEvent(new Event('pointerdown', { bubbles: true })) + } + + it('reports whether the most recent press began inside the tracked element', () => { + const content = document.createElement('div') + const inner = document.createElement('button') + content.append(inner) + const outer = document.createElement('button') + document.body.append(content, outer) + + const tracker = trackPressOrigin(content) + expect(tracker.startedInside()).toBe(false) + + dispatchPointerDown(inner) + expect(tracker.startedInside()).toBe(true) + + dispatchPointerDown(outer) + expect(tracker.startedInside()).toBe(false) + + tracker.dispose() + }) + + it('stops updating once disposed', () => { + const content = document.createElement('div') + const inner = document.createElement('button') + content.append(inner) + document.body.append(content) + + const tracker = trackPressOrigin(content) + tracker.dispose() + dispatchPointerDown(inner) + + expect(tracker.startedInside()).toBe(false) + }) +}) + +describe('watchOutsidePress', () => { + const click = (target: Element): void => { + target.dispatchEvent(new MouseEvent('click', { bubbles: true })) + } + + const setup = ( + startedInside: () => boolean = () => false, + ): { + content: HTMLElement + trigger: HTMLElement + outside: HTMLElement + onOutsidePress: () => void + } => { + const content = mountLayer('dlg', 1) + const trigger = document.createElement('button') + const outside = document.createElement('button') + document.body.append(trigger, outside) + const onOutsidePress = vi.fn() + registered.push(watchOutsidePress('dlg', { content, trigger, startedInside, onOutsidePress })) + return { content, trigger, outside, onOutsidePress } + } + + it('fires for a press outside the window and the trigger', () => { + const { outside, onOutsidePress } = setup() + + click(outside) + + expect(onOutsidePress).toHaveBeenCalledTimes(1) + }) + + it('ignores a press inside the window', () => { + const { content, onOutsidePress } = setup() + + click(content) + + expect(onOutsidePress).not.toHaveBeenCalled() + }) + + it('ignores a press on the trigger — its own press stays a plain toggle', () => { + const { trigger, onOutsidePress } = setup() + + click(trigger) + + expect(onOutsidePress).not.toHaveBeenCalled() + }) + + it('ignores a press once this dialog is no longer topmost', () => { + const { outside, onOutsidePress } = setup() + mountLayer('above', 2) + + click(outside) + + expect(onOutsidePress).not.toHaveBeenCalled() + }) + + it('ignores a press whose gesture started inside the window', () => { + const { outside, onOutsidePress } = setup(() => true) + + click(outside) + + expect(onOutsidePress).not.toHaveBeenCalled() + }) + + it('stops watching once disposed', () => { + const content = mountLayer('dlg', 1) + const outside = document.createElement('button') + document.body.append(outside) + const onOutsidePress = vi.fn() + + const dispose = watchOutsidePress('dlg', { + content, + trigger: null, + startedInside: () => false, + onOutsidePress, + }) + dispose() + click(outside) + + expect(onOutsidePress).not.toHaveBeenCalled() }) }) diff --git a/packages/react/dialog/SPEC.md b/packages/react/dialog/SPEC.md index 1b39064..07515d7 100644 --- a/packages/react/dialog/SPEC.md +++ b/packages/react/dialog/SPEC.md @@ -102,7 +102,7 @@ The root: owns open/close state, renders no DOM. Accepts the core | `role` | `'dialog' \| 'alertdialog'` | `'dialog'` | The ARIA pattern. | | `closeOnEscape` | `boolean` | `true` | Whether Escape closes the dialog. | | `escapeScope` | `'layer' \| 'stack'` | `'layer'` | How far an allowed Escape reaches: this dialog, or its whole stack. | -| `closeOnInteractOutside` | `boolean` | `true` — `false` for `role="alertdialog"` | Whether pressing the backdrop/viewport closes the dialog. | +| `closeOnInteractOutside` | `boolean` | `true` — `false` for `role="alertdialog"` | Whether a press outside the dialog window closes it. | | `animated` | `boolean` | `false` | Keeps the dialog mounted through `data-state="closing"` while its exit animation plays. | | `closeOnBack` | `boolean` | `false` | The browser's Back closes the open dialog instead of navigating (a guard entry in the session history), and Forward reopens what Back closed. | | `onBackNavigation` | `(event?) => void` | — | Fired before a back-navigation dismissal; `preventDefault()` vetoes. | @@ -139,7 +139,11 @@ The layer behind the dialog window; renders nothing when `modal={false}`. ### `Dialog.Viewport` -The positioning + scroll layer around the dialog window. +The positioning + scroll layer around the dialog window. While non-modal it +renders with `pointer-events: none` (the consumer's own `style` wins), so a +press on the empty area falls through to the page instead of being +swallowed; outside presses are then observed at the document level. While +modal it stays the outside-press surface itself. | Prop | Type | Default | Description | | ---------- | ----------------------- | ------- | ---------------------------------- | @@ -147,7 +151,9 @@ The positioning + scroll layer around the dialog window. ### `Dialog.Content` -The dialog window; renders a `
` with the `dialog` role. +The dialog window; renders a `
` with the `dialog` role and +`pointer-events: auto` (the consumer's own `style` wins), so it stays +interactive where a non-modal Viewport lets presses fall through around it. | Prop | Type | Default | Description | | -------------- | -------------------------------- | ----------------- | ------------------------------------------- | diff --git a/packages/react/dialog/src/context.ts b/packages/react/dialog/src/context.ts index 149635a..e21a77c 100644 --- a/packages/react/dialog/src/context.ts +++ b/packages/react/dialog/src/context.ts @@ -15,6 +15,18 @@ export interface DialogContextValue { // sibling parts: Content's stack entry excepts its own backdrop from the // containment so it stays pressable while its dialog is topmost. backdropRef: RefObject + // The rendered Content element, shared so Viewport's non-modal outside- + // press watcher (a document-level listener, since a `pointer-events: none` + // Viewport never receives those presses itself) knows what "inside" means. + contentRef: RefObject + // The rendered Trigger element, shared for the same watcher: its own press + // stays a plain toggle — counting it as outside would close and + // immediately reopen. + triggerRef: RefObject + // Whether the most recent press began inside Content — see + // `trackPressOrigin`. Read by Backdrop and Viewport to refuse a + // text-selection drag that starts inside and releases outside. + pressOriginRef: RefObject<(() => boolean) | null> } export const DialogContext: Context = createContext< diff --git a/packages/react/dialog/src/dialog.tsx b/packages/react/dialog/src/dialog.tsx index 18d1136..725b353 100644 --- a/packages/react/dialog/src/dialog.tsx +++ b/packages/react/dialog/src/dialog.tsx @@ -19,10 +19,14 @@ import type { DialogOptions } from '@dunky.dev/dialog' import { acceptsBackdropPress, acceptsViewportPress, + contentPointerEvents, dialogTrapOptions, guardBackNavigation, openDialogLayer, startExitWindow, + trackPressOrigin, + viewportPointerEvents, + watchOutsidePress, type BackNavigationGuard, } from '@dunky.dev/dom-dialog' import { mergeProps, normalize } from '@dunky.dev/react-state-machine' @@ -46,6 +50,9 @@ export const Dialog: ((props: DialogProps) => ReactNode) & Parts = ({ children, const depth = (useContext(DialogContext)?.depth ?? 0) + 1 const { api, machine } = useDialog(options) const backdropRef = useRef(null) + const contentRef = useRef(null) + const triggerRef = useRef(null) + const pressOriginRef = useRef<(() => boolean) | null>(null) // Read through a ref so the history guard's lifecycle follows the open // state alone: re-arming on every render (fresh callback identities) would @@ -79,7 +86,18 @@ export const Dialog: ((props: DialogProps) => ReactNode) & Parts = ({ children, ) return ( - + {children} ) @@ -95,12 +113,13 @@ export const Trigger: PartComponent = for HTMLButtonElement, DialogTriggerProps >((props, forwardedRef) => { - const { api } = useDialogContext() + const { api, triggerRef } = useDialogContext() + useImperativeHandle(forwardedRef, () => triggerRef.current as HTMLButtonElement) const merged = mergeProps( { type: 'button', ...props }, normalize(api.parts.trigger), ) - return + + Trigger + + + + + + + + + + ) + + // jsdom does no hit-testing, so the blocked-page-click bug itself can't + // be reproduced — assert the mechanism that prevents it: the empty + // viewport area is transparent to pointer events, and the window stays + // reachable regardless. + it('lets pointer events fall through the empty viewport area', () => { + render() + expect(screen.getByTestId('viewport').style.pointerEvents).toBe('none') + expect(screen.getByRole('dialog').style.pointerEvents).toBe('auto') + }) + + it('keeps the viewport itself pressable when modal — it is the outside-press surface there', () => { + render() + expect(screen.getByTestId('viewport').style.pointerEvents).toBe('') + }) + + // With the viewport transparent to pointer events, it never receives a + // press on the empty area to detect as outside — a document-level watch + // is the substitute. + it('dismisses on a press elsewhere in the document', () => { + render() + act(() => screen.getByText('Page button').click()) + expect(screen.queryByRole('dialog')).toBeNull() + }) + + it('does not dismiss on a press inside the content', () => { + render() + act(() => screen.getByText('Action').click()) + expect(screen.queryByRole('dialog')).not.toBeNull() + }) + + it('does not double-fire on the trigger’s own press — it stays a plain toggle', () => { + const onOpenChange = vi.fn() + render( + + Trigger + + + + + + , + ) + act(() => screen.getByText('Trigger').click()) + expect(onOpenChange).toHaveBeenCalledTimes(1) + expect(screen.queryByRole('dialog')).toBeNull() + }) + + it('does not dismiss on a drag that starts inside the content and releases in the document', () => { + render() + fireEvent.pointerDown(screen.getByText('Action')) + act(() => screen.getByText('Page button').click()) + expect(screen.queryByRole('dialog')).not.toBeNull() + }) }) describe('controlled open', () => { @@ -290,6 +389,26 @@ describe('Dialog', () => { expect(dialog.hasAttribute('aria-describedby')).toBe(false) }) + it('warns when the dialog resolves no accessible name', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + render( + + + content + + , + ) + await act(() => new Promise(resolve => setTimeout(resolve, 0))) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('no accessible name')) + }) + + it('does not warn when a Title is rendered', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + render() + await act(() => new Promise(resolve => setTimeout(resolve, 0))) + expect(warn).not.toHaveBeenCalledWith(expect.stringContaining('no accessible name')) + }) + it('renders role=alertdialog when requested', () => { render() expect(screen.queryByRole('alertdialog')).not.toBeNull() diff --git a/packages/solid/dialog/SPEC.md b/packages/solid/dialog/SPEC.md index d9f9200..5e38f0b 100644 --- a/packages/solid/dialog/SPEC.md +++ b/packages/solid/dialog/SPEC.md @@ -107,7 +107,7 @@ The root: owns open/close state, renders no DOM. Accepts the core | `role` | `'dialog' \| 'alertdialog'` | `'dialog'` | The ARIA pattern. | | `closeOnEscape` | `boolean` | `true` | Whether Escape closes the dialog. | | `escapeScope` | `'layer' \| 'stack'` | `'layer'` | How far an allowed Escape reaches: this dialog, or its whole stack. | -| `closeOnInteractOutside` | `boolean` | `true` — `false` for `role="alertdialog"` | Whether pressing the backdrop/viewport closes the dialog. | +| `closeOnInteractOutside` | `boolean` | `true` — `false` for `role="alertdialog"` | Whether a press outside the dialog window closes it. | | `animated` | `boolean` | `false` | Keeps the dialog mounted through `data-state="closing"` while its exit animation plays. | | `closeOnBack` | `boolean` | `false` | The browser's Back closes the open dialog instead of navigating (a guard entry in the session history), and Forward reopens what Back closed. | | `onBackNavigation` | `(event?) => void` | — | Fired before a back-navigation dismissal; `preventDefault()` vetoes. | @@ -144,7 +144,11 @@ The layer behind the dialog window; renders nothing when `modal={false}`. ### `Dialog.Viewport` -The positioning + scroll layer around the dialog window. +The positioning + scroll layer around the dialog window. While non-modal it +renders with `pointer-events: none` (the consumer's own `style` wins), so a +press on the empty area falls through to the page instead of being +swallowed; outside presses are then observed at the document level. While +modal it stays the outside-press surface itself. | Prop | Type | Default | Description | | ---------- | ----------------------- | ------- | ---------------------------------- | @@ -152,7 +156,9 @@ The positioning + scroll layer around the dialog window. ### `Dialog.Content` -The dialog window; renders a `
` with the `dialog` role. +The dialog window; renders a `
` with the `dialog` role and +`pointer-events: auto` (the consumer's own `style` wins), so it stays +interactive where a non-modal Viewport lets presses fall through around it. | Prop | Type | Default | Description | | -------------- | --------------------------------------------------------- | ----------------- | ------------------------------------------------------------------- | diff --git a/packages/solid/dialog/src/context.ts b/packages/solid/dialog/src/context.ts index 2d1ebb5..3ce019a 100644 --- a/packages/solid/dialog/src/context.ts +++ b/packages/solid/dialog/src/context.ts @@ -13,6 +13,17 @@ export interface DialogContextValue { // the containment. A plain box, not a signal: the layer walk reads it in the // same settle that mounts the backdrop, before a signal write would commit. backdropRef: { current: HTMLDivElement | null } + // The rendered Content, shared so Viewport's non-modal outside-press + // watcher (a document-level listener, since a `pointer-events: none` + // Viewport never receives those presses itself) knows what "inside" means. + contentRef: { current: HTMLDivElement | null } + // The rendered Trigger, shared for the same watcher: its own press stays a + // plain toggle — counting it as outside would close and immediately reopen. + triggerRef: { current: HTMLButtonElement | null } + // Whether the most recent press began inside Content — see + // `trackPressOrigin`. Read by Backdrop and Viewport to refuse a + // text-selection drag that starts inside and releases outside. + pressOriginRef: { current: (() => boolean) | null } } // A `null` default: a default-less context throws on the root's optional diff --git a/packages/solid/dialog/src/dialog.tsx b/packages/solid/dialog/src/dialog.tsx index c6c982b..d20cd90 100644 --- a/packages/solid/dialog/src/dialog.tsx +++ b/packages/solid/dialog/src/dialog.tsx @@ -17,10 +17,14 @@ import type { DialogOptions } from '@dunky.dev/dialog' import { acceptsBackdropPress, acceptsViewportPress, + contentPointerEvents, dialogTrapOptions, guardBackNavigation, openDialogLayer, startExitWindow, + trackPressOrigin, + viewportPointerEvents, + watchOutsidePress, type BackNavigationGuard, } from '@dunky.dev/dom-dialog' import { mergeProps, normalize } from '@dunky.dev/solid-state-machine' @@ -53,6 +57,9 @@ export const Dialog: Component & Parts = props => { const depth = (useContext(DialogContext)?.depth ?? 0) + 1 const { api, machine } = useDialog(options) const backdropRef: { current: HTMLDivElement | null } = { current: null } + const contentRef: { current: HTMLDivElement | null } = { current: null } + const triggerRef: { current: HTMLButtonElement | null } = { current: null } + const pressOriginRef: { current: (() => boolean) | null } = { current: null } // The guard lives on the root — it concerns the dialog's openness, not any // rendered part. It spans more than the open state, so it can't be this @@ -80,7 +87,18 @@ export const Dialog: Component & Parts = props => { }) return ( - null, backdropRef }}> + null, + backdropRef, + contentRef, + triggerRef, + pressOriginRef, + }} + > {props.children} ) @@ -93,11 +111,16 @@ export const Dialog: Component & Parts = props => { export interface DialogTriggerProps extends ComponentProps<'button'> {} export const Trigger: Component = props => { - const { api } = useDialogContext() - const rest = omit(props, 'children') + const { api, triggerRef } = useDialogContext() + const rest = omit(props, 'ref', 'children') + onSettled(() => () => (triggerRef.current = null)) return ( @@ -145,7 +168,7 @@ export const Portal: Component = props => { export interface DialogBackdropProps extends ComponentProps<'div'> {} export const Backdrop: Component = props => { - const { api, machine, backdropRef } = useDialogContext() + const { api, machine, backdropRef, pressOriginRef } = useDialogContext() const rest = omit(props, 'ref', 'children') onSettled(() => () => (backdropRef.current = null)) @@ -156,7 +179,8 @@ export const Backdrop: Component = props => { return { ...attrs, onClick: (event: MouseEvent) => { - if (acceptsBackdropPress(machine.context.id)) onClick?.(event) + if (acceptsBackdropPress(machine.context.id, pressOriginRef.current?.() ?? false)) + onClick?.(event) }, } } @@ -184,8 +208,8 @@ export const Backdrop: Component = props => { export interface DialogViewportProps extends ComponentProps<'div'> {} export const Viewport: Component = props => { - const { api, machine } = useDialogContext() - const rest = omit(props, 'children') + const { api, machine, contentRef, triggerRef, pressOriginRef } = useDialogContext() + const rest = omit(props, 'style', 'children') const bindings = (): Record => { const { onClick, ...attrs } = normalize(api.parts.viewport) as { @@ -194,12 +218,47 @@ export const Viewport: Component = props => { return { ...attrs, onClick: (event: MouseEvent) => { - if (acceptsViewportPress(machine.context.id, event)) onClick?.(event) + if (acceptsViewportPress(machine.context.id, event, pressOriginRef.current?.() ?? false)) + onClick?.(event) }, } } - return
(rest, bindings())}>{props.children}
+ // Non-modal: the Viewport's own pointer-events are off (see the style + // below), so it never receives a press on the empty area to detect as + // outside — the document is the only vantage point left. + createEffect( + () => api.open, + open => { + if (!open || machine.context.modal) return + const content = contentRef.current + if (content === null) return + return watchOutsidePress(machine.context.id, { + content, + trigger: triggerRef.current, + startedInside: () => pressOriginRef.current?.() ?? false, + onOutsidePress: event => + untrack(() => { + const { onClick } = normalize(api.parts.viewport) as { + onClick?: (event: MouseEvent) => void + } + onClick?.(event) + }), + }) + }, + ) + + return ( +
(rest, bindings())} + // Non-modal: the page coexists with the dialog, so the empty area + // around the window must let presses fall through rather than swallow + // them. + style={withPointerEvents(props.style, viewportPointerEvents(machine.context.modal))} + > + {props.children} +
+ ) } // ============================================================================= @@ -216,10 +275,23 @@ export interface DialogContentProps extends ComponentProps<'div'> { const resolveInitialFocus = (value: DialogContentProps['initialFocus']): HTMLElement | null => (typeof value === 'function' ? value() : value) ?? null +// The consumer's own style wins over the pointer-events default; a string +// style keeps the default as the earlier (overridable) declaration. +const withPointerEvents = ( + style: ComponentProps<'div'>['style'], + value: 'none' | 'auto' | undefined, +): ComponentProps<'div'>['style'] => { + if (value === undefined) return style + if (typeof style === 'string') return `pointer-events:${value};${style}` + if (typeof style === 'object' && style !== null) return { 'pointer-events': value, ...style } + return { 'pointer-events': value } +} + export const Content: Component = props => { - const { api, machine, depth, container, backdropRef } = useDialogContext() - const rest = omit(props, 'ref', 'initialFocus', 'children') - let contentEl: HTMLDivElement | undefined + const { api, machine, depth, container, backdropRef, contentRef, pressOriginRef } = + useDialogContext() + const rest = omit(props, 'ref', 'initialFocus', 'style', 'children') + onSettled(() => () => (contentRef.current = null)) // The `open` state is the edge, not mount/unmount: an animated dialog stays // mounted through `closing`. The sequence and its inverse are the DOM @@ -227,10 +299,10 @@ export const Content: Component = props => { createEffect( () => api.open, open => { - const content = contentEl - if (!open || content === undefined) return + const content = contentRef.current + if (!open || content === null) return - return openDialogLayer(content, { + const releaseLayer = openDialogLayer(content, { id: machine.context.id, depth, modal: machine.context.modal, @@ -238,6 +310,17 @@ export const Content: Component = props => { initialFocus: untrack(() => resolveInitialFocus(props.initialFocus)), dismiss: () => machine.send({ type: 'close' }), }) + // A click's own target can't tell a text-selection drag from a genuine + // outside press once the browser has collapsed it — see + // `trackPressOrigin`. Shared via context: Backdrop and Viewport are + // separate parts, and both need the answer. + const pressOrigin = trackPressOrigin(content) + pressOriginRef.current = pressOrigin.startedInside + return () => { + pressOriginRef.current = null + pressOrigin.dispose() + releaseLayer() + } }, ) @@ -245,8 +328,8 @@ export const Content: Component = props => { createEffect( () => api.open, open => { - const content = contentEl - if (open || content === undefined) return + const content = contentRef.current + if (open || content === null) return return untrack(() => startExitWindow(content, { @@ -268,7 +351,7 @@ export const Content: Component = props => { ) useFocusTrap( - () => contentEl ?? null, + () => contentRef.current, dialogTrapOptions(machine, () => api.ids.close), ) @@ -278,8 +361,11 @@ export const Content: Component = props => { return (
(rest, normalize(api.parts.content))} + // Always interactive, even where a non-modal Viewport disables pointer + // events to let presses fall through around it. + style={withPointerEvents(props.style, contentPointerEvents)} ref={element => { - contentEl = element + contentRef.current = element applyConsumerRef(props.ref, element) }} > diff --git a/packages/solid/dialog/stories/dialog.stories.tsx b/packages/solid/dialog/stories/dialog.stories.tsx index a1c71ba..d39b85d 100644 --- a/packages/solid/dialog/stories/dialog.stories.tsx +++ b/packages/solid/dialog/stories/dialog.stories.tsx @@ -594,7 +594,8 @@ const ContainedPage = () => { A non-modal layer above the dialog, held out of the containment while its - neighbor article stays in it. Escape closes this layer first. + neighbor article stays in it. Escape — or a press outside it — closes this + layer first. diff --git a/packages/solid/dialog/tests/dialog.test.tsx b/packages/solid/dialog/tests/dialog.test.tsx index 5d0a7f3..ff0c50d 100644 --- a/packages/solid/dialog/tests/dialog.test.tsx +++ b/packages/solid/dialog/tests/dialog.test.tsx @@ -41,6 +41,9 @@ const pressEscape = (): void => { // Auto-cleanup needs vitest globals; this repo runs with globals: false. afterEach(cleanup) +// Spies on globals (console.warn) accumulate across tests otherwise — +// vi.spyOn on an already-spied method returns the same spy, history intact. +afterEach(() => vi.restoreAllMocks()) describe('Dialog', () => { describe('open / close', () => { @@ -150,6 +153,102 @@ describe('Dialog', () => { render(() => ) expect(screen.queryByTestId('backdrop')).toBeNull() }) + + // A text-selection drag starting inside the content and releasing on the + // viewport/backdrop collapses the click's target to whichever it lands + // on — indistinguishable from a genuine outside press by target alone. + it('does not close on a drag that starts inside the content and releases on the viewport', () => { + render(() => ) + fireEvent.pointerDown(screen.getByText('Action')) + press(screen.getByTestId('viewport')) + expect(screen.queryByRole('dialog')).not.toBeNull() + }) + + it('does not close on a drag that starts inside the content and releases on the backdrop', () => { + render(() => ) + fireEvent.pointerDown(screen.getByText('Action')) + press(screen.getByTestId('backdrop')) + expect(screen.queryByRole('dialog')).not.toBeNull() + }) + + it('still closes on a press that starts and ends on the viewport — not a drag', () => { + render(() => ) + fireEvent.pointerDown(screen.getByTestId('viewport')) + press(screen.getByTestId('viewport')) + expect(screen.queryByRole('dialog')).toBeNull() + }) + }) + + describe('non-modal outside interaction', () => { + const NonModalDialog = () => ( + <> + + + Trigger + + + + + + + + + + ) + + // jsdom does no hit-testing, so the blocked-page-click bug itself can't + // be reproduced — assert the mechanism that prevents it: the empty + // viewport area is transparent to pointer events, and the window stays + // reachable regardless. + it('lets pointer events fall through the empty viewport area', () => { + render(() => ) + expect(screen.getByTestId('viewport').style.pointerEvents).toBe('none') + expect(screen.getByRole('dialog').style.pointerEvents).toBe('auto') + }) + + it('keeps the viewport itself pressable when modal — it is the outside-press surface there', () => { + render(() => ) + expect(screen.getByTestId('viewport').style.pointerEvents).toBe('') + }) + + // With the viewport transparent to pointer events, it never receives a + // press on the empty area to detect as outside — a document-level watch + // is the substitute. + it('dismisses on a press elsewhere in the document', () => { + render(() => ) + press(screen.getByText('Page button')) + expect(screen.queryByRole('dialog')).toBeNull() + }) + + it('does not dismiss on a press inside the content', () => { + render(() => ) + press(screen.getByText('Action')) + expect(screen.queryByRole('dialog')).not.toBeNull() + }) + + it('does not double-fire on the trigger’s own press — it stays a plain toggle', () => { + const onOpenChange = vi.fn() + render(() => ( + + Trigger + + + + + + + )) + press(screen.getByText('Trigger')) + expect(onOpenChange).toHaveBeenCalledTimes(1) + expect(screen.queryByRole('dialog')).toBeNull() + }) + + it('does not dismiss on a drag that starts inside the content and releases in the document', () => { + render(() => ) + fireEvent.pointerDown(screen.getByText('Action')) + press(screen.getByText('Page button')) + expect(screen.queryByRole('dialog')).not.toBeNull() + }) }) describe('controlled open', () => { @@ -305,6 +404,28 @@ describe('Dialog', () => { expect(dialog.hasAttribute('aria-describedby')).toBe(false) }) + it('warns when the dialog resolves no accessible name', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + render(() => ( + + + content + + + )) + flush() + await new Promise(resolve => setTimeout(resolve, 0)) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('no accessible name')) + }) + + it('does not warn when a Title is rendered', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + render(() => ) + flush() + await new Promise(resolve => setTimeout(resolve, 0)) + expect(warn).not.toHaveBeenCalledWith(expect.stringContaining('no accessible name')) + }) + it('renders role=alertdialog when requested', () => { render(() => ) expect(screen.queryByRole('alertdialog')).not.toBeNull() From 164c4432ff2423e2b2c22197a0d4a4dbaec443aa Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Thu, 27 Aug 2026 19:46:33 +0200 Subject: [PATCH 2/2] refactor(dom-overlay,dom-press-origin): home the reusable outside-press parts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drag guard and the document-level outside-press watch were born in dom-dialog but nothing about them is dialog's: every light-dismissable layer needs both. Pre-positioned where the next primitive will look — - trackPressOrigin -> @dunky.dev/dom-press-origin, a new util. It imports nothing; the origin answer feeds any outside-press decision. - watchOutsidePress -> @dunky.dev/dom-overlay, next to the stack it queries (topmost-only is its gating), with layer-generic vocabulary (element, not content). viewportPointerEvents / contentPointerEvents stay in dom-dialog: they are values for dialog parts. The substrates now import the utils directly, per the boundary; dom-dialog re-exports nothing it doesn't own. Co-Authored-By: Claude Fable 5 --- .changeset/dialog-drag-never-dismisses.md | 16 ++- .../dialog-nonmodal-pointer-passthrough.md | 16 ++- packages/dom/components/dialog/SPEC.md | 57 ++++---- packages/dom/components/dialog/src/index.ts | 2 - packages/dom/components/dialog/src/press.ts | 2 +- .../dialog/src/watch-outside-press.ts | 37 ------ .../components/dialog/tests/dialog.test.ts | 122 ------------------ packages/dom/utils/overlay/SPEC.md | 20 +++ packages/dom/utils/overlay/src/index.ts | 1 + .../utils/overlay/src/watch-outside-press.ts | 37 ++++++ .../overlay/tests/watch-outside-press.test.ts | 100 ++++++++++++++ packages/dom/utils/press-origin/README.md | 34 +++++ packages/dom/utils/press-origin/SPEC.md | 35 +++++ packages/dom/utils/press-origin/package.json | 38 ++++++ packages/dom/utils/press-origin/src/index.ts | 1 + .../press-origin/src/track-press-origin.ts} | 12 +- .../tests/track-press-origin.test.ts | 45 +++++++ packages/react/dialog/package.json | 2 + packages/react/dialog/src/dialog.tsx | 6 +- packages/solid/dialog/package.json | 2 + packages/solid/dialog/src/dialog.tsx | 6 +- pnpm-lock.yaml | 14 ++ tsconfig.json | 1 + tsdown.config.ts | 1 + 24 files changed, 391 insertions(+), 216 deletions(-) delete mode 100644 packages/dom/components/dialog/src/watch-outside-press.ts create mode 100644 packages/dom/utils/overlay/src/watch-outside-press.ts create mode 100644 packages/dom/utils/overlay/tests/watch-outside-press.test.ts create mode 100644 packages/dom/utils/press-origin/README.md create mode 100644 packages/dom/utils/press-origin/SPEC.md create mode 100644 packages/dom/utils/press-origin/package.json create mode 100644 packages/dom/utils/press-origin/src/index.ts rename packages/dom/{components/dialog/src/press-origin.ts => utils/press-origin/src/track-press-origin.ts} (69%) create mode 100644 packages/dom/utils/press-origin/tests/track-press-origin.test.ts diff --git a/.changeset/dialog-drag-never-dismisses.md b/.changeset/dialog-drag-never-dismisses.md index f028beb..51dc7fd 100644 --- a/.changeset/dialog-drag-never-dismisses.md +++ b/.changeset/dialog-drag-never-dismisses.md @@ -1,4 +1,5 @@ --- +'@dunky.dev/dom-press-origin': minor '@dunky.dev/dom-dialog': minor '@dunky.dev/react-dialog': patch '@dunky.dev/solid-dialog': patch @@ -10,10 +11,11 @@ common ancestor — the viewport or the backdrop — which was indistinguishable from a genuine outside press by the click's target alone, so selecting text across the window's edge closed the dialog and lost the form state with it. -Where a press *began* now decides, not where it ended: `@dunky.dev/dom-dialog` -gains `trackPressOrigin(content)`, which captures each press's origin at -`pointerdown` (before the browser collapses the click target), and -`acceptsBackdropPress` / `acceptsViewportPress` take that as a new -`startedInside` argument and refuse the press when the gesture started -inside the window. A press that starts and ends outside still dismisses as -before. +Where a press *began* now decides, not where it ended. New package: +`@dunky.dev/dom-press-origin` — `trackPressOrigin(element)` captures each +press's origin at `pointerdown`, before the browser collapses the click +target; it is its own util because every light-dismissable layer needs the +same guard. `@dunky.dev/dom-dialog`'s `acceptsBackdropPress` / +`acceptsViewportPress` take the answer as a new `startedInside` argument and +refuse the press when the gesture started inside the window. A press that +starts and ends outside still dismisses as before. diff --git a/.changeset/dialog-nonmodal-pointer-passthrough.md b/.changeset/dialog-nonmodal-pointer-passthrough.md index 0296b74..b3dddca 100644 --- a/.changeset/dialog-nonmodal-pointer-passthrough.md +++ b/.changeset/dialog-nonmodal-pointer-passthrough.md @@ -1,4 +1,5 @@ --- +'@dunky.dev/dom-overlay': minor '@dunky.dev/dom-dialog': minor '@dunky.dev/react-dialog': patch '@dunky.dev/solid-dialog': patch @@ -13,10 +14,13 @@ renders with `pointer-events: none` and the window with presses on the empty area reach the page and the window stays interactive. Outside-press detection moves with it: a transparent Viewport never receives -the press it used to detect, so `@dunky.dev/dom-dialog` gains +the press it used to detect, so `@dunky.dev/dom-overlay` gains `watchOutsidePress` — a document-level watch (the only vantage point that -sees both the portaled dialog and the page) with the same refusals as the -element path: topmost dialog only, the window excepted, and the trigger -excepted so its own press stays a plain toggle instead of a -close-and-reopen. `viewportPointerEvents(modal)` and `contentPointerEvents` -carry the two style values so every DOM substrate applies the same pair. +sees both the portaled layer and the page) with the same refusals as the +element path: topmost layer only, the layer's element excepted, and the +trigger excepted so its own press stays a plain toggle instead of a +close-and-reopen. It lives in the overlay util, not the dialog package, +because any layer whose surface lets presses fall through will need it. +`@dunky.dev/dom-dialog`'s `viewportPointerEvents(modal)` and +`contentPointerEvents` carry the two style values so every DOM substrate +applies the same pair. diff --git a/packages/dom/components/dialog/SPEC.md b/packages/dom/components/dialog/SPEC.md index cb39fc5..c1ffea0 100644 --- a/packages/dom/components/dialog/SPEC.md +++ b/packages/dom/components/dialog/SPEC.md @@ -130,27 +130,28 @@ dialog's to answer: press must have started on the viewport itself, and then the same topmost rule applies. -Both also refuse a press whose gesture began inside the window, from -`trackPressOrigin`. A `click`'s own target can't answer that once the -browser has collapsed it: a mousedown inside the window and a mouseup -outside it fires `click` on their common ancestor — the backdrop or the -viewport, not where the press actually started — so a text-selection drag -that starts inside and releases outside would otherwise read as a genuine -outside press and lose whatever the user was selecting. `trackPressOrigin` -captures the press's origin at `pointerdown`, before that collapse, the -only point it's still recoverable. +Both also refuse a press whose gesture began inside the window. A +`click`'s own target can't answer that once the browser has collapsed it: a +mousedown inside the window and a mouseup outside it fires `click` on their +common ancestor — the backdrop or the viewport, not where the press +actually started — so a text-selection drag that starts inside and releases +outside would otherwise read as a genuine outside press and lose whatever +the user was selecting. The origin comes from +[`@dunky.dev/dom-press-origin`](../../utils/press-origin/SPEC.md)'s +`trackPressOrigin`, which captures it at `pointerdown`, before that +collapse, the only point it's still recoverable — a substrate tracks the +window and feeds `startedInside` into both predicates. A non-modal dialog has no Backdrop, and its Viewport lets a press on the empty area fall through to the page rather than swallow it — `viewportPointerEvents` is `none` there, `contentPointerEvents` on the window stays `auto` regardless. That fall-through means Viewport never -receives those presses to detect them, so `watchOutsidePress` is the -non-modal substitute: a document-level listener, because the document is -the only vantage point that can see both the dialog (portaled out of the -page's own subtree) and the page. It carries the same three refusals — -topmost-only, the window excepted, the trigger excepted (its own press -stays a plain toggle; counting it as outside would close and immediately -reopen) — plus the same started-inside guard as the click-based path. +receives those presses to detect them, so the substrate arms +[`@dunky.dev/dom-overlay`](../../utils/overlay/SPEC.md)'s +`watchOutsidePress` instead — the document-level outside-press watch for +layers whose own surfaces can't see one. It carries the same refusals as +the click-based path (topmost-only, the window excepted, the trigger +excepted, the started-inside guard); the dialog only supplies its pieces. ### Focus trap @@ -160,19 +161,17 @@ part is the cycle's last stop wherever it renders. ## API -| Export | Description | -| ------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------- | -| `domDialogEffects` | Core effects + the document Escape listener, as `DialogEffect` tuples. | -| `openDialogLayer(content, options)` | The open sequence; returns the close sequence. | -| `startExitWindow(content, options)` | Hides and watches the still-painting layer; returns the undo. | -| `guardBackNavigation(options)` | The history guard: report the open state as it changes, release at the end. | -| `acceptsBackdropPress(id, startedInside)` | Whether a backdrop press is this dialog's outside interaction. | -| `acceptsViewportPress(id, event, startedInside)` | Same for the viewport, ignoring presses that bubbled from the content. | -| `trackPressOrigin(content)` | Tracks whether the most recent press began inside `content`; returns `{ startedInside, dispose }`. | -| `watchOutsidePress(id, options)` | The non-modal outside-press surface — a document-level substitute for the Viewport's own click. Returns the undo. | -| `viewportPointerEvents(modal)` | The Viewport's `pointer-events` value: `undefined` while modal, `'none'` otherwise. | -| `contentPointerEvents` | The window's `pointer-events` value — always `'auto'`. | -| `dialogTrapOptions(machine, closeId)` | `TrapFocusOptions` for the dialog window. | +| Export | Description | +| ------------------------------------------------ | ----------------------------------------------------------------------------------- | +| `domDialogEffects` | Core effects + the document Escape listener, as `DialogEffect` tuples. | +| `openDialogLayer(content, options)` | The open sequence; returns the close sequence. | +| `startExitWindow(content, options)` | Hides and watches the still-painting layer; returns the undo. | +| `guardBackNavigation(options)` | The history guard: report the open state as it changes, release at the end. | +| `acceptsBackdropPress(id, startedInside)` | Whether a backdrop press is this dialog's outside interaction. | +| `acceptsViewportPress(id, event, startedInside)` | Same for the viewport, ignoring presses that bubbled from the content. | +| `viewportPointerEvents(modal)` | The Viewport's `pointer-events` value: `undefined` while modal, `'none'` otherwise. | +| `contentPointerEvents` | The window's `pointer-events` value — always `'auto'`. | +| `dialogTrapOptions(machine, closeId)` | `TrapFocusOptions` for the dialog window. | ## Constraints diff --git a/packages/dom/components/dialog/src/index.ts b/packages/dom/components/dialog/src/index.ts index 16ffbe8..4ceaff6 100644 --- a/packages/dom/components/dialog/src/index.ts +++ b/packages/dom/components/dialog/src/index.ts @@ -7,7 +7,5 @@ export { type BackNavigationGuardOptions, } from './back-navigation' export { acceptsBackdropPress, acceptsViewportPress } from './press' -export { trackPressOrigin, type PressOriginTracker } from './press-origin' -export { watchOutsidePress, type WatchOutsidePressOptions } from './watch-outside-press' export { viewportPointerEvents, contentPointerEvents } from './viewport-style' export { dialogTrapOptions } from './focus-trap' diff --git a/packages/dom/components/dialog/src/press.ts b/packages/dom/components/dialog/src/press.ts index 1799220..e6f23fb 100644 --- a/packages/dom/components/dialog/src/press.ts +++ b/packages/dom/components/dialog/src/press.ts @@ -11,7 +11,7 @@ interface PressTarget { * Whether a backdrop press is this dialog's outside interaction. Only the * topmost dialog of a stack answers one — a nested stack dismisses one layer * at a time, the same rule Escape follows. `startedInside` (see - * `trackPressOrigin`) refuses a press whose gesture began inside the + * `trackPressOrigin` in `@dunky.dev/dom-press-origin`) refuses a press whose gesture began inside the * window: a text-selection drag that starts inside and releases on the * backdrop must not read as an outside press. */ diff --git a/packages/dom/components/dialog/src/watch-outside-press.ts b/packages/dom/components/dialog/src/watch-outside-press.ts deleted file mode 100644 index 88b2651..0000000 --- a/packages/dom/components/dialog/src/watch-outside-press.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { isTopmostLayer } from '@dunky.dev/dom-overlay' - -export interface WatchOutsidePressOptions { - /** The dialog window; a press landing inside it is never outside. */ - content: HTMLElement - /** The trigger that opened this dialog, excepted so its own press stays a - * plain toggle — counting it as outside would close and immediately - * reopen. */ - trigger: Element | null - /** Whether the gesture that produced this click began inside `content` — - * see `trackPressOrigin`. A text-selection drag starting inside and - * releasing outside must not read as an outside press either. */ - startedInside: () => boolean - onOutsidePress: (event: MouseEvent) => void -} - -/** - * The non-modal outside-press surface. A non-modal dialog has no Backdrop, - * and its Viewport lets a press on the empty area fall through to the page - * rather than swallow it (see the pointer-events split the substrate - * bindings apply) — which means Viewport itself never receives that press - * to detect it. The document is the only vantage point left that can see - * both the dialog and the page. - */ -export function watchOutsidePress(id: string, options: WatchOutsidePressOptions): () => void { - const onClick = (event: MouseEvent): void => { - if (!isTopmostLayer(id)) return - if (options.startedInside()) return - const target = event.target - if (!(target instanceof Node)) return - if (options.content.contains(target)) return - if (options.trigger?.contains(target) === true) return - options.onOutsidePress(event) - } - document.addEventListener('click', onClick, true) - return () => document.removeEventListener('click', onClick, true) -} diff --git a/packages/dom/components/dialog/tests/dialog.test.ts b/packages/dom/components/dialog/tests/dialog.test.ts index 67ff627..6733bb9 100644 --- a/packages/dom/components/dialog/tests/dialog.test.ts +++ b/packages/dom/components/dialog/tests/dialog.test.ts @@ -18,8 +18,6 @@ import { guardBackNavigation, openDialogLayer, startExitWindow, - trackPressOrigin, - watchOutsidePress, } from '@dunky.dev/dom-dialog' type DialogService = Machine @@ -420,126 +418,6 @@ describe('outside-press gating', () => { }) }) -describe('trackPressOrigin', () => { - const dispatchPointerDown = (target: Element): void => { - target.dispatchEvent(new Event('pointerdown', { bubbles: true })) - } - - it('reports whether the most recent press began inside the tracked element', () => { - const content = document.createElement('div') - const inner = document.createElement('button') - content.append(inner) - const outer = document.createElement('button') - document.body.append(content, outer) - - const tracker = trackPressOrigin(content) - expect(tracker.startedInside()).toBe(false) - - dispatchPointerDown(inner) - expect(tracker.startedInside()).toBe(true) - - dispatchPointerDown(outer) - expect(tracker.startedInside()).toBe(false) - - tracker.dispose() - }) - - it('stops updating once disposed', () => { - const content = document.createElement('div') - const inner = document.createElement('button') - content.append(inner) - document.body.append(content) - - const tracker = trackPressOrigin(content) - tracker.dispose() - dispatchPointerDown(inner) - - expect(tracker.startedInside()).toBe(false) - }) -}) - -describe('watchOutsidePress', () => { - const click = (target: Element): void => { - target.dispatchEvent(new MouseEvent('click', { bubbles: true })) - } - - const setup = ( - startedInside: () => boolean = () => false, - ): { - content: HTMLElement - trigger: HTMLElement - outside: HTMLElement - onOutsidePress: () => void - } => { - const content = mountLayer('dlg', 1) - const trigger = document.createElement('button') - const outside = document.createElement('button') - document.body.append(trigger, outside) - const onOutsidePress = vi.fn() - registered.push(watchOutsidePress('dlg', { content, trigger, startedInside, onOutsidePress })) - return { content, trigger, outside, onOutsidePress } - } - - it('fires for a press outside the window and the trigger', () => { - const { outside, onOutsidePress } = setup() - - click(outside) - - expect(onOutsidePress).toHaveBeenCalledTimes(1) - }) - - it('ignores a press inside the window', () => { - const { content, onOutsidePress } = setup() - - click(content) - - expect(onOutsidePress).not.toHaveBeenCalled() - }) - - it('ignores a press on the trigger — its own press stays a plain toggle', () => { - const { trigger, onOutsidePress } = setup() - - click(trigger) - - expect(onOutsidePress).not.toHaveBeenCalled() - }) - - it('ignores a press once this dialog is no longer topmost', () => { - const { outside, onOutsidePress } = setup() - mountLayer('above', 2) - - click(outside) - - expect(onOutsidePress).not.toHaveBeenCalled() - }) - - it('ignores a press whose gesture started inside the window', () => { - const { outside, onOutsidePress } = setup(() => true) - - click(outside) - - expect(onOutsidePress).not.toHaveBeenCalled() - }) - - it('stops watching once disposed', () => { - const content = mountLayer('dlg', 1) - const outside = document.createElement('button') - document.body.append(outside) - const onOutsidePress = vi.fn() - - const dispose = watchOutsidePress('dlg', { - content, - trigger: null, - startedInside: () => false, - onOutsidePress, - }) - dispose() - click(outside) - - expect(onOutsidePress).not.toHaveBeenCalled() - }) -}) - describe('dialogTrapOptions', () => { it('traps only while modal and topmost', () => { const service = build({ defaultOpen: true }) diff --git a/packages/dom/utils/overlay/SPEC.md b/packages/dom/utils/overlay/SPEC.md index 139b251..7a58bca 100644 --- a/packages/dom/utils/overlay/SPEC.md +++ b/packages/dom/utils/overlay/SPEC.md @@ -72,6 +72,25 @@ one; the predicates are [`@dunky.dev/dom-element`](../element/SPEC.md)'s `isRendered` and `isFocusable`, shared with the focus trap so the two can't disagree on what counts. +### Outside presses without a surface + +A layer's own elements usually detect an outside press — a backdrop click, a +press on the viewport gutter. A layer that coexists with the page has +neither: no backdrop renders, and its viewport lets presses fall through +(`pointer-events: none`) precisely so the page stays interactive — which +also means the viewport never receives the press it would have detected. +`watchOutsidePress` is the substitute: a document-level listener, because +the document is the only vantage point that sees both the layer (portaled +out of the page's own subtree) and the page. + +It refuses four ways: only the topmost layer of the stack answers; a press +inside the layer's element is never outside; the trigger that opened the +layer is excepted, so its own press stays a plain toggle rather than a +close-and-immediately-reopen; and a press whose gesture _began_ inside the +element is refused via the caller-supplied `startedInside` (see +[`@dunky.dev/dom-press-origin`](../press-origin/SPEC.md)) — a +text-selection drag ending outside is not an outside press. + ### The exit window A closing overlay has already left the stack — the page beneath is live @@ -94,6 +113,7 @@ again — but keeps painting until its exit visual finishes: | Export | Description | | ------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------- | | `registerLayer(layer)` | Joins the shared stack and syncs containment; returns the disposer that restores it. | +| `watchOutsidePress(id, options)` | Document-level outside-press watch for a layer whose own surfaces can't detect one. Returns the undo. | | `Layer` | `OverlayLayer` + `element`, `modal`, an optional `backdrop` getter, and an optional `dismiss`. | | `isTopmostLayer(id)` | Whether the layer owns Escape and the focus trap right now. | | `layersBelow(id)` | The layers stacked beneath, topmost first — the unwinding order for a stack-scoped dismissal. | diff --git a/packages/dom/utils/overlay/src/index.ts b/packages/dom/utils/overlay/src/index.ts index 1753d05..d62237f 100644 --- a/packages/dom/utils/overlay/src/index.ts +++ b/packages/dom/utils/overlay/src/index.ts @@ -1,4 +1,5 @@ export { registerLayer, isTopmostLayer, layersBelow, type Layer } from './stack' +export { watchOutsidePress, type WatchOutsidePressOptions } from './watch-outside-press' export { getInitialFocus } from './get-initial-focus' export { watchExitAnimation } from './watch-exit-animation' export { hideExitingLayer } from './hide-exiting-layer' diff --git a/packages/dom/utils/overlay/src/watch-outside-press.ts b/packages/dom/utils/overlay/src/watch-outside-press.ts new file mode 100644 index 0000000..a7c1dae --- /dev/null +++ b/packages/dom/utils/overlay/src/watch-outside-press.ts @@ -0,0 +1,37 @@ +import { isTopmostLayer } from './stack' + +export interface WatchOutsidePressOptions { + /** The layer's element; a press landing inside it is never outside. */ + element: HTMLElement + /** The trigger that opened this layer, excepted so its own press stays a + * plain toggle — counting it as outside would close and immediately + * reopen. */ + trigger: Element | null + /** Whether the gesture that produced this click began inside `element` — + * see `trackPressOrigin` in `@dunky.dev/dom-press-origin`. A + * text-selection drag starting inside and releasing outside must not read + * as an outside press either. */ + startedInside: () => boolean + onOutsidePress: (event: MouseEvent) => void +} + +/** + * The outside-press watch for a layer whose own surfaces can't detect one — + * a non-modal layer has no backdrop, and a viewport that lets presses fall + * through to the page (`pointer-events: none`) never receives them itself. + * The document is the only vantage point left that can see both the layer + * (portaled out of the page's own subtree) and the page. + */ +export function watchOutsidePress(id: string, options: WatchOutsidePressOptions): () => void { + const onClick = (event: MouseEvent): void => { + if (!isTopmostLayer(id)) return + if (options.startedInside()) return + const target = event.target + if (!(target instanceof Node)) return + if (options.element.contains(target)) return + if (options.trigger?.contains(target) === true) return + options.onOutsidePress(event) + } + document.addEventListener('click', onClick, true) + return () => document.removeEventListener('click', onClick, true) +} diff --git a/packages/dom/utils/overlay/tests/watch-outside-press.test.ts b/packages/dom/utils/overlay/tests/watch-outside-press.test.ts new file mode 100644 index 0000000..8cc1e75 --- /dev/null +++ b/packages/dom/utils/overlay/tests/watch-outside-press.test.ts @@ -0,0 +1,100 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from 'vitest' +import { registerLayer, watchOutsidePress } from '@dunky.dev/dom-overlay' + +const registered: Array<() => void> = [] + +const mountLayer = (id: string, depth: number): HTMLElement => { + const element = document.createElement('div') + document.body.append(element) + registered.push(registerLayer({ id, depth, element, modal: false, backdrop: () => null })) + return element +} + +const click = (target: Element): void => { + target.dispatchEvent(new MouseEvent('click', { bubbles: true })) +} + +afterEach(() => { + for (const undo of registered) undo() + registered.length = 0 + document.body.innerHTML = '' +}) + +describe('watchOutsidePress', () => { + const setup = ( + startedInside: () => boolean = () => false, + ): { + element: HTMLElement + trigger: HTMLElement + outside: HTMLElement + onOutsidePress: () => void + } => { + const element = mountLayer('layer', 1) + const trigger = document.createElement('button') + const outside = document.createElement('button') + document.body.append(trigger, outside) + const onOutsidePress = vi.fn() + registered.push(watchOutsidePress('layer', { element, trigger, startedInside, onOutsidePress })) + return { element, trigger, outside, onOutsidePress } + } + + it('fires for a press outside the layer and the trigger', () => { + const { outside, onOutsidePress } = setup() + + click(outside) + + expect(onOutsidePress).toHaveBeenCalledTimes(1) + }) + + it('ignores a press inside the layer', () => { + const { element, onOutsidePress } = setup() + + click(element) + + expect(onOutsidePress).not.toHaveBeenCalled() + }) + + it('ignores a press on the trigger — its own press stays a plain toggle', () => { + const { trigger, onOutsidePress } = setup() + + click(trigger) + + expect(onOutsidePress).not.toHaveBeenCalled() + }) + + it('ignores a press once this layer is no longer topmost', () => { + const { outside, onOutsidePress } = setup() + mountLayer('above', 2) + + click(outside) + + expect(onOutsidePress).not.toHaveBeenCalled() + }) + + it('ignores a press whose gesture started inside the layer', () => { + const { outside, onOutsidePress } = setup(() => true) + + click(outside) + + expect(onOutsidePress).not.toHaveBeenCalled() + }) + + it('stops watching once disposed', () => { + const element = mountLayer('layer', 1) + const outside = document.createElement('button') + document.body.append(outside) + const onOutsidePress = vi.fn() + + const dispose = watchOutsidePress('layer', { + element, + trigger: null, + startedInside: () => false, + onOutsidePress, + }) + dispose() + click(outside) + + expect(onOutsidePress).not.toHaveBeenCalled() + }) +}) diff --git a/packages/dom/utils/press-origin/README.md b/packages/dom/utils/press-origin/README.md new file mode 100644 index 0000000..ec68f01 --- /dev/null +++ b/packages/dom/utils/press-origin/README.md @@ -0,0 +1,34 @@ +# @dunky.dev/dom-press-origin + +Framework-free tracking of where a pointer press began — kept past the +browser's click-target collapse. + +A mousedown inside an element and a mouseup outside it fire `click` on their +common ancestor, not where the press started. Any "click outside dismisses" +surface misreads a text-selection drag because of that collapse; this util +captures the origin at `pointerdown`, the last moment it is still observable. + +## Install + +```sh +npm install @dunky.dev/dom-press-origin +``` + +## Usage + +```ts +import { trackPressOrigin } from '@dunky.dev/dom-press-origin' + +const tracker = trackPressOrigin(panel) + +document.addEventListener('click', event => { + // A drag that began inside the panel is not an outside press, + // wherever the click's own target ended up. + if (tracker.startedInside()) return + if (!(event.target instanceof Node) || panel.contains(event.target)) return + dismiss() +}) + +// later +tracker.dispose() +``` diff --git a/packages/dom/utils/press-origin/SPEC.md b/packages/dom/utils/press-origin/SPEC.md new file mode 100644 index 0000000..0259f77 --- /dev/null +++ b/packages/dom/utils/press-origin/SPEC.md @@ -0,0 +1,35 @@ +# SPEC / DOM / Press origin + +## Overview + +Framework-free tracking of where a pointer press began. The browser answers +"where did this press _end_" for free; where it _began_ is gone by the time +`click` fires — a mousedown in one place and a mouseup in another dispatch +`click` on their common ancestor, and nothing on the event says the gesture +started somewhere else. Any surface that treats a click as an "outside press" +misreads a text-selection drag because of exactly that collapse; this package +recovers the origin by capturing it at `pointerdown`, the last moment it is +still observable. + +## Behavior + +- `trackPressOrigin(element)` listens for `pointerdown` on the document + (capture phase) and remembers whether the press began inside `element`. +- `startedInside()` answers for the most recent press — it is a reading of + the latest gesture, not a log. +- Before any press is observed, the answer is `false`. +- `dispose()` detaches the listener; the last answer freezes. + +## API + +| Export | Description | +| --------------------------- | ----------------------------------------------------------------- | +| `trackPressOrigin(element)` | Starts tracking; returns `{ startedInside, dispose }`. | +| `PressOriginTracker` | The returned pair: `startedInside(): boolean`, `dispose(): void`. | + +## Constraints + +- No framework import, and nothing from this repo — the util sits at the + bottom of the DOM layer. +- One tracker per tracked element; consumers that need the answer in several + places share the `startedInside` reference rather than tracking twice. diff --git a/packages/dom/utils/press-origin/package.json b/packages/dom/utils/press-origin/package.json new file mode 100644 index 0000000..3ee035a --- /dev/null +++ b/packages/dom/utils/press-origin/package.json @@ -0,0 +1,38 @@ +{ + "name": "@dunky.dev/dom-press-origin", + "version": "0.0.0", + "description": "Framework-free press-origin tracking — where a pointer press began, kept past the click-target collapse.", + "license": "MIT", + "repository": { + "type": "git", + "url": "git+https://github.com/dunky-dev/ui.git", + "directory": "packages/dom/utils/press-origin" + }, + "files": [ + "dist", + "src", + "SPEC.md" + ], + "type": "module", + "sideEffects": false, + "main": "./src/index.ts", + "types": "./src/index.ts", + "exports": { + ".": "./src/index.ts" + }, + "publishConfig": { + "main": "./dist/index.js", + "module": "./dist/index.js", + "types": "./dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js" + } + }, + "access": "public" + }, + "scripts": { + "build": "tsdown" + } +} diff --git a/packages/dom/utils/press-origin/src/index.ts b/packages/dom/utils/press-origin/src/index.ts new file mode 100644 index 0000000..ef8f89c --- /dev/null +++ b/packages/dom/utils/press-origin/src/index.ts @@ -0,0 +1 @@ +export { trackPressOrigin, type PressOriginTracker } from './track-press-origin' diff --git a/packages/dom/components/dialog/src/press-origin.ts b/packages/dom/utils/press-origin/src/track-press-origin.ts similarity index 69% rename from packages/dom/components/dialog/src/press-origin.ts rename to packages/dom/utils/press-origin/src/track-press-origin.ts index 36dcdce..d3e56d0 100644 --- a/packages/dom/components/dialog/src/press-origin.ts +++ b/packages/dom/utils/press-origin/src/track-press-origin.ts @@ -7,22 +7,22 @@ export interface PressOriginTracker { } /** - * Tracks whether the most recent pointer press began inside `content`. A + * Tracks whether the most recent pointer press began inside `element`. A * click's own target can't answer that once the browser has collapsed it: a - * mousedown inside the window and a mouseup outside it fires `click` on + * mousedown inside the element and a mouseup outside it fires `click` on * their common ancestor, not on where the press actually started — a * text-selection drag then reads as a fresh press on whatever sits above * that ancestor. Capturing at pointerdown, before the collapse, is the only * way to recover it. */ -export function trackPressOrigin(content: HTMLElement): PressOriginTracker { - let insideContent = false +export function trackPressOrigin(element: HTMLElement): PressOriginTracker { + let insideElement = false const onPointerDown = (event: PointerEvent): void => { - insideContent = event.target instanceof Node && content.contains(event.target) + insideElement = event.target instanceof Node && element.contains(event.target) } document.addEventListener('pointerdown', onPointerDown, true) return { - startedInside: () => insideContent, + startedInside: () => insideElement, dispose: () => document.removeEventListener('pointerdown', onPointerDown, true), } } diff --git a/packages/dom/utils/press-origin/tests/track-press-origin.test.ts b/packages/dom/utils/press-origin/tests/track-press-origin.test.ts new file mode 100644 index 0000000..84829e7 --- /dev/null +++ b/packages/dom/utils/press-origin/tests/track-press-origin.test.ts @@ -0,0 +1,45 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it } from 'vitest' +import { trackPressOrigin } from '@dunky.dev/dom-press-origin' + +const dispatchPointerDown = (target: Element): void => { + target.dispatchEvent(new Event('pointerdown', { bubbles: true })) +} + +afterEach(() => { + document.body.innerHTML = '' +}) + +describe('trackPressOrigin', () => { + it('reports whether the most recent press began inside the tracked element', () => { + const element = document.createElement('div') + const inner = document.createElement('button') + element.append(inner) + const outer = document.createElement('button') + document.body.append(element, outer) + + const tracker = trackPressOrigin(element) + expect(tracker.startedInside()).toBe(false) + + dispatchPointerDown(inner) + expect(tracker.startedInside()).toBe(true) + + dispatchPointerDown(outer) + expect(tracker.startedInside()).toBe(false) + + tracker.dispose() + }) + + it('stops updating once disposed', () => { + const element = document.createElement('div') + const inner = document.createElement('button') + element.append(inner) + document.body.append(element) + + const tracker = trackPressOrigin(element) + tracker.dispose() + dispatchPointerDown(inner) + + expect(tracker.startedInside()).toBe(false) + }) +}) diff --git a/packages/react/dialog/package.json b/packages/react/dialog/package.json index 2d9d979..5fdd9e7 100644 --- a/packages/react/dialog/package.json +++ b/packages/react/dialog/package.json @@ -38,6 +38,8 @@ "dependencies": { "@dunky.dev/dialog": "workspace:*", "@dunky.dev/dom-dialog": "workspace:*", + "@dunky.dev/dom-overlay": "workspace:*", + "@dunky.dev/dom-press-origin": "workspace:*", "@dunky.dev/react-state-machine": "^0.3.4", "@dunky.dev/react-use-focus-trap": "workspace:*", "@dunky.dev/react-use-scroll-lock": "workspace:*" diff --git a/packages/react/dialog/src/dialog.tsx b/packages/react/dialog/src/dialog.tsx index 725b353..26b2760 100644 --- a/packages/react/dialog/src/dialog.tsx +++ b/packages/react/dialog/src/dialog.tsx @@ -16,6 +16,8 @@ import { useFocusTrap } from '@dunky.dev/react-use-focus-trap' import { useScrollLock } from '@dunky.dev/react-use-scroll-lock' import type { DialogOptions } from '@dunky.dev/dialog' +import { watchOutsidePress } from '@dunky.dev/dom-overlay' +import { trackPressOrigin } from '@dunky.dev/dom-press-origin' import { acceptsBackdropPress, acceptsViewportPress, @@ -24,9 +26,7 @@ import { guardBackNavigation, openDialogLayer, startExitWindow, - trackPressOrigin, viewportPointerEvents, - watchOutsidePress, type BackNavigationGuard, } from '@dunky.dev/dom-dialog' import { mergeProps, normalize } from '@dunky.dev/react-state-machine' @@ -213,7 +213,7 @@ export const Viewport: PartComponent = forw const content = contentRef.current if (content === null) return return watchOutsidePress(machine.context.id, { - content, + element: content, trigger: triggerRef.current, startedInside: () => pressOriginRef.current?.() ?? false, onOutsidePress: event => onClickRef.current?.(event as unknown as MouseEvent), diff --git a/packages/solid/dialog/package.json b/packages/solid/dialog/package.json index c885d16..6b50e18 100644 --- a/packages/solid/dialog/package.json +++ b/packages/solid/dialog/package.json @@ -38,6 +38,8 @@ "dependencies": { "@dunky.dev/dialog": "workspace:*", "@dunky.dev/dom-dialog": "workspace:*", + "@dunky.dev/dom-overlay": "workspace:*", + "@dunky.dev/dom-press-origin": "workspace:*", "@dunky.dev/solid-state-machine": "^0.3.0", "@dunky.dev/solid-use-focus-trap": "workspace:*", "@dunky.dev/solid-use-scroll-lock": "workspace:*" diff --git a/packages/solid/dialog/src/dialog.tsx b/packages/solid/dialog/src/dialog.tsx index d20cd90..7a85175 100644 --- a/packages/solid/dialog/src/dialog.tsx +++ b/packages/solid/dialog/src/dialog.tsx @@ -14,6 +14,8 @@ import { useFocusTrap } from '@dunky.dev/solid-use-focus-trap' import { useScrollLock } from '@dunky.dev/solid-use-scroll-lock' import type { DialogOptions } from '@dunky.dev/dialog' +import { watchOutsidePress } from '@dunky.dev/dom-overlay' +import { trackPressOrigin } from '@dunky.dev/dom-press-origin' import { acceptsBackdropPress, acceptsViewportPress, @@ -22,9 +24,7 @@ import { guardBackNavigation, openDialogLayer, startExitWindow, - trackPressOrigin, viewportPointerEvents, - watchOutsidePress, type BackNavigationGuard, } from '@dunky.dev/dom-dialog' import { mergeProps, normalize } from '@dunky.dev/solid-state-machine' @@ -234,7 +234,7 @@ export const Viewport: Component = props => { const content = contentRef.current if (content === null) return return watchOutsidePress(machine.context.id, { - content, + element: content, trigger: triggerRef.current, startedInside: () => pressOriginRef.current?.() ?? false, onOutsidePress: event => diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2022777..24315c5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -106,6 +106,8 @@ importers: specifier: workspace:* version: link:../../../core/utils/overlay + packages/dom/utils/press-origin: {} + packages/dom/utils/scroll-lock: {} packages/native: @@ -220,6 +222,12 @@ importers: '@dunky.dev/dom-dialog': specifier: workspace:* version: link:../../dom/components/dialog + '@dunky.dev/dom-overlay': + specifier: workspace:* + version: link:../../dom/utils/overlay + '@dunky.dev/dom-press-origin': + specifier: workspace:* + version: link:../../dom/utils/press-origin '@dunky.dev/react-state-machine': specifier: ^0.3.4 version: 0.3.4(react@19.2.7) @@ -322,6 +330,12 @@ importers: '@dunky.dev/dom-dialog': specifier: workspace:* version: link:../../dom/components/dialog + '@dunky.dev/dom-overlay': + specifier: workspace:* + version: link:../../dom/utils/overlay + '@dunky.dev/dom-press-origin': + specifier: workspace:* + version: link:../../dom/utils/press-origin '@dunky.dev/solid-state-machine': specifier: ^0.3.0 version: 0.3.0(solid-js@2.0.0-rc.1) diff --git a/tsconfig.json b/tsconfig.json index 4e39ec3..d0572cb 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -24,6 +24,7 @@ "@dunky.dev/dom-overlay": ["./packages/dom/utils/overlay/src"], "@dunky.dev/dom-focus-trap": ["./packages/dom/utils/focus-trap/src"], "@dunky.dev/dom-element": ["./packages/dom/utils/element/src"], + "@dunky.dev/dom-press-origin": ["./packages/dom/utils/press-origin/src"], "@dunky.dev/browser-navigation": ["./packages/dom/utils/navigation/src"], "@dunky.dev/dom-scroll-lock": ["./packages/dom/utils/scroll-lock/src"], "@dunky.dev/react-use-focus-trap": ["./packages/react/hooks/use-focus-trap/src"], diff --git a/tsdown.config.ts b/tsdown.config.ts index 2704eb7..38e3747 100644 --- a/tsdown.config.ts +++ b/tsdown.config.ts @@ -18,6 +18,7 @@ export default defineConfig({ 'packages/dom/utils/focus-trap', 'packages/dom/utils/overlay', 'packages/dom/utils/navigation', + 'packages/dom/utils/press-origin', 'packages/dom/utils/scroll-lock', 'packages/native/dialog', 'packages/react/dialog',