diff --git a/.changeset/back-guard-batched-release.md b/.changeset/back-guard-batched-release.md new file mode 100644 index 0000000..f98470f --- /dev/null +++ b/.changeset/back-guard-batched-release.md @@ -0,0 +1,23 @@ +--- +'@dunky.dev/browser-navigation': patch +--- + +Fix: releasing a whole guarded stack in one turn (close-all, a route change, +an unmounting subtree) only consumed the topmost guard's entry — each entry +beneath stayed behind and silently swallowed a later browser Back. + +```ts +const releaseOuter = interceptBackNavigation(() => closeOuter()) +const releaseInner = interceptBackNavigation(() => closeInner()) + +// "close all" — both released in the same turn +releaseInner() +releaseOuter() + +history.back() +// before: ❌ nothing happens — spent on outer's leftover entry +// after: ✅ leaves the page — every freed entry was consumed +``` + +Release order doesn't matter, and an entry genuinely buried under later +in-app navigation is still left alone. diff --git a/.changeset/back-guard-chained-consumption.md b/.changeset/back-guard-chained-consumption.md new file mode 100644 index 0000000..fbbf80f --- /dev/null +++ b/.changeset/back-guard-chained-consumption.md @@ -0,0 +1,16 @@ +--- +'@dunky.dev/browser-navigation': patch +--- + +Two `interceptBackNavigation` hardenings: + +- Released entries are now consumed one traversal at a time — a chain of + single pops — instead of one `history.go(-n)` jump. Entries below the + current one are opaque, so a multi-step jump could cross history entries + the app planted itself; the chain stops at the first entry that isn't the + guard's to spend. A released entry buried beneath a live layer is also no + longer able to swallow a Back: the press that surfaces it unwinds the live + layer and consumes the dead entry in one go. +- An `onBack` that throws now counts as a decline: the guard re-arms so the + next Back still reaches the layer, and the error propagates instead of + aborting the unwind in an inconsistent state. diff --git a/.changeset/dialog-escape-scope.md b/.changeset/dialog-escape-scope.md new file mode 100644 index 0000000..c567db0 --- /dev/null +++ b/.changeset/dialog-escape-scope.md @@ -0,0 +1,33 @@ +--- +'@dunky.dev/overlay': minor +'@dunky.dev/dom-overlay': minor +'@dunky.dev/dom-dialog': minor +'@dunky.dev/dialog': minor +'@dunky.dev/react-dialog': minor +'@dunky.dev/solid-dialog': minor +--- + +`escapeScope` now exists. It was documented in the dialog specs — one layer per +Escape by default, or the whole stack — but no package implemented it, so +passing it did nothing. + +```tsx +// One press closes this dialog and every layer it was opened from. + +``` + +Only the dialog that receives the Escape gates and vetoes it: its +`closeOnEscape` and `onEscapeKeyDown` decide, exactly as before. Once allowed, +the layers beneath receive a plain close — their own dismissal settings are not +consulted again — unwinding top-down, so focus lands where it was before the +bottom-most dialog opened. A vetoed Escape leaves the whole stack standing. + +The mechanics are shared rather than per-dialog: the layer stack gained +`below(id)` (`@dunky.dev/overlay`) and `layersBelow(id)` plus an optional +`Layer.dismiss` (`@dunky.dev/dom-overlay`), so any overlay family can offer a +stack-scoped dismissal on the same stack. A layer that registers no `dismiss` +opts out and stays open, which is what keeps a stack that mixes primitives from +being closed out from under them. + +The specs also described a stack-scoped Close _press_; nothing implements that, +so the claim is removed rather than left standing. diff --git a/.changeset/dialog-forward-reopens.md b/.changeset/dialog-forward-reopens.md new file mode 100644 index 0000000..c90ea8b --- /dev/null +++ b/.changeset/dialog-forward-reopens.md @@ -0,0 +1,60 @@ +--- +'@dunky.dev/browser-navigation': minor +'@dunky.dev/dom-dialog': minor +'@dunky.dev/dialog': minor +'@dunky.dev/react-dialog': minor +'@dunky.dev/solid-dialog': minor +--- + +`closeOnBack` is now symmetric: the browser's Forward reopens what Back +closed. The history entry a Back press spends survives in the forward stack +and keeps marking the dialog's open ground — traversing forward into it +reopens the dialog, guarded again for the next Back. Reopening through the +trigger instead plants a fresh entry, exactly like navigating after a Back. +No new setting: back-close and forward-reopen are one behavior, so the +existing `closeOnBack` gates both. Both DOM substrates get it — React and +Solid — from the same code. + +The reopen follows the shared dismissal contract — a new +`onForwardNavigation` callback fires first and `preventDefault()` vetoes, +and a controlled dialog only records the intent: + +```tsx + { + // e.g. decline the history-driven reopen while a form is mid-submit + if (submitting) event?.preventDefault?.() + }} +> +``` + +A nested dialog comes back too. Closing the layer it was opened from unmounts +it — machine and all — so the ground it lost to a Back press has no owner left +to reopen it. It reopens anyway: the ground belongs to the dialog's place in +the stack rather than to the instance that planted it, so the dialog that +comes back with its parent recognizes it. Two dialogs at the same place can't +be told apart, and then neither reopens. The same recognition survives a +reload, so a traversal back into that ground reopens the dialog even after the +page went away. + +Under the hood, `interceptBackNavigation(onBack, options?)` takes its optional +callbacks as an object and grew `claim`, the name for that ground, plus a +`watchSpentEntry(claim, reopen)` for a closed layer waiting to be recognized. +A Back-closed guard parks instead of dropping, a traversal re-entering its +spent entry asks the layer to reopen, and the guard re-arms on that entry in +place. A layer that passes neither option behaves exactly as before. A layer +that _closed_ gave its ground up on purpose and nothing reopens from it — +Forward never undoes a dismissal the user made deliberately. + +`guardBackNavigation` (`@dunky.dev/dom-dialog`) now returns +`{ sync, release }` rather than a bare disposer: the guard outlives the open +state — that is the whole point of the Forward watch — so a host reports +every change through `sync(open)` and ends the episode with `release()`. +Whether a close parks the registration or releases it stays a DOM-layer +decision, made once for every substrate. + +One web-mechanics caveat, spec'd in the navigation util and both DOM +bindings: a controlled dialog's Back-close is completed by the consumer +rather than by the press, so its entry is consumed and Forward has nothing to +re-enter. diff --git a/.changeset/focus-trap-rendered-walk.md b/.changeset/focus-trap-rendered-walk.md new file mode 100644 index 0000000..cf250d4 --- /dev/null +++ b/.changeset/focus-trap-rendered-walk.md @@ -0,0 +1,17 @@ +--- +'@dunky.dev/dom-focus-trap': patch +--- + +Two fixes to which elements the trap's Tab cycle visits: + +- **Rendered-ness is now decided by a computed-style walk instead of + `Element.checkVisibility()`.** The API is recent (Chrome/Edge 105+, + Firefox 106+, Safari 17.4+), and the trap resolves focusables after the Tab + keydown's `preventDefault()` — on a browser without it, the resulting throw + left Tab dead entirely. The walk checks the same conditions (`hidden` + attribute, `visibility: hidden`, `display: none` on the element or an + ancestor) and works everywhere. +- **`iframe` and `details > summary` now participate in the cycle.** Browsers + tab to both, but the trap — which steps focus itself — skipped them, making + them unreachable by keyboard while trapped. Only a details' first summary is + matched, since that is the disclosure widget browsers focus. diff --git a/.changeset/focus-trap-tab-stop-fidelity.md b/.changeset/focus-trap-tab-stop-fidelity.md index fb3d936..0bf1432 100644 --- a/.changeset/focus-trap-tab-stop-fidelity.md +++ b/.changeset/focus-trap-tab-stop-fidelity.md @@ -13,9 +13,8 @@ Three fixes, all consumer-visible: trap. Initial focus on open remains the caller's job. - **Non-rendered elements no longer enter the cycle.** Elements hidden via the `hidden` attribute, `display: none` (own or ancestor), or - `visibility: hidden` are filtered out with `Element.checkVisibility()`. - Focusing a non-rendered element is a no-op, so a hidden element in the cycle - used to stall the trap on it. + `visibility: hidden` are filtered out. Focusing a non-rendered element is a + no-op, so a hidden element in the cycle used to stall the trap on it. - **A same-name radio group is one tab stop.** Per the APG radio group pattern, the stop is the checked radio, else the group's first; groups are scoped by name and form owner. The trap steps focus itself, so it now diff --git a/.changeset/overlay-ordered-layers.md b/.changeset/overlay-ordered-layers.md new file mode 100644 index 0000000..6b294af --- /dev/null +++ b/.changeset/overlay-ordered-layers.md @@ -0,0 +1,27 @@ +--- +'@dunky.dev/overlay': minor +'@dunky.dev/dom-overlay': patch +--- + +Assistive-tech containment no longer lapses while a non-modal layer is open +above a modal one. + +Containment now follows the topmost **modal** layer rather than the topmost +layer. The ordinary layers — a select menu, a combobox list, a tooltip, a +context menu — are non-modal and live inside dialogs; opening one used to +release the dialog's containment, leaving the page behind reachable by +pointer, keyboard, and screen reader for exactly as long as someone was +interacting with the menu. The layers stacked above the modal one are held +out of the hiding — they portal to the body as siblings of the dialog, so +without the exception the containment would inert the very layer the user is +in. Topmost keeps its meaning: a non-modal layer above still owns Escape and +the focus trap; only containment stays put. + +To support this, the agnostic stack gains a public `ordered()` method +returning every layer topmost first — the host needs to look past the top of +the stack, while modality stays a host concept: + +```ts +const stack = createLayerStack() +stack.ordered() // every layer, topmost first +``` diff --git a/.changeset/scroll-lock-overflow-per-axis.md b/.changeset/scroll-lock-overflow-per-axis.md new file mode 100644 index 0000000..32ff5e5 --- /dev/null +++ b/.changeset/scroll-lock-overflow-per-axis.md @@ -0,0 +1,16 @@ +--- +'@dunky.dev/dom-scroll-lock': patch +--- + +`lockScroll` now saves, hides, and restores `overflow` per axis +(`overflow-x` / `overflow-y`), never via the shorthand. Per CSSOM the +`overflow` shorthand serializes back to `''` unless both longhands are set, +so a container that declares its scrolling on one axis only — + +```tsx +
+``` + +— saved as "unset"; release then removed the consumer's own declaration and +the container stopped scrolling permanently. Restore now returns the inline +style to exactly what the first holder saw, as the contract promises. diff --git a/.lintstagedrc.json b/.lintstagedrc.json deleted file mode 100644 index 099f6e3..0000000 --- a/.lintstagedrc.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "*.{ts,tsx}": ["oxlint --fix", "oxfmt"] -} diff --git a/.lintstagedrc.ts b/.lintstagedrc.ts new file mode 100644 index 0000000..59e0b21 --- /dev/null +++ b/.lintstagedrc.ts @@ -0,0 +1,20 @@ +import type { Configuration } from 'lint-staged' + +// oxlint and oxfmt both ignore `scripts/templates/**` (see their rc files — the +// placeholder files aren't valid TS on their own), and both treat a fully +// ignored file list as an error rather than a no-op. So a commit touching only +// templates would fail the hook on "no files to check": drop them here instead. +const IGNORED = '/scripts/templates/' + +const quote = (paths: string[]): string => paths.map(path => JSON.stringify(path)).join(' ') + +const config: Configuration = { + '*.{ts,tsx}': files => { + const checkable = files.filter(file => !file.includes(IGNORED)) + if (checkable.length === 0) return [] + const targets = quote(checkable) + return [`oxlint --fix ${targets}`, `oxfmt ${targets}`] + }, +} + +export default config diff --git a/.oxlintrc.json b/.oxlintrc.json index 3a5e3c3..398c73a 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -33,7 +33,8 @@ { "files": ["**/*.test.ts", "**/*.test.tsx"], "rules": { - "vitest/require-mock-type-parameters": "off" + "vitest/require-mock-type-parameters": "off", + "vitest/expect-expect": ["warn", { "assertFunctionNames": ["expect", "expectOverflow"] }] } }, { diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index e232527..1503ef3 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -6,7 +6,7 @@ one per host environment — the **substrates**. Behavior cannot drift between hosts because it exists in exactly one place. A substrate is any environment a primitive is delivered to: a framework -(react), another framework (vue, solid), or a different host entirely +(react), another framework (solid), or a different host entirely (native). Substrates are cheap by design; the expensive thing — the behavior — is written once. @@ -65,8 +65,8 @@ the dialog's Escape listener, the ordered sequence around its open and exit edges — lives under `dom/components/` instead. A util is primitive-agnostic and imports nothing from the repo; a component package is the opposite, and may import the primitive's core package and any DOM util. Both are equally -framework-free. The split matters as substrates multiply: React, Solid, and -Vue differ in how they schedule an effect, not in what the effect does, so the +framework-free. The split matters as substrates multiply: React and Solid +differ in how they schedule an effect, not in what the effect does, so the what is written once and each binding contributes only its lifecycle. Machine logic that several primitives need — the controlled/uncontrolled diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4a81acf..01912a0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,7 +36,7 @@ pnpm test packages/core/dialog/tests/machine.test.ts ## Storybook -Each UI substrate (React, Vue, ...) is a self-contained package under +Each UI substrate (React, Solid, ...) is a self-contained package under `packages/` with its own Storybook — the fastest way to see a change actually render. Every substrate gets an explicit `dev:` script: diff --git a/README.md b/README.md index 64d7ccf..428d2e8 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ machine (its **core**) and delivered through a thin binding per host environment v v v +-----------+ +-----------+ +-----------+ | substrate | | substrate | | substrate | packages// - | (react) | | (vue) | | (native) | render + host wiring + | (react) | | (solid) | | (native) | render + host wiring +-----------+ +-----------+ +-----------+ same behavior, same a11y — only the render differs ``` diff --git a/packages/core/dialog/SPEC.md b/packages/core/dialog/SPEC.md index 18fe891..2ca4129 100644 --- a/packages/core/dialog/SPEC.md +++ b/packages/core/dialog/SPEC.md @@ -79,11 +79,16 @@ default): while the dialog is open, Back closes it instead of leaving the page — the pattern mobile users expect from a full-screen overlay. It follows the shared dismissal contract: `onBackNavigation` fires first and `preventDefault()` vetoes, a controlled dialog only records the intent, and a -nested stack unwinds one layer per press. The substrate wires the host -mechanics (the web plants a guard entry in the session history; a native host -wires its hardware back handler); a dialog closed any other way leaves no -trace behind — its guard entry is consumed, not left to swallow the next -Back press. +nested stack unwinds one layer per press. Back's mirror is Forward: on a host +whose forward navigation can re-enter what Back left (the web's forward +stack), traversing forward into the spent entry reopens the dialog — the +same `closeOnBack` setting gates it, `onForwardNavigation` fires first and +`preventDefault()` vetoes, and a controlled dialog only records the intent. +The substrate wires the host mechanics (the web plants a guard entry in the +session history; a native host wires its hardware back handler and has no +forward); a dialog closed any other way leaves no trace behind — its guard +entry is consumed, not left to swallow the next Back press, and there is +nothing for Forward to reopen. Dialogs can be nested — a dialog opened from within another stacks on top of it, and the stack unwinds one layer at a time. The full contract is @@ -173,7 +178,7 @@ stack of dialogs only the topmost one exists until it closes. topmost again — re-exposed, interactive, with focus restored to the element focused before the closed dialog opened (normally its trigger). - **Closing the stack**: a close intent can be scoped to the whole stack — an - Escape whose scope is the stack, or a stack-scoped Close press. Only the + Escape whose scope is the stack (`escapeScope`). Only the dialog that received the intent gates or vetoes it; once allowed, the stack unwinds top-down, every layer beneath receiving a plain close — no Escape or outside-press gating — and reporting it through its own callback, a @@ -206,14 +211,15 @@ choice, not the behavior it produces (that's spec'd above). The dialog ships headless: parts carry behavior and ARIA wiring plus a `data-state` attribute (`open` / `closed`) for styling and animation; visuals belong to the consumer. -| Position | Why | -| ------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------- | -| `open` delegates to `@dunky.dev/controllable`; `onOpenChange` reacts to the state, not to intents | One shared mechanic across primitives, and the callback structurally can't drift from the controlled contract. | -| Dismissal intents are distinct events (`escape`, `interact.outside`, `history.back`) | Their gating lives in core guards — no substrate re-implements the settings. | -| Back navigation reports through one `backNavigate` on the api | The callback, veto, and controlled fork live once in the connect; only the host's back mechanics differ per substrate. | -| One base id, per-part ids derived from it | The cross-part ARIA references (controls / labelledby / describedby) can never disagree. | -| Part presence lives in machine context (`part.presence` events) | The rendered-parts rule holds in every substrate with no substrate bookkeeping. | -| This contract owns modality, dismissal, and focus | A substrate must not hand authority to host built-ins (e.g. `showModal()`) — behavior can't fork per host. | -| The exit window is a machine state; `exit.complete` comes from the substrate | Reopen-during-exit is a named transition, not a substrate-side unmount race; only the host knows when paint finished. | -| A `closing` dialog has already left the stack — focus, Escape, containment move on immediately | The exit is purely cosmetic; the layer beneath must not wait on an animation to become interactive again. | -| The `intent` slot records every declared intent, drives no callback | Reserved as the request channel a stack-scoped close needs to traverse controlled layers. | +| Position | Why | +| ------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | +| `open` delegates to `@dunky.dev/controllable`; `onOpenChange` reacts to the state, not to intents | One shared mechanic across primitives, and the callback structurally can't drift from the controlled contract. | +| Dismissal intents are distinct events (`escape`, `interact.outside`, `history.back`) | Their gating lives in core guards — no substrate re-implements the settings. | +| `history.forward` is `history.back`'s mirror, gated by the same `closeOnBack` | Back-close and Forward-reopen are one feature — the openness tracking the history position — not two settings to drift apart. | +| History navigation reports through `backNavigate` / `forwardNavigate` on the api | The callback, veto, and controlled fork live once in the connect; only the host's traversal mechanics differ per substrate. | +| One base id, per-part ids derived from it | The cross-part ARIA references (controls / labelledby / describedby) can never disagree. | +| Part presence lives in machine context (`part.presence` events) | The rendered-parts rule holds in every substrate with no substrate bookkeeping. | +| This contract owns modality, dismissal, and focus | A substrate must not hand authority to host built-ins (e.g. `showModal()`) — behavior can't fork per host. | +| The exit window is a machine state; `exit.complete` comes from the substrate | Reopen-during-exit is a named transition, not a substrate-side unmount race; only the host knows when paint finished. | +| A `closing` dialog has already left the stack — focus, Escape, containment move on immediately | The exit is purely cosmetic; the layer beneath must not wait on an animation to become interactive again. | +| The `intent` slot records every declared intent, drives no callback | Reserved as the request channel a stack-scoped close needs to traverse controlled layers. | diff --git a/packages/core/dialog/src/connect.ts b/packages/core/dialog/src/connect.ts index 1adbfc2..f9596f4 100644 --- a/packages/core/dialog/src/connect.ts +++ b/packages/core/dialog/src/connect.ts @@ -43,6 +43,11 @@ export interface DialogApi { * only wires its host mechanics (a session-history guard entry on the web, a * hardware back handler on native) to this call. */ backNavigate: () => void + /** Reports the host's Forward navigation re-entering the ground a + * Back-close left behind. `backNavigate`'s mirror, decided the same way: + * `onForwardNavigation` fires first (`preventDefault()` vetoes), the + * machine gates on `closeOnBack`, and the controlled contract applies. */ + forwardNavigate: () => void parts: { trigger: DialogPartBindings backdrop: DialogPartBindings @@ -74,6 +79,22 @@ export const dialogConnect: Connect< if (event?.defaultPrevented !== true) send({ type: 'interact.outside' }) } + // The host's traversal has no cancelable event — synthesize the veto + // payload so the callback contract matches the other dismissals. + const historyNavigate = ( + callback: ((event?: BackNavigationPayload) => void) | undefined, + event: DialogMachineEvent, + ): void => { + const payload: BackNavigationPayload = { + defaultPrevented: false, + preventDefault() { + payload.defaultPrevented = true + }, + } + callback?.(payload) + if (payload.defaultPrevented !== true) send(event) + } + return { open, mounted: state !== 'closed', @@ -84,16 +105,10 @@ export const dialogConnect: Connect< send({ type: next ? 'open' : 'close' }) }, backNavigate() { - // The host's back has no cancelable event — synthesize the veto payload - // so the callback contract matches the other dismissals. - const payload: BackNavigationPayload = { - defaultPrevented: false, - preventDefault() { - payload.defaultPrevented = true - }, - } - props.onBackNavigation?.(payload) - if (payload.defaultPrevented !== true) send({ type: 'history.back' }) + historyNavigate(props.onBackNavigation, { type: 'history.back' }) + }, + forwardNavigate() { + historyNavigate(props.onForwardNavigation, { type: 'history.forward' }) }, parts: { trigger: { diff --git a/packages/core/dialog/src/index.ts b/packages/core/dialog/src/index.ts index b2bc205..1a6fb43 100644 --- a/packages/core/dialog/src/index.ts +++ b/packages/core/dialog/src/index.ts @@ -5,6 +5,7 @@ export type { BackNavigationPayload, DialogCallbacks, DialogContext, + DialogEscapeScope, DialogIds, DialogMachineEvent, DialogOptions, diff --git a/packages/core/dialog/src/machine.ts b/packages/core/dialog/src/machine.ts index ad27a49..ecbee5a 100644 --- a/packages/core/dialog/src/machine.ts +++ b/packages/core/dialog/src/machine.ts @@ -42,6 +42,7 @@ export function dialogMachine( role, modal: options.modal ?? true, closeOnEscape: options.closeOnEscape ?? true, + escapeScope: options.escapeScope ?? 'layer', // An alert dialog interrupts for a response — an outside press must not // dismiss it unless explicitly opted in. closeOnInteractOutside: options.closeOnInteractOutside ?? role === 'dialog', @@ -64,6 +65,13 @@ export function dialogMachine( on: { open: intend('open', { target: 'open', value: true }), toggle: intend('open', { target: 'open', value: true }), + // Forward re-enters the ground a Back-close left behind — the + // mirror of `history.back`, gated by the same setting. + 'history.forward': intend('open', { + guard: canCloseOnBack, + target: 'open', + value: true, + }), 'controlled.sync': synced('open', { value: true, target: 'open' }), }, }, @@ -89,6 +97,11 @@ export function dialogMachine( on: { open: intend('open', { target: 'open', value: true }), toggle: intend('open', { target: 'open', value: true }), + 'history.forward': intend('open', { + guard: canCloseOnBack, + target: 'open', + value: true, + }), 'exit.complete': { target: 'closed' }, 'controlled.sync': synced('open', { value: true, target: 'open' }), }, diff --git a/packages/core/dialog/src/types.ts b/packages/core/dialog/src/types.ts index 34b72df..2af9ddf 100644 --- a/packages/core/dialog/src/types.ts +++ b/packages/core/dialog/src/types.ts @@ -28,10 +28,13 @@ export interface DialogIds { close: string } +export type DialogEscapeScope = 'layer' | 'stack' + export interface DialogContext { role: DialogRole modal: boolean closeOnEscape: boolean + escapeScope: DialogEscapeScope closeOnInteractOutside: boolean closeOnBack: boolean // The consumer-ownable open value. A controlled machine never moves on its @@ -59,13 +62,15 @@ export type DialogMachineEvent = | { type: 'escape' } | { type: 'interact.outside' } | { type: 'history.back' } + | { type: 'history.forward' } | { type: 'exit.complete' } | ControlledSync | { type: 'part.presence'; part: DialogPart; present: boolean } -/** The payload for a back-navigation dismissal. Synthesized by the connect — - * the host's back has no cancelable event of its own — carrying only the veto - * contract every dismissal callback shares. */ +/** The payload for a history-navigation change — a Back dismissal or a + * Forward reopen. Synthesized by the connect — the host's traversal has no + * cancelable event of its own — carrying only the veto contract every + * dismissal callback shares. */ export interface BackNavigationPayload { defaultPrevented?: boolean preventDefault?: () => void @@ -80,6 +85,8 @@ export interface DialogCallbacks { onInteractOutside?: (event?: PointerPayload) => void /** Fired before a back-navigation dismissal; `preventDefault()` vetoes it. */ onBackNavigation?: (event?: BackNavigationPayload) => void + /** Fired before a forward-navigation reopen; `preventDefault()` vetoes it. */ + onForwardNavigation?: (event?: BackNavigationPayload) => void } /** @@ -103,13 +110,19 @@ export interface DialogOptions extends DialogCallbacks { role?: DialogRole /** Whether Escape closes the dialog. @default true */ closeOnEscape?: boolean + /** How far an allowed Escape reaches in a nested stack: this layer only, so + * the stack unwinds one press at a time, or the whole stack at once. Only + * this dialog gates and vetoes it; the layers beneath receive a plain close. + * @default 'layer' */ + escapeScope?: DialogEscapeScope /** Whether pressing the backdrop closes the dialog. * @default true — false when `role="alertdialog"` */ closeOnInteractOutside?: boolean /** Treats the host's Back navigation as a dismissal: while the dialog is * open, Back closes it instead of leaving the page — one layer per press in - * a nested stack. The substrate wires the host mechanics (the web plants a - * guard entry in the session history). @default false */ + * a nested stack — and, on a host with a forward stack, Forward reopens + * what Back closed. The substrate wires the host mechanics (the web plants + * a guard entry in the session history). @default false */ closeOnBack?: boolean /** Reserves an exit window for a close animation: closing passes through the * `closing` state (`data-state="closing"` styles the exit) and the dialog diff --git a/packages/core/dialog/tests/machine.test.ts b/packages/core/dialog/tests/machine.test.ts index 3be44f6..edef154 100644 --- a/packages/core/dialog/tests/machine.test.ts +++ b/packages/core/dialog/tests/machine.test.ts @@ -288,6 +288,49 @@ describe('dialog machine — back navigation', () => { }) }) +describe('dialog machine — forward navigation', () => { + it('ignores history.forward without closeOnBack (the default)', () => { + const { service } = build() + service.send({ type: 'history.forward' }) + expect(service.state).toBe('closed') + expect(service.context.open.intent).toBeNull() + }) + + it('reopens on history.forward when closeOnBack, interrupting the exit window too', () => { + const { service } = build({ closeOnBack: true }) + service.send({ type: 'history.forward' }) + expect(service.state).toBe('open') + + const animated = build({ defaultOpen: true, closeOnBack: true, animated: true }) + animated.service.send({ type: 'history.back' }) + expect(animated.service.state).toBe('closing') + animated.service.send({ type: 'history.forward' }) + expect(animated.service.state).toBe('open') + }) + + it('forwardNavigate fires the callback and reopens unless vetoed', () => { + const onForwardNavigation = vi.fn() + const { service, connection } = build({ closeOnBack: true, onForwardNavigation }) + connection.snapshot.forwardNavigate() + expect(onForwardNavigation).toHaveBeenCalledTimes(1) + expect(service.state).toBe('open') + + const vetoed = build({ + closeOnBack: true, + onForwardNavigation: event => event?.preventDefault?.(), + }) + vetoed.connection.snapshot.forwardNavigate() + expect(vetoed.service.state).toBe('closed') + }) + + it('a controlled dialog records the reopen intent and stays put', () => { + const { service, connection } = build({ open: false, closeOnBack: true }) + connection.snapshot.forwardNavigate() + expect(service.state).toBe('closed') + expect(service.context.open.intent).toEqual({ value: true }) + }) +}) + describe('dialog machine — animated exit', () => { it('a close intent holds the exit window open until exit.complete', () => { const { service } = build({ defaultOpen: true, animated: true }) diff --git a/packages/core/utils/overlay/SPEC.md b/packages/core/utils/overlay/SPEC.md index 49a17bb..57f981a 100644 --- a/packages/core/utils/overlay/SPEC.md +++ b/packages/core/utils/overlay/SPEC.md @@ -11,9 +11,10 @@ The agnostic half of overlay coordination. The overlay family — dialog, drawer, popover, menu, combobox — shares one problem: when overlays stack, -which layer is topmost? The topmost owns Escape, the focus trap, and (when -modal) assistive-tech containment. This package is the registry and the -topmost decision, with no host assumptions — it knows nothing about how a +which layer is topmost? The topmost owns Escape and the focus trap; +assistive-tech containment follows the topmost modal layer, which is not +always the same one (see the DOM spec). This package is the registry and the +ranking decision, with no host assumptions — it knows nothing about how a layer is drawn or how containment is applied. A host realization extends each layer with a payload — the element or view — and applies its own containment as the stack shifts; @@ -31,14 +32,21 @@ one. The shared instance is what makes one Escape close exactly one layer, even across different primitives. - Registering returns a disposer; an empty stack has no topmost. +- The whole stack is readable in rank order — topmost first — for a host + that treats layers differently by kind and must look past the top (the DOM + half's containment does). The kind itself stays the host's concern: the + stack ranks, the host decides which rank it cares about. +- A dismissal that reaches past its own layer gets the ones beneath in + unwinding order — topmost first — so closing a whole stack at once ends up + where closing it one layer at a time would have. ## API -| Export | Description | -| ----------------------- | ------------------------------------------------------------ | -| `createLayerStack()` | A fresh stack — one per host binding, not per primitive. | -| `OverlayLayer` | What every layer carries: `id` and `depth` (1 = top-level). | -| `LayerStack` | `register(layer)` -> disposer, `topmost()`, `isTopmost(id)`. | +| Export | Description | +| ----------------------- | -------------------------------------------------------------------------------------- | +| `createLayerStack()` | A fresh stack — one per host binding, not per primitive. | +| `OverlayLayer` | What every layer carries: `id` and `depth` (1 = top-level). | +| `LayerStack` | `register(layer)` -> disposer, `topmost()`, `ordered()`, `isTopmost(id)`, `below(id)`. | ## Constraints @@ -48,7 +56,8 @@ one. ## Internals -| Position | Why | -| --------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- | -| `depth` is carried by the layer, not derived by the stack | Only the primitive knows its nesting; the stack has no host to ask, and document order lies under portals. | -| Topmost is a linear scan, not a maintained order | Stacks hold a handful of layers; scanning beats keeping an order coherent across out-of-order removals. | +| Position | Why | +| --------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `depth` is carried by the layer, not derived by the stack | Only the primitive knows its nesting; the stack has no host to ask, and document order lies under portals. | +| Topmost is a linear scan, not a maintained order | Stacks hold a handful of layers; scanning beats keeping an order coherent across out-of-order removals. | +| The stack orders layers; only the DOM half reads `modal` | Finding the topmost modal layer needs the whole stack in rank order, but modality is a host concept — the agnostic stack ranks, the host decides which rank it cares about. | diff --git a/packages/core/utils/overlay/src/layer-stack.ts b/packages/core/utils/overlay/src/layer-stack.ts index 3033fe9..95dbe4d 100644 --- a/packages/core/utils/overlay/src/layer-stack.ts +++ b/packages/core/utils/overlay/src/layer-stack.ts @@ -1,10 +1,11 @@ // The overlay family — dialog, drawer, popover, menu, combobox — shares one // coordination problem: when overlays stack, which layer is topmost? The -// topmost owns Escape, the focus trap, and (when modal) assistive-tech -// containment. This is the agnostic half of the answer: the registry and the -// topmost decision, with no host assumptions. A host realization (DOM, native) -// gives each layer a payload — the element or view — and applies its own -// containment when the stack shifts. +// topmost owns Escape and the focus trap; assistive-tech containment follows +// the topmost modal layer, which is not always the same one. This is the +// agnostic half of the answer: the registry and the ranking decision, with no +// host assumptions. A host realization (DOM, native) gives each layer a +// payload — the element or view — and applies its own containment when the +// stack shifts. export interface OverlayLayer { id: string @@ -21,7 +22,16 @@ export interface LayerStack { register: (layer: T) => () => void // The topmost layer, or undefined when the stack is empty. topmost: () => T | undefined + // Every layer, topmost first. A host that treats layers differently by kind + // — containment follows the topmost *modal* layer, not the topmost layer — + // needs to look past the top of the stack; the kind itself stays the host's + // concern, so this orders and the host decides. + ordered: () => T[] isTopmost: (id: string) => boolean + // The layers stacked beneath `id`, topmost first — the unwinding order for a + // dismissal scoped to the whole stack rather than one layer. An id that + // isn't registered has nothing beneath it. + below: (id: string) => T[] } // One stack per running host: a browser page or a native app is one or the @@ -32,20 +42,27 @@ export function createLayerStack(): LayerStack { const layers: Array = [] let nextOrder = 0 + // Topmost first: deeper nesting wins, open order breaks ties at equal depth. + const isAbove = (layer: T & { order: number }, other: T & { order: number }): boolean => + layer.depth > other.depth || (layer.depth === other.depth && layer.order > other.order) + const topmost = (): T | undefined => { let top: (T & { order: number }) | undefined for (const layer of layers) { - if ( - top === undefined || - layer.depth > top.depth || - (layer.depth === top.depth && layer.order > top.order) - ) { - top = layer - } + if (top === undefined || isAbove(layer, top)) top = layer } return top } + const ordered = (): T[] => { + // Copy before sorting: the registry's own order is identity, not rank. + // The comparator never returns 0, which is fine — `order` is unique per + // layer, so `isAbove` is a strict total order over distinct layers. + const sorted = layers.slice() + sorted.sort((a, b) => (isAbove(a, b) ? -1 : 1)) + return sorted + } + return { register(layer) { const entry = { ...layer, order: nextOrder++ } @@ -56,8 +73,21 @@ export function createLayerStack(): LayerStack { } }, topmost, + ordered, isTopmost(id) { return topmost()?.id === id }, + below(id) { + const self = layers.find(layer => layer.id === id) + if (self === undefined) return [] + // Same ordering as `topmost`, applied to the whole stack: deeper first, + // open order breaking ties. + return layers + .filter( + layer => + layer.depth < self.depth || (layer.depth === self.depth && layer.order < self.order), + ) + .sort((left, right) => right.depth - left.depth || right.order - left.order) + }, } } diff --git a/packages/core/utils/overlay/tests/layer-stack.test.ts b/packages/core/utils/overlay/tests/layer-stack.test.ts index 1ee7e5e..3d711fe 100644 --- a/packages/core/utils/overlay/tests/layer-stack.test.ts +++ b/packages/core/utils/overlay/tests/layer-stack.test.ts @@ -31,6 +31,28 @@ describe('createLayerStack', () => { const stack = createLayerStack() expect(stack.topmost()).toBeUndefined() expect(stack.isTopmost('anything')).toBe(false) + expect(stack.ordered()).toEqual([]) + }) + + it('orders every layer topmost first', () => { + const stack = createLayerStack() + stack.register({ id: 'shallow', depth: 1 }) + stack.register({ id: 'deep-first', depth: 2 }) + stack.register({ id: 'deep-second', depth: 2 }) + + expect(stack.ordered().map(layer => layer.id)).toEqual(['deep-second', 'deep-first', 'shallow']) + }) + + it('lists the layers beneath a layer in unwinding order, topmost first', () => { + const stack = createLayerStack() + stack.register({ id: 'bottom', depth: 1 }) + stack.register({ id: 'sibling', depth: 2 }) + stack.register({ id: 'middle', depth: 2 }) + stack.register({ id: 'top', depth: 3 }) + + expect(stack.below('top').map(layer => layer.id)).toEqual(['middle', 'sibling', 'bottom']) + expect(stack.below('bottom')).toEqual([]) + expect(stack.below('never-registered')).toEqual([]) }) it('stacks are independent — registering in one never affects another', () => { diff --git a/packages/dom/components/dialog/SPEC.md b/packages/dom/components/dialog/SPEC.md index 5f5fa9e..a2321b1 100644 --- a/packages/dom/components/dialog/SPEC.md +++ b/packages/dom/components/dialog/SPEC.md @@ -28,7 +28,7 @@ primitive, so it may import that primitive's core package and any DOM util. What it must not do is import a framework, or another primitive. Substrate bindings are the only consumers. Each one supplies its host's -lifecycle — an effect, a `createEffect`, a `watchEffect` — and calls into +lifecycle — React's `useEffect`, Solid's `createEffect` — and calls into these; none of them re-derives the order or the conditions. ## Behavior @@ -40,16 +40,27 @@ listener, as the same plain-data tuples the core defines. A substrate passes the list to its adapter's `useMachine` untouched. Escape is bound on the document in the capture phase, not on a part: it must -answer wherever focus is. It closes only the topmost layer, so a nested stack -unwinds one dialog per press, and it offers the consumer's `onEscapeKeyDown` a -veto through `preventDefault` before it moves the machine. +answer wherever focus is. Only the topmost layer answers, and it offers the +consumer's `onEscapeKeyDown` a veto through `preventDefault` before it moves +the machine. + +How far an allowed Escape reaches is that dialog's `escapeScope`: itself, so a +nested stack unwinds one dialog per press, or the whole stack at once. Either +way the dialog that received the press is the only one that gates or vetoes +it — a veto leaves the stack standing — and a stack-scoped press closes the +layers beneath from the top down, each one plainly, as if its consumer had +closed it. + +Overlays that offer no way to be closed from above are left open, so a stack +that mixes primitives is never dismissed out from under them. ### The open edge `openDialogLayer` runs one ordered sequence and returns its exact inverse: 1. remember what had focus, -2. join the shared layer stack (which re-syncs assistive-tech containment), +2. join the shared layer stack (which re-syncs assistive-tech containment, and + makes the layer reachable by a stack-scoped dismissal from above), 3. move focus to the consumer's `initialFocus`, or the overlay's own choice, 4. fall back to the dialog window when that target refuses focus. @@ -80,10 +91,23 @@ interrupt as much as the final unmount. ### Back navigation -`guardBackNavigation` plants the session-history entry that turns the host's -Back into a dismissal. It wires mechanics only: whether the dialog may close, -whether the consumer vetoed, and whether a controlled dialog followed are all -the core's answers, read back as "is it still open". +`guardBackNavigation` makes the host's Back a dismissal, and its Forward the +reopen of what Back closed. It wires mechanics only: whether the dialog may +close or reopen, whether the consumer vetoed, and whether a controlled dialog +followed are all the core's answers, read back as "is it open". + +The guard outlives the open state, because the Forward watch has to: a dialog +the host's Back closed can still be reopened by the host's Forward, while a +dialog closed any other way is gone and leaves no way back. So the substrate +reports the dialog's open state as it changes and says when the dialog is gone +for good; which of those two closes just happened is answered here, once, for +every substrate. + +A nested dialog doesn't outlive the parent it was opened from — closing the +parent unmounts it, machine and all — so the ground it lost to a Back press +would be lost with it. It isn't: the ground belongs to the dialog's place in +the stack, so the dialog that comes back with the parent reopens from it. Two +dialogs at the same place can't be told apart, and then neither reopens. ### Outside presses @@ -103,15 +127,15 @@ 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)` | Arms the history guard; returns the release. | -| `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)` | 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. | ## Constraints @@ -119,17 +143,21 @@ part is the cycle's last stop wherever it renders. - No decisions of its own. Anything a substrate could answer differently belongs in the core machine; what lives here is only the DOM realization of a decision already made. -- Every entry point returns its own disposer, and the disposer undoes exactly - what the call did — substrate lifecycles differ, so nothing may rely on a - particular teardown order between calls. +- Every entry point returns its own teardown — a disposer, or a `release` on a + call that outlives one lifecycle scope — and it undoes exactly what the call + did: substrate lifecycles differ, so nothing may rely on a particular + teardown order between calls. - Reads that must stay live (`modal`, the topmost check, the Close id) are taken as the machine or as accessors, never snapshotted at call time. ## Internals -| Position | Why | -| -------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| The open edge is one call, not a `registerLayer` + focus pair | The two orders (join before focus in, release before focus out) are the contract; splitting them puts that ordering back in every substrate, where it drifted before. | -| `dialogTrapOptions` takes the machine rather than plain values | `modal` and the layer id are read per Tab press. Snapshotting them freezes the trap against a context the machine still owns. | -| `closeId` is an accessor while the machine is not | The machine instance is stable; the connected api that carries the ids is re-created per render. | -| Press gating takes a structural `{ target, currentTarget }` | React's synthetic event and Solid's native one share only that shape; requiring either would drag a framework type into this layer. | +| Position | Why | +| ----------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| The open edge is one call, not a `registerLayer` + focus pair | The two orders (join before focus in, release before focus out) are the contract; splitting them puts that ordering back in every substrate, where it drifted before. | +| `dialogTrapOptions` takes the machine rather than plain values | `modal` and the layer id are read per Tab press. Snapshotting them freezes the trap against a context the machine still owns. | +| `closeId` is an accessor while the machine is not | The machine instance is stable; the connected api that carries the ids is re-created per render. | +| Press gating takes a structural `{ target, currentTarget }` | React's synthetic event and Solid's native one share only that shape; requiring either would drag a framework type into this layer. | +| The back guard reports state instead of returning a disposer | Its life spans a Back-close, so no host's "while open" scope fits it. Reporting the open state keeps the arm/park/release decision here rather than in each host. | +| A stack-scoped Escape reads the stack before it moves the machine | Closing the layer releases it from the stack, and the answer to "what was beneath me" goes with it. Dismissing only after the machine actually left `open` is what makes a veto leave the stack standing. | +| A returning dialog is recognized by its nesting depth, not its id | The auto-generated id does not survive the remount (React's `useId` mints a fresh one), and requiring an explicit id would make the reopen an opt-in. Depth is what genuinely survives — at the cost of the same-depth ambiguity, resolved by reopening nobody. | diff --git a/packages/dom/components/dialog/src/back-navigation.ts b/packages/dom/components/dialog/src/back-navigation.ts index 1948eb7..6554e70 100644 --- a/packages/dom/components/dialog/src/back-navigation.ts +++ b/packages/dom/components/dialog/src/back-navigation.ts @@ -1,21 +1,102 @@ -import { interceptBackNavigation } from '@dunky.dev/browser-navigation' +import { + interceptBackNavigation, + watchSpentEntry, + type ReleaseOptions, +} from '@dunky.dev/browser-navigation' export interface BackNavigationGuardOptions { /** The api's `backNavigate` — every decision (gate, veto, controlled) is the core's. */ backNavigate: () => void - /** Whether the machine is still open after `backNavigate` ran. */ + /** The api's `forwardNavigate` — the reopen half, gated by the same `closeOnBack`. */ + forwardNavigate: () => void + /** Whether the machine is open, read back after a navigation ran. */ isOpen: () => boolean + /** + * The dialog's nesting depth (1 = top-level). It is what a returning dialog + * recognizes its own spent entry by: the machine is new after a remount, but + * the position in the stack is the same. + */ + depth: number +} + +export interface BackNavigationGuard { + /** + * The dialog's open state, reported on every change: an open edge (re)arms + * the guard, a close either parks it — the Back press itself closed the + * dialog, so Forward may still reopen it — or releases it and watches for + * the dialog's own ground to be re-entered. + */ + sync: (open: boolean) => void + /** The dialog is gone for good; ends the episode in whichever phase it is. */ + release: () => void } /** * closeOnBack: while open, a guard entry in the session history turns the - * host's Back into a dismissal instead of a navigation. This only wires the - * web mechanics — whether the dialog actually closed is the machine's answer, - * and a decline re-arms the guard. + * host's Back into a dismissal instead of a navigation — and the entry a Back + * press pops survives in the forward stack, so the host's Forward reopens what + * Back closed. This only wires the web mechanics: whether the dialog actually + * closed (or reopened) is the machine's answer, read back through `isOpen`, and + * a decline leaves the guard armed. + * + * One registration spans the whole episode rather than the open state alone — + * releasing on a Back-close would end the Forward watch along with it. A closed + * dialog keeps the weaker watch instead: not on an entry it owns, but on its + * own ground being re-entered, which is the only way back for a nested dialog + * that was unmounted with the parent it was opened from. */ -export function guardBackNavigation(options: BackNavigationGuardOptions): () => void { - return interceptBackNavigation(() => { - options.backNavigate() - return !options.isOpen() - }) +export function guardBackNavigation(options: BackNavigationGuardOptions): BackNavigationGuard { + const claim = `dialog:${options.depth}` + let releaseIntercept: ((options?: ReleaseOptions) => void) | null = null + let releaseWatch: (() => void) | null = null + let closedByBack = false + + // Torn down (unmounted) rather than closed: the dialog's ground stays its + // own, so the instance that takes its place can reopen from it. A close is + // the opposite — see `sync`. + const release = (): void => { + releaseIntercept?.({ keepClaim: true }) + releaseIntercept = null + releaseWatch?.() + releaseWatch = null + } + + const reopen = (): boolean => { + options.forwardNavigate() + return options.isOpen() + } + + return { + sync(open) { + if (open) { + // (Re)arm on every open edge. Reopened by Forward, release + + // re-register adopts the re-entered entry in place; opened any other + // way it plants a fresh entry, truncating a stale Forward leftover + // exactly like the browser does for any navigation after a Back. + release() + releaseIntercept = interceptBackNavigation( + () => { + options.backNavigate() + closedByBack = !options.isOpen() + return closedByBack + }, + { onForward: reopen, claim }, + ) + } else if (closedByBack) { + // The registration stays parked in the util, watching the spent entry + // it still owns — a stronger claim than the one below. + closedByBack = false + } else { + // Closed by something other than Back: this dialog is done with its + // entry, and Forward must not bring it back. It still watches for its + // own ground to be re-entered — ground a previous instance of this + // dialog lost when it was torn down mid-episode. + releaseIntercept?.() + releaseIntercept = null + releaseWatch?.() + releaseWatch = watchSpentEntry(claim, reopen) + } + }, + release, + } } diff --git a/packages/dom/components/dialog/src/effects.ts b/packages/dom/components/dialog/src/effects.ts index 60c149d..285b6d1 100644 --- a/packages/dom/components/dialog/src/effects.ts +++ b/packages/dom/components/dialog/src/effects.ts @@ -1,5 +1,5 @@ import { dialogEffects, type DialogEffect } from '@dunky.dev/dialog' -import { isTopmostLayer } from '@dunky.dev/dom-overlay' +import { isTopmostLayer, layersBelow } from '@dunky.dev/dom-overlay' // Escape is a document-level concern, not a part's — it must work wherever // focus is. @@ -8,10 +8,19 @@ const trackEscape: DialogEffect = [ const onKeyDown = (event: KeyboardEvent): void => { if (event.key !== 'Escape' || !machine.matches('open')) return // Only the topmost dialog answers Escape — a nested stack closes one - // layer at a time. + // layer at a time, unless this dialog's scope is the whole stack. if (!isTopmostLayer(machine.context.id)) return props.onEscapeKeyDown?.(event) - if (!event.defaultPrevented) machine.send({ type: 'escape' }) + if (event.defaultPrevented) return + // Read the stack before the send: closing this layer releases it, and + // the answer to "what was beneath me" goes with it. + const beneath = machine.context.escapeScope === 'stack' ? layersBelow(machine.context.id) : [] + machine.send({ type: 'escape' }) + // Only an Escape this dialog actually allowed unwinds the rest — and the + // layers beneath receive a plain close, their own dismissal settings not + // consulted again, because the intent was gated and vetoed here. + if (machine.matches('open')) return + for (const layer of beneath) layer.dismiss?.() } document.addEventListener('keydown', onKeyDown, true) return () => document.removeEventListener('keydown', onKeyDown, true) diff --git a/packages/dom/components/dialog/src/index.ts b/packages/dom/components/dialog/src/index.ts index 795b741..cecf6bd 100644 --- a/packages/dom/components/dialog/src/index.ts +++ b/packages/dom/components/dialog/src/index.ts @@ -1,6 +1,10 @@ export { domDialogEffects } from './effects' export { openDialogLayer, type OpenDialogLayerOptions } from './open-layer' export { startExitWindow, type ExitWindowOptions } from './exit-window' -export { guardBackNavigation, type BackNavigationGuardOptions } from './back-navigation' +export { + guardBackNavigation, + type BackNavigationGuard, + type BackNavigationGuardOptions, +} from './back-navigation' export { acceptsBackdropPress, acceptsViewportPress } from './press' 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 57a12f2..be277b4 100644 --- a/packages/dom/components/dialog/src/open-layer.ts +++ b/packages/dom/components/dialog/src/open-layer.ts @@ -9,6 +9,9 @@ export interface OpenDialogLayerOptions { backdrop: () => Element | null /** The consumer's `initialFocus`, already resolved. @default the window */ initialFocus?: HTMLElement | null + /** Closes this dialog when a layer above unwinds the whole stack; see + * `Layer.dismiss` in dom-overlay. */ + dismiss?: () => void } /** @@ -28,6 +31,7 @@ export function openDialogLayer(content: HTMLElement, options: OpenDialogLayerOp element: content, modal: options.modal, backdrop: options.backdrop, + dismiss: options.dismiss, }) // preventScroll everywhere: the scroll lock already froze the surface, so diff --git a/packages/dom/components/dialog/tests/dialog.test.ts b/packages/dom/components/dialog/tests/dialog.test.ts index 2fdf3a6..94377e0 100644 --- a/packages/dom/components/dialog/tests/dialog.test.ts +++ b/packages/dom/components/dialog/tests/dialog.test.ts @@ -15,6 +15,7 @@ import { acceptsViewportPress, dialogTrapOptions, domDialogEffects, + guardBackNavigation, openDialogLayer, startExitWindow, } from '@dunky.dev/dom-dialog' @@ -44,12 +45,14 @@ const pressEscape = (): boolean => const registered: (() => void)[] = [] // A layer, mounted and registered, standing in for a rendered dialog window. -const mountLayer = (id: string, depth: number, html = ''): HTMLElement => { +const mountLayer = (id: string, depth: number, html = '', dismiss?: () => void): HTMLElement => { const content = document.createElement('div') content.tabIndex = -1 content.innerHTML = html document.body.append(content) - registered.push(registerLayer({ id, depth, element: content, modal: true, backdrop: () => null })) + registered.push( + registerLayer({ id, depth, element: content, modal: true, backdrop: () => null, dismiss }), + ) return content } @@ -89,6 +92,30 @@ describe('domDialogEffects — Escape', () => { expect(service.matches('open')).toBe(true) }) + // escapeScope: 'stack' — the receiving dialog gates and vetoes, then the + // layers beneath get a plain close, top-down. + it('unwinds the whole stack when the topmost dialog scopes Escape to it', () => { + const lower = build({ defaultOpen: true, id: 'lower' }) + const upper = build({ defaultOpen: true, id: 'upper', escapeScope: 'stack' }) + mountLayer('lower', 1, '', () => lower.send({ type: 'close' })) + mountLayer('upper', 2, '', () => upper.send({ type: 'close' })) + armEscape(upper) + + pressEscape() + expect([upper.matches('open'), lower.matches('open')]).toEqual([false, false]) + }) + + it('leaves the stack alone when the topmost dialog vetoes its stack-scoped Escape', () => { + const lower = build({ defaultOpen: true, id: 'lower' }) + const upper = build({ defaultOpen: true, id: 'upper', escapeScope: 'stack' }) + mountLayer('lower', 1, '', () => lower.send({ type: 'close' })) + mountLayer('upper', 2, '', () => upper.send({ type: 'close' })) + armEscape(upper, { onEscapeKeyDown: event => event.preventDefault?.() }) + + pressEscape() + expect([upper.matches('open'), lower.matches('open')]).toEqual([true, true]) + }) + it('detaches its listener on dispose', () => { const service = build({ defaultOpen: true }) mountLayer('dlg', 1) @@ -233,6 +260,100 @@ describe('startExitWindow', () => { }) }) +describe('guardBackNavigation', () => { + // jsdom's history traversal is asynchronous — await the popstate itself. + const nextPop = (): Promise => + new Promise(resolve => { + window.addEventListener('popstate', () => resolve(), { once: true }) + }) + + // A dialog reduced to what the guard reads: an open flag the core would + // move, plus the host's job of reporting every change — and only a change, + // the way an effect keyed on the open state does. + const wire = (): { + isOpen: () => boolean + open: () => void + close: () => void + report: () => void + release: () => void + } => { + let opened = false + let reported = false + const guard = guardBackNavigation({ + backNavigate: () => void (opened = false), + forwardNavigate: () => void (opened = true), + isOpen: () => opened, + depth: 1, + }) + const report = (): void => { + if (opened === reported) return + reported = opened + guard.sync(opened) + } + return { + isOpen: () => opened, + open: () => { + opened = true + report() + }, + close: () => { + opened = false + report() + }, + report, + release: guard.release, + } + } + + // A host traversal, then the report the substrate makes once it landed. + const traverse = async (dialog: ReturnType, go: () => void): Promise => { + const pop = nextPop() + go() + await pop + dialog.report() + } + + it('parks a Back-closed dialog so Forward reopens it, guarded again', async () => { + const dialog = wire() + dialog.open() + + await traverse(dialog, () => window.history.back()) + expect(dialog.isOpen()).toBe(false) + + await traverse(dialog, () => window.history.forward()) + expect(dialog.isOpen()).toBe(true) + + await traverse(dialog, () => window.history.back()) + expect(dialog.isOpen()).toBe(false) + dialog.release() // parked, so nothing left to consume + }) + + it('releases on a close by any other means — Forward reopens nothing', async () => { + const dialog = wire() + dialog.open() + + // The release consumes the still-current guard entry through a real + // traversal; settle it here rather than in the next test. + const consume = nextPop() + dialog.close() + await consume + + await traverse(dialog, () => window.history.forward()) + expect(dialog.isOpen()).toBe(false) + }) + + it('release ends a parked episode — the Forward watch goes with it', async () => { + const dialog = wire() + dialog.open() + + await traverse(dialog, () => window.history.back()) + dialog.release() + + await traverse(dialog, () => window.history.forward()) + expect(dialog.isOpen()).toBe(false) + }) +}) + describe('outside-press gating', () => { it('lets only the topmost dialog answer a backdrop press', () => { mountLayer('dlg', 1) diff --git a/packages/dom/utils/focus-trap/SPEC.md b/packages/dom/utils/focus-trap/SPEC.md index ad72dfb..e365aa6 100644 --- a/packages/dom/utils/focus-trap/SPEC.md +++ b/packages/dom/utils/focus-trap/SPEC.md @@ -64,8 +64,8 @@ identical containment. ## Internals -| Position | Why | -| --------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | -| Focus is stepped manually on every press, not only at the edges | The `last` re-ordering makes the logical cycle diverge from DOM order, so native tabbing can't be trusted mid-cycle. | -| Document-level, capture-phase keydown listener | A container listener misses presses while focus is still outside; capture delivery survives a `stopPropagation` in the subtree. | -| Rendered-ness via `Element.checkVisibility()` | The platform's own check, no bespoke style probing; jsdom lacks it, so the test setup (`vitest.setup.ts`) shims it for every suite. | +| Position | Why | +| ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Focus is stepped manually on every press, not only at the edges | The `last` re-ordering makes the logical cycle diverge from DOM order, so native tabbing can't be trusted mid-cycle. | +| Document-level, capture-phase keydown listener | A container listener misses presses while focus is still outside; capture delivery survives a `stopPropagation` in the subtree. | +| Rendered-ness via a computed-style walk, not `Element.checkVisibility()` | The API is recent (Chrome/Edge 105+, Firefox 106+, Safari 17.4+) and the trap resolves focusables after the Tab keydown's `preventDefault()`, so on any browser without it the throw would leave Tab dead entirely; the walk is spec-defined behavior everywhere (and needs no test-environment shim). | diff --git a/packages/dom/utils/focus-trap/src/get-focusables.ts b/packages/dom/utils/focus-trap/src/get-focusables.ts index f71f7da..ae6e901 100644 --- a/packages/dom/utils/focus-trap/src/get-focusables.ts +++ b/packages/dom/utils/focus-trap/src/get-focusables.ts @@ -7,10 +7,31 @@ export const FOCUSABLE_SELECTOR: string = [ 'textarea:not([disabled])', 'audio[controls]', 'video[controls]', + 'iframe', + // Only a details' first summary is the disclosure widget browsers tab to. + 'details > summary:first-of-type', '[contenteditable]:not([contenteditable="false"])', '[tabindex]', ].join(', ') +function isRendered(element: HTMLElement, container: HTMLElement): boolean { + // The attribute check also covers hidden="until-found", which hides via + // content-visibility instead of display. + if (element.closest('[hidden]') !== null) return false + // `visibility` inherits, so the element's own computed value suffices. + const visibility = getComputedStyle(element).visibility + if (visibility === 'hidden' || visibility === 'collapse') return false + // `display` does not inherit, so ancestors must be walked. + for ( + let node: HTMLElement | null = element; + node && node !== container; + node = node.parentElement + ) { + if (getComputedStyle(node).display === 'none') return false + } + return true +} + // A named radio participates in a group; groups are scoped by name AND form // owner, matching the browser's own grouping. function isGroupedRadio(element: HTMLElement): element is HTMLInputElement { @@ -24,7 +45,7 @@ export function getFocusables(container: HTMLElement): HTMLElement[] { const element = candidates[i]! // Focusing a non-rendered element is a no-op, so keeping one in the cycle // would stall the trap on it. - if (element.tabIndex >= 0 && element.checkVisibility({ checkVisibilityCSS: true })) { + if (element.tabIndex >= 0 && isRendered(element, container)) { eligible.push(element) } } diff --git a/packages/dom/utils/focus-trap/tests/trap-focus.test.ts b/packages/dom/utils/focus-trap/tests/trap-focus.test.ts index 546c3ae..675bb97 100644 --- a/packages/dom/utils/focus-trap/tests/trap-focus.test.ts +++ b/packages/dom/utils/focus-trap/tests/trap-focus.test.ts @@ -123,6 +123,25 @@ describe('trapFocus', () => { expect(document.activeElement?.id).toBe('last') }) + it('includes iframes and a details summary in the cycle', () => { + const container = mount( + '' + + '' + + '
morecontent
' + + '', + ) + document.getElementById('first')?.focus() + + pressTab(container) + expect(document.activeElement?.id).toBe('frame') + + pressTab(container) + expect(document.activeElement?.id).toBe('summary') + + pressTab(container) + expect(document.activeElement?.id).toBe('last') + }) + it('collapses a same-name radio group to its checked radio', () => { const container = mount( '' + diff --git a/packages/dom/utils/navigation/SPEC.md b/packages/dom/utils/navigation/SPEC.md index dfeec67..a14e490 100644 --- a/packages/dom/utils/navigation/SPEC.md +++ b/packages/dom/utils/navigation/SPEC.md @@ -6,7 +6,9 @@ Framework-free browser-navigation helpers. Today that is one: `interceptBackNavigation`, the web mechanics behind a layer's Back dismissal (the dialog contract's `closeOnBack`) — a guard entry planted in the session history so the browser's Back closes an overlaid layer (dialog, drawer, -sheet) instead of leaving the page. +sheet) instead of leaving the page. The entry a Back press pops survives in +the forward stack, so for a layer that opts in, the Forward that re-enters +it reopens the layer. ## Behavior @@ -20,11 +22,46 @@ sheet) instead of leaving the page. traversal crossed, topmost first. - **`onBack` returns whether the layer actually closed.** A decline — vetoed, or a controlled layer whose consumer hasn't followed — re-arms the - guard entry, so the next Back reaches the same layer again. -- **Release** (the layer closed by any other means) consumes a still-current - guard entry so it can't swallow the next Back. An entry buried under later - in-app navigation is unreachable and left alone — Back then both navigates - and closes the layer. + guard entry, so the next Back reaches the same layer again. An `onBack` + that throws counts as a decline: the guard re-arms and the error + propagates. +- **Forward reopens** (opt-in `onForward`): the entry a Back press spent + still marks the layer's open ground in the forward stack, and a traversal + re-entering it fires `onForward`, which returns whether the layer actually + reopened — the guard re-arms on the entry in place, no new entry. A decline + keeps the watch: a later traversal into the entry offers the reopen again. + A multi-entry jump across several spent entries reopens each crossed layer, + lowest first. +- **A marked entry with no live owner never unwinds anything.** Marked ground + above the armed guards is forward residue, not a Back — landing there either + reopens the layer or does nothing; it never closes one. +- **A layer torn down mid-episode can still come back.** A layer that was + Back-closed and then destroyed — a nested layer unmounted along with the + surroundings that held it — leaves ground it never gave up. The layer that + takes its place recognizes that ground as its own and reopens from it, even + though the registration that planted the entry is long gone. Only a sole + claimant answers: when two layers claim the same ground there is no telling + which one was there, and reopening the wrong layer is worse than reopening + none. A layer that _closed_ gave its ground up on purpose, so nothing + reopens from it — that is what keeps Forward from undoing a dismissal the + user made deliberately. +- **The Forward watch ends** when the layer releases, when a newly planted + entry truncates the forward stack the spent entry lives in, or when a new + registration adopts the entry. A layer that tears itself down inside + `onBack` — releasing rather than closing — keeps no watch at all: there is + nothing left to reopen. +- **Release** (the layer closed by any other means, or gone for good) + consumes a still-current guard entry so it can't swallow the next Back, + and ends a parked guard's Forward watch. Layers closing together consume + all their entries, one traversal at a time, until a live guard's entry — + or navigation this package doesn't own — surfaces. An entry buried under + later in-app navigation is unreachable and left alone — Back then both + navigates and closes the layer; a released entry a later Back does surface + is consumed then, alongside the layer that press unwound. +- **A whole stack closing at once** — a close-all affordance, an unmounting + subtree — leaves nothing behind either: every entry the layers planted is + gone, so the next Back goes back rather than being spent on a layer that is + no longer there. It makes no difference which layer releases first. - **Release then re-register in the same synchronous turn** nets out to zero traversals: the re-register adopts the entry in place, and the deferred consumption finds it no longer owned and queues nothing. @@ -35,30 +72,58 @@ sheet) instead of leaving the page. ### Reload The guard entry survives a reload; the layer's open-state doesn't, leaving a -dead same-URL entry the first Back appears to spend on nothing. That is out -of this package's scope by design: on reload only the host knows whether the -layer should reopen. A layer that must survive reload (or be shareable, or -reopen on Forward) keeps its open-state in the URL and derives itself from -it — Back then closes for free and needs no interceptor. +same-URL entry the first Back appears to spend on nothing. What the entry does +keep is the layer's claim on it, so the ground is still recognizable: a +traversal back into it reopens the layer. Nothing opens on load itself — only +a traversal onto that ground does. One memory does not survive: whether the +ground was given up on purpose. A deliberate close is remembered in script (an +entry already in the forward stack can no longer be rewritten), so after a +reload the page cannot tell surrendered ground from lost ground, and a +traversal onto either offers the reopen. + +Two things this still doesn't give you: an entry planted before the reload +can't say whether its layer was open when the page went away, so a Back that +lands short of it closes nothing; and the ground is a place in the stack, not a +URL, so it isn't shareable. A layer that must survive reload in its own right +(or be linkable) keeps its open-state in the URL and derives itself from it — +Back then closes for free and needs no interceptor. ## API -| Export | Description | -| --------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- | -| `interceptBackNavigation(onBack)` | Arms a guard; `onBack` fires when the user pops it and returns whether the layer closed. Returns the release for a layer closed by other means. | +| Export | Description | +| ------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `interceptBackNavigation(onBack, options?)` | Arms a guard; `onBack` fires when the user pops it and returns whether the layer closed. `options.onForward` fires when a traversal re-enters the popped entry and returns whether the layer reopened. `options.claim` names the ground so a later layer can recognize it. Returns the release — `{ keepClaim: true }` for a layer torn down rather than closed. | +| `watchSpentEntry(claim, reopen)` | For a closed layer: a landing on spent ground bearing this claim asks it to reopen. Returns the release. | ## Constraints - One shared registry and one `popstate` listener module-wide — the one-pop-one-guard ordering is the whole unwinding contract. -- The listener detaches only when nothing is left to hear: no guards, no - in-flight self-caused pop, and no release still waiting on its deferred - consumption. +- Parked entries always sit above every armed entry: parking only ever pops + topmost entries, and every planted entry truncates the forward stack the + parked ones live in. +- The listener detaches only when nothing is left to hear: no armed guards, no + parked watchers, no layer waiting to claim its ground back, no in-flight + self-caused pop, and no release still waiting on its deferred consumption. +- A claim identifies ground, not an instance: it has to outlive the + registration that planted the entry, which is the whole point, so it can + only ever be as precise as the caller's own naming of that ground. +- **Nested layers that open in the same commit unwind outside-in.** Arming + order is the caller's lifecycle order, and effect-based hosts (React runs + child effects before parent effects) arm an inner layer that mounts + already open beneath its parent — the first Back then closes the outer + layer. A layer that opens after its parent is already mounted unwinds + inside-out as expected. An ordering signal that isn't mount order is + tracked separately. ## Internals -| Position | Why | -| ------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| One registry + one listener across every layer | A Back pops one entry; only the guard whose entry vanished may answer — that ordering is what unwinds stacks one press at a time with no cross-layer bookkeeping. | -| Consumption is deferred a microtask | A queued `history.back()` is not reliably delivered once another entry is pushed before it lands; letting a same-turn re-register adopt the entry removes the race instead of compensating for it. | -| Self-caused pops are counted, and re-arm a live guard whose entry they consumed | The browser reports them through the same `popstate` as a user's Back; uncounted, one release would unwind another layer. | +| Position | Why | +| ---------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| One registry + one listener across every layer | A Back pops one entry; only the guard whose entry vanished may answer — that ordering is what unwinds stacks one press at a time with no cross-layer bookkeeping. | +| Consumption is deferred a microtask | A queued `history.back()` is not reliably delivered once another entry is pushed before it lands; letting a same-turn re-register adopt the entry removes the race instead of compensating for it. | +| Self-caused pops are counted, and re-arm a live guard whose entry they consumed | The browser reports them through the same `popstate` as a user's Back; uncounted, one release would unwind another layer. | +| A Back-closed guard parks instead of dropping; ownership of the landing entry — not traversal direction — decides reopen vs unwind | `popstate` carries no direction. A parked or stale marker can only be forward residue above the armed guards (pushes truncate it everywhere else), so landing on one must never unwind — it would close layers on a Forward. | +| Reopening re-arms the guard on the spent entry in place | The traversal already made the entry current; planting another would truncate the remaining forward stack and stack junk entries. | +| Sibling releases consume entries one traversal at a time, not one `history.go(-n)` | Entries below the current one are opaque, so a multi-step jump could cross navigation this package doesn't own; chaining single pops — each landing continuing the chain — stops at the first entry that isn't ours to spend. Call order still can't matter: the pending set is order-free. | +| Built on the History API, not the Navigation API | The Navigation API answers natively what this module reconstructs — whose traversal it was and which direction it ran — dissolving the self-caused-pop counting and the direction inference. It is not cross-browser yet (Chromium ships it; Safari and Firefox don't fully); once it is, this module should be rebuilt on it. | diff --git a/packages/dom/utils/navigation/src/index.ts b/packages/dom/utils/navigation/src/index.ts index 9918093..c21fc06 100644 --- a/packages/dom/utils/navigation/src/index.ts +++ b/packages/dom/utils/navigation/src/index.ts @@ -1 +1,6 @@ -export { interceptBackNavigation } from './intercept-back-navigation' +export { + interceptBackNavigation, + watchSpentEntry, + type BackNavigationOptions, + type ReleaseOptions, +} from './intercept-back-navigation' diff --git a/packages/dom/utils/navigation/src/intercept-back-navigation.ts b/packages/dom/utils/navigation/src/intercept-back-navigation.ts index 093a3b4..a9ecea0 100644 --- a/packages/dom/utils/navigation/src/intercept-back-navigation.ts +++ b/packages/dom/utils/navigation/src/intercept-back-navigation.ts @@ -1,29 +1,61 @@ // Marks a layer's guard entry in the session history; the value says which // interceptor owns the entry. const STATE_KEY = '@dunky.back' +// The layer's name for its ground — outlives the registration (and a reload), +// so a layer that comes back can recognize its entry (see `watchSpentEntry`). +const CLAIM_KEY = '@dunky.claim' interface BackGuard { id: number + claim: string | undefined onBack: () => boolean + onForward: (() => boolean) | undefined } -// One shared registry + one popstate listener across every layer: a Back -// press pops exactly one entry, so only the interceptor whose guard entry -// vanished may answer — the ones beneath see their entry still current and -// stay armed. That ordering is what makes stacked layers (nested dialogs, -// a drawer under a sheet) unwind one per press with no cross-layer -// bookkeeping. +export interface BackNavigationOptions { + /** Fires when a traversal re-enters the entry a Back press spent; returns + * whether the layer actually reopened. */ + onForward?: () => boolean + /** Stamped into the entry; see `watchSpentEntry`. */ + claim?: string +} + +// Closed layers waiting for a landing on a spent entry bearing their claim — +// the way back when the registration that planted the entry didn't survive. +interface ClaimWatcher { + claim: string + reopen: () => boolean +} + +const watchers: ClaimWatcher[] = [] +// Entries whose layer closed rather than being torn down: given up on purpose, +// so no later layer may claim them — Forward must not undo a deliberate close. +const abandoned = new Set() + +export interface ReleaseOptions { + /** The layer is being torn down, not closed — keep its entry claimable so + * the layer that takes its place may reopen from it. @default false */ + keepClaim?: boolean +} + +// One shared registry + one popstate listener: a Back pops exactly one entry, +// so only the guard whose entry vanished answers — stacked layers unwind one +// per press with no cross-layer bookkeeping. const guards: BackGuard[] = [] +// Guards whose entry a Back press popped, kept so the host's Forward can +// reopen the layer. Parked entries always sit above every armed one. +const parked: BackGuard[] = [] let nextGuardId = 0 -// Pops this module caused itself (consuming a guard entry on release). The -// browser reports them through the same popstate as a user's Back — count -// them so they are never read as one and unwind another layer. +// Pops this module caused itself — counted so they are never read as a user's +// Back and unwind another layer. let swallow = 0 -// Releases whose deferred consumption hasn't run yet. Each may still queue a -// self-caused pop, so the listener must outlive them: without this, one -// release's idle check could detach the listener while a sibling release from -// the same turn is about to call history.back(). -let pendingReleases = 0 +// Entries whose guards released but whose consumption hasn't happened yet. +// Sibling releases from one turn all land here; consumption then chains one +// traversal at a time (each pop surfaces the next spent entry) instead of one +// history.go(-n) jump, because entries below the current one are opaque — a +// multi-step jump could cross navigation this module doesn't own. +const pendingConsumption = new Set() +let consumptionScheduled = false function currentGuardId(): number | undefined { const state: unknown = history.state @@ -32,16 +64,72 @@ function currentGuardId(): number | undefined { return typeof id === 'number' ? id : undefined } -function isRegistered(id: number): boolean { +function currentClaim(): string | undefined { + const state: unknown = history.state + if (typeof state !== 'object' || state === null) return undefined + const claim = (state as Record)[CLAIM_KEY] + return typeof claim === 'string' ? claim : undefined +} + +// Offers a spent entry to the layer that has taken the planter's place. Only a +// sole candidate may answer: two layers claiming the same ground can't be told +// apart, and reopening the wrong one is worse than reopening none. +function offerToClaimant(): void { + const id = currentGuardId() + if (id !== undefined && abandoned.has(id)) return + const claim = currentClaim() + if (claim === undefined) return + let candidate: ClaimWatcher | undefined + for (const watcher of watchers) { + if (watcher.claim !== claim) continue + if (candidate !== undefined) return + candidate = watcher + } + candidate?.reopen() +} + +function isArmed(id: number): boolean { for (const guard of guards) if (guard.id === id) return true return false } -// The listener detaches only when nothing is left to hear: an in-flight -// self-caused pop (swallow) or an undecided release (pendingReleases) still -// needs it even with every guard released. +function parkedIndex(id: number): number { + for (let index = 0; index < parked.length; index++) { + if ((parked[index] as BackGuard).id === id) return index + } + return -1 +} + +// Every planted entry truncates the forward stack, taking every parked entry +// with it — the guards watching them have nothing left to hear. +function plantEntry(guard: BackGuard): void { + parked.length = 0 + history.pushState({ [STATE_KEY]: guard.id, [CLAIM_KEY]: guard.claim }, '') +} + +// Consume the current entry if its guard released: one history.back() whose +// pop re-enters onPopState and continues the chain from there. +function consumeCurrentIfPending(): void { + const current = currentGuardId() + if (current !== undefined && pendingConsumption.delete(current)) { + swallow++ + history.back() + } +} + +// Detach only when nothing is left to hear — parked guards, claim watchers, +// in-flight self-caused pops, and a scheduled consumption pass all still need +// it. Entries still pending at that point are buried under navigation this +// module doesn't own — unreachable for good. function detachWhenIdle(): void { - if (guards.length === 0 && swallow === 0 && pendingReleases === 0) { + if ( + guards.length === 0 && + parked.length === 0 && + watchers.length === 0 && + swallow === 0 && + !consumptionScheduled + ) { + pendingConsumption.clear() window.removeEventListener('popstate', onPopState) } } @@ -53,75 +141,161 @@ function onPopState(): void { // (it adopted the entry while the traversal was in flight), re-arm it. const top = guards[guards.length - 1] if (top !== undefined && top.id !== currentGuardId()) { - history.pushState({ [STATE_KEY]: top.id }, '') + plantEntry(top) + } else { + // The pop may have surfaced the next spent sibling entry — continue. + consumeCurrentIfPending() + } + detachWhenIdle() + return + } + const current = currentGuardId() + // A marked entry with no armed owner and no pending consumption is forward + // residue and never unwinds anything. A parked owner reopens (every crossed + // guard, lowest first; a decline stays parked). No owner at all: offer the + // entry's claim to the layer that took the planter's place. An entry + // pending consumption is the opposite of residue — a dead entry a user's + // Back just surfaced — so it falls through to the unwind below. + if (current !== undefined && !isArmed(current) && !pendingConsumption.has(current)) { + const landed = parkedIndex(current) + if (landed !== -1) { + for (let index = parked.length - 1; index >= landed; index--) { + const guard = parked[index] as BackGuard + if (guard.onForward?.() === true) { + // Reopened: re-arm on the entry in place — it is already current, + // and planting another would truncate the rest of the way forward. + parked.splice(index, 1) + guards.push(guard) + } + } + } else { + offerToClaimant() } detachWhenIdle() return } // Unwind every guard the traversal jumped over, topmost first — a Back // press covers one; a multi-entry jump (history.go(-n)) covers several. - const current = currentGuardId() while (guards.length > 0) { const top = guards[guards.length - 1] as BackGuard if (top.id === current) break - if (top.onBack()) { - // By identity, not position: onBack may have released this guard - // itself, and a positional pop would evict the guard beneath. - const index = guards.indexOf(top) - if (index !== -1) guards.splice(index, 1) - continue + let closed = false + try { + closed = top.onBack() + } finally { + // Declined — vetoed, a controlled layer that hasn't followed yet, or + // onBack threw: re-arm the guard entry so the next Back reaches this + // layer again, even as the error propagates. + if (!closed) plantEntry(top) + } + if (!closed) break + // By identity, not position: onBack may have released this guard + // itself, and a positional pop would evict the guard beneath. + const index = guards.indexOf(top) + if (index !== -1) { + guards.splice(index, 1) + // Park for the way back — unless the guard released itself in onBack: + // gone for good, nothing left to reopen. + if (top.onForward !== undefined) parked.push(top) } - // Declined — vetoed, or a controlled layer that hasn't followed yet: - // re-arm the guard entry so the next Back reaches this layer again. - history.pushState({ [STATE_KEY]: top.id }, '') - break } + // The unwind may have landed on an entry whose guard already released (a + // mid-stack release buried beneath a live layer) — consume it. + consumeCurrentIfPending() detachWhenIdle() } /** - * Plants a guard entry in the session history so the host's Back dismisses a - * layer (a dialog, drawer, sheet — anything overlaid) instead of leaving the - * page. `onBack` fires when the user pops the entry and returns whether the - * layer actually closed — a decline re-arms the guard. The returned release - * (for a layer closed by any other means) consumes a still-current guard - * entry so it can't swallow the next Back; an entry buried under later - * navigation is unreachable and left alone. + * Plants a guard entry so the host's Back dismisses a layer instead of leaving + * the page. `onBack` returns whether the layer closed — a decline re-arms. The + * returned release consumes a still-current entry (a buried one is left + * alone); with `onForward`, Forward reopens what Back closed, re-armed on the + * entry in place. * - * Consumption is deferred a microtask so a release immediately followed by a - * re-register in the same synchronous turn nets out to zero traversals: the - * re-register finds the entry still current but no longer owned and adopts it - * in place (rewrites the marker), so when the deferred consumption runs the - * entry is no longer this guard's and no `history.back()` is queued. That - * matters because a traversal queued by `history.back()` is not reliably - * delivered once another entry is pushed before it lands; not queuing one in - * that window removes the race instead of compensating for it. + * Consumption is deferred a microtask so a same-turn release + re-register + * adopts the entry in place (rewrites the marker and withdraws it from pending + * consumption), so the deferred pass finds nothing to spend and queues no + * traversal — a queued `history.back()` is not reliably delivered once another + * push lands first, so not queuing one removes the race. See SPEC.md for the + * full contract. */ -export function interceptBackNavigation(onBack: () => boolean): () => void { - const guard: BackGuard = { id: ++nextGuardId, onBack } +export function interceptBackNavigation( + onBack: () => boolean, + options: BackNavigationOptions = {}, +): (releaseOptions?: ReleaseOptions) => void { + const guard: BackGuard = { + id: ++nextGuardId, + claim: options.claim, + onBack, + onForward: options.onForward, + } // Identical (type, listener) pairs dedupe, so attaching is idempotent. window.addEventListener('popstate', onPopState) const current = currentGuardId() - const adoptable = current !== undefined && !isRegistered(current) guards.push(guard) - if (adoptable) history.replaceState({ [STATE_KEY]: guard.id }, '') - else history.pushState({ [STATE_KEY]: guard.id }, '') + if (current !== undefined && !isArmed(current)) { + // Adoption steals the entry from a parked watcher too, and withdraws it + // from any pending consumption — the ground now belongs to this + // registration, so no traversal may spend it. + const stale = parkedIndex(current) + if (stale !== -1) parked.splice(stale, 1) + pendingConsumption.delete(current) + history.replaceState({ [STATE_KEY]: guard.id, [CLAIM_KEY]: guard.claim }, '') + } else { + plantEntry(guard) + } - return () => { - const index = guards.indexOf(guard) - if (index === -1) return // already unwound by the Back press itself - guards.splice(index, 1) - pendingReleases++ - queueMicrotask(() => { - pendingReleases-- - // Still ours and still current: nobody adopted it and no Back popped - // it — consume the entry. The listener stays until the pop lands. + return (releaseOptions: ReleaseOptions = {}) => { + if (releaseOptions.keepClaim !== true) abandoned.add(guard.id) + + const rest = parked.indexOf(guard) + if (rest !== -1) { + parked.splice(rest, 1) + // A parked guard's entry sits in the forward stack — not the chain's to + // spend — unless a declined reopen left it current: consume that one, + // or it swallows the next Back. if (currentGuardId() === guard.id) { - swallow++ - history.back() + pendingConsumption.add(guard.id) + scheduleConsumption() } else { detachWhenIdle() } - }) + return + } + const index = guards.indexOf(guard) + if (index === -1) return // already unwound by the Back press itself + guards.splice(index, 1) + pendingConsumption.add(guard.id) + scheduleConsumption() + } +} + +// One deferred pass per turn, shared by every sibling release; it starts the +// consumption chain, and each landing pop continues it. +function scheduleConsumption(): void { + if (consumptionScheduled) return + consumptionScheduled = true + queueMicrotask(() => { + consumptionScheduled = false + consumeCurrentIfPending() + detachWhenIdle() + }) +} + +/** + * Reopens a layer whose guard is gone (unmounted, or reloaded): landing on a + * spent entry with a matching `claim` asks `reopen`. If two watchers share a + * claim, neither answers. + */ +export function watchSpentEntry(claim: string, reopen: () => boolean): () => void { + const watcher: ClaimWatcher = { claim, reopen } + // Identical (type, listener) pairs dedupe, so attaching is idempotent. + window.addEventListener('popstate', onPopState) + watchers.push(watcher) + return () => { + const index = watchers.indexOf(watcher) + if (index === -1) return + watchers.splice(index, 1) + detachWhenIdle() } } diff --git a/packages/dom/utils/navigation/tests/intercept-back-navigation.test.ts b/packages/dom/utils/navigation/tests/intercept-back-navigation.test.ts index 0386b44..b4c1bfe 100644 --- a/packages/dom/utils/navigation/tests/intercept-back-navigation.test.ts +++ b/packages/dom/utils/navigation/tests/intercept-back-navigation.test.ts @@ -15,6 +15,20 @@ const pressBack = async (): Promise => { await pop } +const pressForward = async (): Promise => { + const pop = nextPop() + history.forward() + await pop +} + +// Releasing consumes a still-current entry through an async self-caused pop — +// await it so the next test starts from settled history. +const releaseAndSettle = async (release: () => void): Promise => { + const pop = nextPop() + release() + await pop +} + describe('interceptBackNavigation', () => { it('plants a guard entry; Back pops it and fires onBack once', async () => { const before: unknown = history.state @@ -70,8 +84,8 @@ describe('interceptBackNavigation', () => { }) // Two sibling layers closing in the same commit: the first release's - // deferred check must not detach the listener while the second release's - // self-caused pop is still on its way. + // deferred check must not detach the listener while the consumption chain's + // pops are still on their way. it('two releases in the same turn leave the next guard able to hear Back', async () => { const before: unknown = history.state const releaseLower = interceptBackNavigation(() => true) @@ -80,7 +94,8 @@ describe('interceptBackNavigation', () => { const pop = nextPop() releaseLower() releaseUpper() - await pop // the upper release's self-caused pop lands + await pop // the consumption chain's first pop lands... + await nextPop() // ...and its second consumes the sibling entry beneath const onBack = vi.fn(() => true) interceptBackNavigation(onBack) @@ -89,6 +104,80 @@ describe('interceptBackNavigation', () => { expect(history.state).toEqual(before) }) + // A whole stack freed in one commit — close-all, or an unmounting subtree. + // Only the topmost entry is current, so a single check would leave every + // entry beneath behind, each swallowing a later Back. The chain consumes one + // traversal at a time; release order is irrelevant. + it('a stack released in one turn consumes every entry it planted', async () => { + const before: unknown = history.state + const releaseLower = interceptBackNavigation(() => true) + const releaseUpper = interceptBackNavigation(() => true) + + const pop = nextPop() + releaseUpper() + releaseLower() + await pop + await nextPop() + expect(history.state).toEqual(before) + + const outerFirst = interceptBackNavigation(() => true) + const innerFirst = interceptBackNavigation(() => true) + const second = nextPop() + outerFirst() + innerFirst() + await second + await nextPop() + expect(history.state).toEqual(before) + }) + + // A mid-stack release buried beneath a live layer: the next Back pops the + // live guard's entry and lands on the dead one. The guard the press crossed + // must still close, and the dead entry must be consumed rather than left to + // swallow the press. + it('a Back landing on a released entry beneath a live guard still unwinds it', async () => { + const before: unknown = history.state + const lower = vi.fn(() => true) + const upper = vi.fn(() => true) + const releaseLower = interceptBackNavigation(lower) + interceptBackNavigation(upper) + + releaseLower() // buried under the upper guard's entry — nothing to consume yet + await new Promise(resolve => queueMicrotask(resolve)) + + const pop = nextPop() + history.back() + await pop // the user's Back: closes upper, lands on the dead entry... + await nextPop() // ...which the chain consumes + expect(upper).toHaveBeenCalledTimes(1) + expect(lower).not.toHaveBeenCalled() + expect(history.state).toEqual(before) + }) + + // A throwing onBack must not corrupt the unwind: the layer didn't confirm + // closing, so it counts as a decline while the error propagates. + it('a throwing onBack re-arms the guard and keeps it reachable', async () => { + const before: unknown = history.state + let shouldThrow = true + const onBack = vi.fn(() => { + if (shouldThrow) throw new Error('consumer bug') + return true + }) + interceptBackNavigation(onBack) + + // jsdom reports listener exceptions through window "error" — absorb it. + const absorb = (event: ErrorEvent): void => event.preventDefault() + window.addEventListener('error', absorb) + await pressBack() + window.removeEventListener('error', absorb) + expect(onBack).toHaveBeenCalledTimes(1) + expect(history.state).not.toEqual(before) // re-armed + + shouldThrow = false + await pressBack() + expect(onBack).toHaveBeenCalledTimes(2) + expect(history.state).toEqual(before) + }) + it('a guard releasing itself inside onBack leaves the guard beneath armed', async () => { const before: unknown = history.state const lower = vi.fn(() => true) @@ -132,4 +221,103 @@ describe('interceptBackNavigation', () => { expect(first).not.toHaveBeenCalled() expect(history.state).toEqual(before) }) + + it('a Back-closed guard reopens on Forward and re-arms on the entry in place', async () => { + const onBack = vi.fn(() => true) + const onForward = vi.fn(() => true) + const release = interceptBackNavigation(onBack, { onForward }) + await pressBack() + expect(onBack).toHaveBeenCalledTimes(1) + + const lengthBefore = history.length + await pressForward() + expect(onForward).toHaveBeenCalledTimes(1) + expect(history.length).toBe(lengthBefore) // re-armed in place, nothing planted + + await pressBack() // the re-armed guard answers the next Back + expect(onBack).toHaveBeenCalledTimes(2) + release() + await new Promise(resolve => queueMicrotask(resolve)) + }) + + it('a declined reopen keeps watching; a later Forward offers again', async () => { + let accept = false + const onForward = vi.fn(() => accept) + const release = interceptBackNavigation(() => true, { onForward }) + await pressBack() + + await pressForward() + expect(onForward).toHaveBeenCalledTimes(1) // declined — still parked + + await pressBack() // a plain navigation off the declined entry + accept = true + await pressForward() + expect(onForward).toHaveBeenCalledTimes(2) + await releaseAndSettle(release) // accepted — armed again, entry current + }) + + it('release while parked ends the Forward watch', async () => { + const onForward = vi.fn(() => true) + const release = interceptBackNavigation(() => true, { onForward }) + await pressBack() + release() + await new Promise(resolve => queueMicrotask(resolve)) + + await pressForward() // re-enters the now-unwatched entry + expect(onForward).not.toHaveBeenCalled() + await pressBack() // step off the stale entry + }) + + // A layer that tears itself down inside onBack is gone, not Back-closed: + // parking it would offer a reopen to something that no longer exists. + it('a guard releasing itself inside onBack never parks', async () => { + const onForward = vi.fn(() => true) + let release = (): void => undefined + release = interceptBackNavigation( + () => { + release() + return true + }, + { onForward }, + ) + + await pressBack() + await pressForward() // re-enters the spent entry, nobody watching + expect(onForward).not.toHaveBeenCalled() + await pressBack() // step off the stale entry + }) + + it('a newly planted entry ends the Forward watch of the layer before it', async () => { + const firstForward = vi.fn(() => true) + interceptBackNavigation(() => true, { onForward: firstForward }) + await pressBack() // parked, entry in the forward stack + + const second = vi.fn(() => true) + interceptBackNavigation(second) // planting truncates the parked entry + await pressBack() + expect(second).toHaveBeenCalledTimes(1) + + await pressForward() // lands on second's spent entry, nobody watching + expect(firstForward).not.toHaveBeenCalled() + await pressBack() // step off the stale entry + }) + + it('stacked Back-closed guards reopen one per Forward, lowest first', async () => { + const lowerForward = vi.fn(() => true) + const upperForward = vi.fn(() => true) + const releaseLower = interceptBackNavigation(() => true, { onForward: lowerForward }) + const releaseUpper = interceptBackNavigation(() => true, { onForward: upperForward }) + await pressBack() + await pressBack() + + await pressForward() + expect(lowerForward).toHaveBeenCalledTimes(1) + expect(upperForward).not.toHaveBeenCalled() + + await pressForward() + expect(upperForward).toHaveBeenCalledTimes(1) + + await releaseAndSettle(releaseUpper) + await releaseAndSettle(releaseLower) + }) }) diff --git a/packages/dom/utils/overlay/SPEC.md b/packages/dom/utils/overlay/SPEC.md index 19992dd..d2a40e1 100644 --- a/packages/dom/utils/overlay/SPEC.md +++ b/packages/dom/utils/overlay/SPEC.md @@ -21,11 +21,23 @@ against each other. - Every open overlay registers with its element, modality, depth, and — when it has one — its backdrop. Topmost follows the core stack's rule. -- While a modal layer is topmost, everything outside its subtree is hidden - from assistive tech and taken out of pointer and keyboard reach - (`aria-hidden` + `inert` on the siblings of its ancestor path). The - layer's own backdrop — rendered outside the content's subtree yet part of - the layer — stays pressable so an outside press can still dismiss. +- Containment follows the **topmost modal layer**, not the topmost layer. + Everything outside that layer's subtree is hidden from assistive tech and + taken out of pointer and keyboard reach (`aria-hidden` + `inert` on the + siblings of its ancestor path). Two kinds of element are held out of it: + the layer's own backdrop — rendered outside the content's subtree yet part + of the layer — so an outside press can still dismiss, and every layer + stacked above it. +- A non-modal layer above a modal one does not release the modal layer's + containment. The ordinary layers — a select menu, a combobox list, a + tooltip, a context menu — are non-modal, and living inside a dialog is + their normal habitat; `aria-modal` means the modal window is the only + content exposed for as long as it is open, so containment cannot lapse + just because a menu opened on top of it. Such a layer still takes over + Escape and the focus trap, because it is topmost; it simply leaves + containment where it is. Since these layers portal to the body — siblings + of the dialog rather than descendants — holding them out of the + containment is what keeps them reachable. - Containment re-syncs on every stack change: a nested layer hides the one beneath it, and closing it restores the layer. Restoring puts back exactly what was there — an `inert` or a truthy `aria-hidden` the author already @@ -60,14 +72,15 @@ again — but keeps painting until its exit visual finishes: ## API -| Export | Description | -| ------------------------------------------------ | ------------------------------------------------------------------------------------ | -| `registerLayer(layer)` | Joins the shared stack and syncs containment; returns the disposer that restores it. | -| `Layer` | `OverlayLayer` + `element`, `modal`, and an optional `backdrop` getter. | -| `isTopmostLayer(id)` | Whether the layer owns Escape and the focus trap right now. | -| `getInitialFocus(content)` | The element to focus on open: first form field, else the overlay window itself. | -| `hideExitingLayer(content, boundary, backdrop?)` | Inerts the still-painting layer for the exit window; returns the undo. | -| `watchExitAnimation(element, onComplete)` | Reports the exit visual's end once; returns the cancel. | +| Export | Description | +| ------------------------------------------------ | ---------------------------------------------------------------------------------------------- | +| `registerLayer(layer)` | Joins the shared stack and syncs containment; returns the disposer that restores it. | +| `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. | +| `getInitialFocus(content)` | The element to focus on open: first form field, else the overlay window itself. | +| `hideExitingLayer(content, boundary, backdrop?)` | Inerts the still-painting layer for the exit window; returns the undo. | +| `watchExitAnimation(element, onComplete)` | Reports the exit visual's end once; returns the cancel. | ## Constraints @@ -85,5 +98,6 @@ again — but keeps painting until its exit visual finishes: | -------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | The store anchors on a realm-global keyed by `Symbol.for`, resolved lazily | A monorepo or micro-frontend can load duplicate copies of this module; separate stores drift apart (the duplicate-singleton bug class of radix-ui/primitives#2815). Lazy keeps `sideEffects: false` honest. | | Containment re-runs from scratch on every stack change | Undo-then-rehide is idempotent and order-free; incremental patching would have to reason about interleaved opens and closes. | +| Excluded elements match by containment, not identity | A layer above is reached through its own portal wrapper, and it is the wrapper that turns up as the sibling on the walk; an identity check would inert the layer with it. | | Containment sync guards on `element.isConnected` | At teardown the content may already be detached; hiding against a dead node would leak the undo. | | Completion is the element's own end event, first one wins | A transition ends once per property and descendants bubble theirs; the exit belongs to the element carrying `data-state`, styled to finish as one piece. | diff --git a/packages/dom/utils/overlay/src/hide-outside.ts b/packages/dom/utils/overlay/src/hide-outside.ts index a26be4d..6e0c3d4 100644 --- a/packages/dom/utils/overlay/src/hide-outside.ts +++ b/packages/dom/utils/overlay/src/hide-outside.ts @@ -5,26 +5,40 @@ const HIDE_SKIP = /^(SCRIPT|STYLE|LINK|TEMPLATE)$/ * The containment trick: walks from `target` up to the document root and marks * every sibling along the way `aria-hidden` + `inert`, so assistive tech sees * only the target's subtree and nothing outside it can be reached — by pointer, - * find-in-page, or programmatic focus. `exclude` names the one same-layer - * element rendered outside the target's subtree (the layer's own backdrop, - * portalled alongside its viewport) that must stay pressable. Returns a - * function that removes exactly what it added. Callers hide one target at a - * time. + * find-in-page, or programmatic focus. `exclude` names the elements rendered + * outside the target's subtree that must stay reachable: the layer's own + * backdrop (portalled alongside its viewport, and it must stay pressable) and + * the layers stacked above the target. A match is by containment, not + * identity — an excluded element is usually nested inside its own portal + * wrapper, and it is the wrapper that turns up as the sibling on the walk. + * Returns a function that removes exactly what it added. Callers hide one + * target at a time. */ -export function hideOutside(target: HTMLElement, exclude?: Element | null): () => void { +export function hideOutside(target: HTMLElement, exclude?: readonly Element[]): () => void { const hidden: Array<[Element, string | null]> = [] + // Hoisted out of the sibling loop: hot path, and the closure a `.some()` + // would allocate per sibling buys nothing here. `Node.contains` returns true + // for the node itself, so this also covers the direct-sibling backdrop case. + function isExcluded(sibling: Element): boolean { + if (exclude === undefined) return false + for (const element of exclude) { + if (sibling.contains(element)) return true + } + return false + } + let node: HTMLElement | null = target while (node !== null && node !== document.body && node.parentElement !== null) { for (const sibling of Array.from(node.parentElement.children)) { - // Skip the path itself, the layer's own excluded element, content-less - // tags, and anything the author already hides — an existing `inert` or - // a truthy `aria-hidden` is theirs. `aria-hidden="false"` asserts - // visible, the opposite of author-hidden, so it doesn't count. + // Skip the path itself, the excluded elements, content-less tags, and + // anything the author already hides — an existing `inert` or a truthy + // `aria-hidden` is theirs. `aria-hidden="false"` asserts visible, the + // opposite of author-hidden, so it doesn't count. const ariaHidden = sibling.getAttribute('aria-hidden') if ( sibling === node || - sibling === exclude || + isExcluded(sibling) || HIDE_SKIP.test(sibling.tagName) || (ariaHidden !== null && ariaHidden !== 'false') || sibling.hasAttribute('inert') diff --git a/packages/dom/utils/overlay/src/index.ts b/packages/dom/utils/overlay/src/index.ts index eee4d03..1753d05 100644 --- a/packages/dom/utils/overlay/src/index.ts +++ b/packages/dom/utils/overlay/src/index.ts @@ -1,4 +1,4 @@ -export { registerLayer, isTopmostLayer, type Layer } from './stack' +export { registerLayer, isTopmostLayer, layersBelow, type Layer } from './stack' 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/stack.ts b/packages/dom/utils/overlay/src/stack.ts index 6ad11f1..c0c8a90 100644 --- a/packages/dom/utils/overlay/src/stack.ts +++ b/packages/dom/utils/overlay/src/stack.ts @@ -14,6 +14,12 @@ export interface Layer extends OverlayLayer { * closes — sees the element current at that moment. */ backdrop?: () => Element | null + /** + * Closes this layer, for a dismissal scoped to the whole stack rather than + * one layer: the layer that received the intent unwinds the ones beneath by + * calling theirs. A layer that provides none opts out and stays open. + */ + dismiss?: () => void } // One Escape closes exactly one layer only if every overlay shares a single @@ -41,16 +47,39 @@ function getStore(): OverlayStore { return store } -// Keep the assistive-tech view in sync: only the topmost modal layer stays -// reachable; everything else is hidden. Re-runs whenever the stack changes so a -// nested layer hides the one beneath it, and closing it restores the layer. +// Keep the assistive-tech view in sync: the topmost modal layer and the layers +// stacked above it stay reachable; everything else is hidden. Re-runs whenever +// the stack changes so a nested layer hides the one beneath it, and closing it +// restores the layer. function syncContainment(store: OverlayStore): void { store.undoHide?.() store.undoHide = undefined - const top = store.stack.topmost() - if (top?.modal !== true) return + + // Containment follows the topmost *modal* layer, not the topmost layer: the + // ordinary layers — a select menu, a combobox list, a tooltip — are + // non-modal and live inside dialogs, and a modal layer's containment must + // not lapse for as long as one of them is open on top of it. + const ordered = store.stack.ordered() + const index = ordered.findIndex(layer => layer.modal) + if (index === -1) return + + const modal = ordered[index] // `isConnected` guards teardown, when the content is already detached. - if (top.element.isConnected) store.undoHide = hideOutside(top.element, top.backdrop?.() ?? null) + if (!modal.element.isConnected) return + + // The layers at or above the modal one are legitimately open and portalled + // outside its subtree, so they'd be caught by its containment: hold them + // out. For the modal layer itself only the backdrop needs it — its element + // is the containment target. + const exclude: Element[] = [] + for (let i = 0; i <= index; i++) { + const layer = ordered[i] + if (i !== index) exclude.push(layer.element) + const backdrop = layer.backdrop?.() + if (backdrop != null) exclude.push(backdrop) + } + + store.undoHide = hideOutside(modal.element, exclude) } export function registerLayer(layer: Layer): () => void { @@ -66,3 +95,10 @@ export function registerLayer(layer: Layer): () => void { export function isTopmostLayer(id: string): boolean { return getStore().stack.isTopmost(id) } + +// The layers beneath `id`, topmost first — the unwinding order for a +// stack-scoped dismissal. Read it before closing the layer that received the +// intent: leaving the stack takes the answer with it. +export function layersBelow(id: string): Layer[] { + return getStore().stack.below(id) +} diff --git a/packages/dom/utils/overlay/tests/containment.test.ts b/packages/dom/utils/overlay/tests/containment.test.ts index 1dfd159..3e10ddf 100644 --- a/packages/dom/utils/overlay/tests/containment.test.ts +++ b/packages/dom/utils/overlay/tests/containment.test.ts @@ -138,6 +138,73 @@ describe('registerLayer containment', () => { }) }) +describe('containment under a non-modal layer', () => { + // The ordinary layers — select menu, combobox list, tooltip, context menu — + // are non-modal and portal to the body, so inside a dialog they land as a + // sibling of it rather than a descendant. + const setup = (): { + outside: HTMLElement + dialog: MountedLayer + menu: MountedLayer + unregisterMenu: () => void + } => { + const outside = document.createElement('main') + document.body.append(outside) + const dialog = mountLayer() + const menu = mountLayer() + + register({ + id: 'dialog', + depth: 1, + element: dialog.content, + modal: true, + backdrop: () => dialog.backdrop, + }) + const unregisterMenu = register({ id: 'menu', depth: 2, element: menu.content, modal: false }) + return { outside, dialog, menu, unregisterMenu } + } + + it("keeps the modal layer's containment while a non-modal layer is topmost", () => { + const { outside } = setup() + + // The decoupling: topmost has moved to the non-modal layer — it owns + // Escape and the trap — yet containment stays with the modal layer. + expect(isTopmostLayer('menu')).toBe(true) + expect(hiddenFrom(outside)).toBe(true) + }) + + it('leaves the non-modal layer above reachable through its portal wrapper', () => { + const { outside, menu } = setup() + + // Asserted against live containment: the menu's own portal wrapper is what + // turns up as the sibling on the walk, so an identity-only exclude check + // would inert the menu along with it. + expect(hiddenFrom(outside)).toBe(true) + expect(menu.viewport.hasAttribute('inert')).toBe(false) + expect(menu.content.hasAttribute('inert')).toBe(false) + }) + + it('holds containment through the non-modal layer closing', () => { + const { outside, unregisterMenu } = setup() + + unregisterMenu() + expect(hiddenFrom(outside)).toBe(true) + }) + + it('follows the upper modal layer when a non-modal layer sits above both', () => { + const outer = mountLayer() + const inner = mountLayer() + const menu = mountLayer() + register({ id: 'outer', depth: 1, element: outer.content, modal: true }) + register({ id: 'inner', depth: 2, element: inner.content, modal: true }) + register({ id: 'menu', depth: 3, element: menu.content, modal: false }) + + expect(hiddenFrom(outer.viewport)).toBe(true) + expect(inner.viewport.hasAttribute('inert')).toBe(false) + expect(menu.viewport.hasAttribute('inert')).toBe(false) + }) +}) + describe('layer stack global anchoring', () => { it('shares its stack with a duplicate module copy via the realm global', () => { // A second bundled copy of this module resolves the same stack through this diff --git a/packages/dom/utils/scroll-lock/SPEC.md b/packages/dom/utils/scroll-lock/SPEC.md index 22cfd58..6c47456 100644 --- a/packages/dom/utils/scroll-lock/SPEC.md +++ b/packages/dom/utils/scroll-lock/SPEC.md @@ -50,3 +50,4 @@ behavior. | ----------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | The registry anchors on a realm-global keyed by `Symbol.for`, resolved lazily | Duplicate module copies (monorepo, micro-frontend) would double-lock or leak the lock — the duplicate-singleton bug class of radix-ui/primitives#2815. Lazy keeps `sideEffects: false` honest. | | Restore assigns saved values via `setProperty`, not the camelCase setters | A saved `''` (originally unset) must remove the declaration, which `setProperty` does per CSSOM. | +| Overflow is saved, hidden, and restored per axis, never via the shorthand | The `overflow` shorthand serializes back to `''` unless both longhands are set, so a container scrolling on one axis (`overflow-y: auto`) would save as unset and restore by removing it. | diff --git a/packages/dom/utils/scroll-lock/src/lock-scroll.ts b/packages/dom/utils/scroll-lock/src/lock-scroll.ts index 16fa906..391f330 100644 --- a/packages/dom/utils/scroll-lock/src/lock-scroll.ts +++ b/packages/dom/utils/scroll-lock/src/lock-scroll.ts @@ -1,6 +1,7 @@ interface Lock { count: number - savedOverflow: string + savedOverflowX: string + savedOverflowY: string savedPaddingInlineEnd: string savedPaddingBlockEnd: string } @@ -69,7 +70,13 @@ export function lockScroll(target: HTMLElement = document.body): () => void { if (lock === undefined) { lock = { count: 0, - savedOverflow: target.style.overflow, + // Both axes are handled by longhand throughout — saved, hidden, and + // restored. The `overflow` shorthand serializes back to '' unless both + // longhands are set, so a container that scrolls on one axis + // (`overflow-y: auto`) would save as "unset" and restore by removing + // the consumer's own declaration. + savedOverflowX: target.style.overflowX, + savedOverflowY: target.style.overflowY, savedPaddingInlineEnd: target.style.paddingInlineEnd, savedPaddingBlockEnd: target.style.paddingBlockEnd, } @@ -91,7 +98,8 @@ export function lockScroll(target: HTMLElement = document.body): () => void { }px` } } - target.style.overflow = 'hidden' + target.style.overflowX = 'hidden' + target.style.overflowY = 'hidden' } lock.count++ @@ -104,7 +112,8 @@ export function lockScroll(target: HTMLElement = document.body): () => void { locks.delete(target) // setProperty, not the camelCase setters: a saved `''` (originally unset) // must remove the declaration, which setProperty does per CSSOM. - target.style.setProperty('overflow', held.savedOverflow) + target.style.setProperty('overflow-x', held.savedOverflowX) + target.style.setProperty('overflow-y', held.savedOverflowY) target.style.setProperty('padding-inline-end', held.savedPaddingInlineEnd) target.style.setProperty('padding-block-end', held.savedPaddingBlockEnd) } diff --git a/packages/dom/utils/scroll-lock/tests/lock-scroll.test.ts b/packages/dom/utils/scroll-lock/tests/lock-scroll.test.ts index 8ed942d..c4ccb1b 100644 --- a/packages/dom/utils/scroll-lock/tests/lock-scroll.test.ts +++ b/packages/dom/utils/scroll-lock/tests/lock-scroll.test.ts @@ -2,6 +2,14 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { lockScroll } from '@dunky.dev/dom-scroll-lock' +// Both axes by name, never the `overflow` shorthand: the shorthand can't +// express a one-axis declaration, and jsdom doesn't link it to its longhands, +// so asserting it would pass whatever the axes actually hold. +function expectOverflow(target: HTMLElement, value: 'hidden' | ''): void { + expect(target.style.overflowX).toBe(value) + expect(target.style.overflowY).toBe(value) +} + // Browsers resolve computed lengths on a rendered element to `Npx`; jsdom // returns '' for logical longhands, so tests exercising the padding math hand // lockScroll the values a browser would compute. One-shot: lockScroll reads @@ -18,10 +26,10 @@ afterEach(() => { describe('lockScroll', () => { it('locks body scroll by default and releases it', () => { const release = lockScroll() - expect(document.body.style.overflow).toBe('hidden') + expectOverflow(document.body, 'hidden') release() - expect(document.body.style.overflow).toBe('') + expectOverflow(document.body, '') }) it('holds the lock until the last holder releases, in any order', () => { @@ -29,10 +37,10 @@ describe('lockScroll', () => { const releaseSecond = lockScroll() releaseFirst() - expect(document.body.style.overflow).toBe('hidden') + expectOverflow(document.body, 'hidden') releaseSecond() - expect(document.body.style.overflow).toBe('') + expectOverflow(document.body, '') }) it('ignores a double release', () => { @@ -41,10 +49,10 @@ describe('lockScroll', () => { releaseFirst() releaseFirst() - expect(document.body.style.overflow).toBe('hidden') + expectOverflow(document.body, 'hidden') releaseSecond() - expect(document.body.style.overflow).toBe('') + expectOverflow(document.body, '') }) it('compensates both vanished scrollbars logically and clears them on release', () => { @@ -67,9 +75,11 @@ describe('lockScroll', () => { mockComputedStyleOnce({ paddingInlineEnd: '7px', paddingBlockEnd: '9px' }) const release = lockScroll() - expect(document.body.style.overflow).toBe('hidden') + expectOverflow(document.body, 'hidden') release() + // The shorthand, not the axes: this test declared the shorthand, and + // that's the inline state restore must hand back. expect(document.body.style.overflow).toBe('auto') expect(document.body.style.paddingInlineEnd).toBe('7px') expect(document.body.style.paddingBlockEnd).toBe('9px') @@ -117,16 +127,33 @@ describe('lockScroll', () => { document.body.append(container) const releaseContainer = lockScroll(container) - expect(container.style.overflow).toBe('hidden') - expect(document.body.style.overflow).toBe('') + expectOverflow(container, 'hidden') + expectOverflow(document.body, '') const releaseBody = lockScroll() releaseContainer() - expect(container.style.overflow).toBe('') - expect(document.body.style.overflow).toBe('hidden') + expectOverflow(container, '') + expectOverflow(document.body, 'hidden') releaseBody() - expect(document.body.style.overflow).toBe('') + expectOverflow(document.body, '') + container.remove() + }) + + it('restores a container that declares its scrolling on one axis only', () => { + // The `overflow` shorthand serializes back to '' unless both longhands + // are set, so a one-axis container has to be saved and restored per axis + // or release removes the consumer's own declaration. + const container = document.createElement('div') + container.style.overflowY = 'auto' + document.body.append(container) + + const release = lockScroll(container) + expectOverflow(container, 'hidden') + + release() + expect(container.style.overflowY).toBe('auto') + container.remove() }) }) diff --git a/packages/native/dialog/SPEC.md b/packages/native/dialog/SPEC.md index c43a55a..71d77f2 100644 --- a/packages/native/dialog/SPEC.md +++ b/packages/native/dialog/SPEC.md @@ -47,6 +47,10 @@ Native-specific notes on top of the core contract: same role Escape plays on the web — while the core default stays `false`; the binding seeds the substrate default into the machine config at build time. Opt out with `closeOnBack={false}`. + The core's Forward half (`forwardNavigate`, which reopens a Back-closed + dialog on the web) has no counterpart here and stays unwired: the platform + offers a Back gesture but no Forward one, and the app's own back stack is + the navigator's to replay, not a dialog's. - **Outside press** is a press on the Backdrop. The Viewport defaults to `pointerEvents="box-none"`, so a press on the empty area around the window falls through to the Backdrop behind it — same net contract as the web's @@ -75,6 +79,7 @@ option as a prop. | `role` | `'dialog' \| 'alertdialog'` | `'dialog'` | The dialog flavor (see core spec for alert defaults). | | `modal` | `boolean` | `true` | Modality; carried to assistive tech. | | `closeOnEscape` | `boolean` | `true` | Kept for cross-substrate parity; no Escape key on touch. | +| `escapeScope` | `'layer' \| 'stack'` | `'layer'` | Same parity: with no Escape on touch, nothing reads it here. | | `closeOnInteractOutside` | `boolean` | varies | Whether a Backdrop press dismisses. | | `closeOnBack` | `boolean` | `true` | Whether the hardware Back press dismisses. Native default diverges from the core's `false`. | | `onInteractOutside` | `(event?) => void` | — | Outside-press report; `preventDefault()` vetoes. | diff --git a/packages/react/dialog/SPEC.md b/packages/react/dialog/SPEC.md index eaf2f99..1b39064 100644 --- a/packages/react/dialog/SPEC.md +++ b/packages/react/dialog/SPEC.md @@ -73,6 +73,16 @@ React-specific notes on top of the core contract: dialog closed any other way consumes its entry, leaving nothing to swallow a later Back; an entry buried under in-app navigation while the dialog is open is left alone (Back then both navigates and closes the dialog). + The entry a Back press spends survives in the forward stack, so the + browser's Forward reopens the dialog it closed (`onForwardNavigation` + fires first; `preventDefault()` vetoes, per the core contract). Reopening + through the trigger instead plants a fresh entry — the browser truncates + the spent one, exactly like navigating after a Back. Two web-mechanics + caveat: a controlled dialog's Back-close is completed by the consumer rather + than by the press itself, so its entry is consumed and Forward has nothing to + re-enter. A nested dialog unmounted along with the parent it was opened from + does come back, and so does one whose page reloaded in between — the entry + remembers the dialog's place in the stack, not the instance that planted it. - Everything ships headless, per the core contract's [Internals](../../core/dialog/SPEC.md#internals). @@ -83,23 +93,24 @@ React-specific notes on top of the core contract: The root: owns open/close state, renders no DOM. Accepts the core `DialogOptions`. -| Prop | Type | Default | Description | -| ------------------------ | --------------------------- | ----------------------------------------- | --------------------------------------------------------------------------------------------------------------------- | -| `open` | `boolean` | — | Controlled open state — the dialog follows it alone. Back to `undefined` hands the state over, uncontrolled in place. | -| `defaultOpen` | `boolean` | `false` | Initial open state for the uncontrolled dialog. | -| `onOpenChange` | `(open: boolean) => void` | — | Fired on every open/close transition with the new value. | -| `modal` | `boolean` | `true` | `aria-modal`, focus trap, scroll lock, backdrop. | -| `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. | -| `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). | -| `onBackNavigation` | `(event?) => void` | — | Fired before a back-navigation dismissal; `preventDefault()` vetoes. | -| `onEscapeKeyDown` | `(event) => void` | — | Fired before an Escape dismissal; `preventDefault()` vetoes. | -| `onInteractOutside` | `(event?) => void` | — | Fired before an outside-press dismissal; `preventDefault()` vetoes. | -| `id` | `string` | auto (`useId`) | Base id for the parts; per-part ids are derived from it. | -| `children` | `ReactNode` | — | The dialog's parts. | +| Prop | Type | Default | Description | +| ------------------------ | --------------------------- | ----------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- | +| `open` | `boolean` | — | Controlled open state — the dialog follows it alone. Back to `undefined` hands the state over, uncontrolled in place. | +| `defaultOpen` | `boolean` | `false` | Initial open state for the uncontrolled dialog. | +| `onOpenChange` | `(open: boolean) => void` | — | Fired on every open/close transition with the new value. | +| `modal` | `boolean` | `true` | `aria-modal`, focus trap, scroll lock, backdrop. | +| `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. | +| `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. | +| `onForwardNavigation` | `(event?) => void` | — | Fired before a forward-navigation reopen; `preventDefault()` vetoes. | +| `onEscapeKeyDown` | `(event) => void` | — | Fired before an Escape dismissal; `preventDefault()` vetoes. | +| `onInteractOutside` | `(event?) => void` | — | Fired before an outside-press dismissal; `preventDefault()` vetoes. | +| `id` | `string` | auto (`useId`) | Base id for the parts; per-part ids are derived from it. | +| `children` | `ReactNode` | — | The dialog's parts. | ### `Dialog.Trigger` diff --git a/packages/react/dialog/src/dialog.tsx b/packages/react/dialog/src/dialog.tsx index 1794860..18d1136 100644 --- a/packages/react/dialog/src/dialog.tsx +++ b/packages/react/dialog/src/dialog.tsx @@ -23,6 +23,7 @@ import { guardBackNavigation, openDialogLayer, startExitWindow, + type BackNavigationGuard, } from '@dunky.dev/dom-dialog' import { mergeProps, normalize } from '@dunky.dev/react-state-machine' import { DialogContext, useDialogContext } from './context' @@ -53,14 +54,29 @@ export const Dialog: ((props: DialogProps) => ReactNode) & Parts = ({ children, apiRef.current = api // The guard lives on the root — it concerns the dialog's openness, not any - // rendered part. + // rendered part. It spans more than the open state, so it outlives this + // effect: a Back-close leaves the registration parked for the Forward that + // may reopen it, and only an unmount ends the episode outright. + const guardRef = useRef(null) + useEffect(() => { - if (!api.open || !machine.context.closeOnBack) return - return guardBackNavigation({ + if (!machine.context.closeOnBack) return + guardRef.current ??= guardBackNavigation({ backNavigate: () => apiRef.current.backNavigate(), + forwardNavigate: () => apiRef.current.forwardNavigate(), isOpen: () => machine.matches('open'), + depth, }) - }, [api.open, machine]) + guardRef.current.sync(api.open) + }, [api.open, machine, depth]) + + useEffect( + () => () => { + guardRef.current?.release() + guardRef.current = null + }, + [], + ) return ( @@ -199,6 +215,7 @@ export const Content: PartComponent = forwar modal: machine.context.modal, backdrop: () => backdropRef.current, initialFocus: initialFocusRef.current?.current, + dismiss: () => machine.send({ type: 'close' }), }) }, [api.open, machine, depth, backdropRef]) diff --git a/packages/react/dialog/stories/dialog.stories.tsx b/packages/react/dialog/stories/dialog.stories.tsx index 12c7dbb..9b27a00 100644 --- a/packages/react/dialog/stories/dialog.stories.tsx +++ b/packages/react/dialog/stories/dialog.stories.tsx @@ -427,29 +427,98 @@ export const nested: StoryType = { // closeOnBack turns the host's Back into a dismissal: while the dialog is open, // a guard entry sits in the session history, so the browser's Back closes the // dialog instead of leaving the page — what mobile users expect from a -// full-screen overlay. The canvas has no browser chrome, so the in-dialog -// button stands in for a real Back press by calling `history.back()`. +// full-screen overlay. The spent entry survives in the forward stack, so the +// browser's Forward reopens what Back closed. The canvas has no browser +// chrome, so the buttons stand in for real presses by calling +// `history.back()` / `history.forward()`. export const closeOnBack: StoryType = { render: () => ( - - Open dialog - - - - - - Rename board - - The browser's Back closes this dialog instead of navigating away. Press Back — or - the button below, which stands in for it here — and the dialog dismisses while the - page stays put. - -
- -
-
-
-
-
+ <> + + Open dialog + + + + + + Rename board + + The browser's Back closes this dialog instead of navigating away. Press Back — + or the button below, which stands in for it here — and the dialog dismisses while + the page stays put. Forward, from the canvas, reopens it. + +
+ +
+
+
+
+
{' '} + + + ), +} + +// A stack of guards: every open layer plants its own history entry, so Back +// unwinds the stack one layer per press and Forward re-enters it one layer per +// press. Uncontrolled on purpose — a controlled dialog's Back-close is +// completed by the consumer, so its entry is consumed and Forward has nothing +// to re-enter (the `nested` story above is the controlled shape). +// +// Two sequences worth walking, with the in-dialog buttons or the canvas ones +// (the canvas is inert while any modal layer is open): +// +// 1. Both open -> Back closes the inner only -> Forward reopens it. The outer +// never moves. +// 2. Back, Back closes both -> Forward reopens the outer -> Forward again +// reopens the inner. Closing the outer unmounted the inner along with it, +// so the one that comes back is a different machine; it recognizes the +// entry as its own ground by its place in the stack. +const HistoryButtons = () => ( +
+ + +
+) + +export const nestedCloseOnBack: StoryType = { + render: () => ( + <> + + Open outer + + + + + + Outer dialog + + Two guard entries while both layers are open. Back closes the topmost one first. + + + Open inner + + + + + + Inner dialog + + Back closes this layer and leaves the outer alone; Forward brings it back, + guarded again. + + + + + + + + + + + {' '} + {' '} + + ), } diff --git a/packages/react/dialog/tests/dialog.test.tsx b/packages/react/dialog/tests/dialog.test.tsx index d2fd82a..bac6a27 100644 --- a/packages/react/dialog/tests/dialog.test.tsx +++ b/packages/react/dialog/tests/dialog.test.tsx @@ -414,15 +414,15 @@ describe('Dialog', () => { it('locks body scroll while a modal dialog is open', () => { render() openDialog() - expect(document.body.style.overflow).toBe('hidden') + expect(document.body.style.overflowY).toBe('hidden') act(pressEscape) - expect(document.body.style.overflow).not.toBe('hidden') + expect(document.body.style.overflowY).not.toBe('hidden') }) it('does not lock scroll when modal=false', () => { render() - expect(document.body.style.overflow).not.toBe('hidden') + expect(document.body.style.overflowY).not.toBe('hidden') }) it('locks the portal container, not the body, when scoped', () => { @@ -437,11 +437,11 @@ describe('Dialog', () => {
, ) - expect(panel.style.overflow).toBe('hidden') - expect(document.body.style.overflow).not.toBe('hidden') + expect(panel.style.overflowY).toBe('hidden') + expect(document.body.style.overflowY).not.toBe('hidden') act(pressEscape) - expect(panel.style.overflow).not.toBe('hidden') + expect(panel.style.overflowY).not.toBe('hidden') panel.remove() }) }) @@ -486,6 +486,191 @@ describe('Dialog', () => { render() expect(window.history.state).toEqual(before) }) + + it('the browser Forward reopens what Back closed, guarded again', async () => { + render() + + const pop = nextPop() + await act(async () => { + window.history.back() + await pop + }) + expect(screen.queryByRole('dialog')).toBeNull() + + const reenter = nextPop() + await act(async () => { + window.history.forward() + await reenter + }) + expect(screen.queryByRole('dialog')).not.toBeNull() + + // The reopened dialog is guarded again: the next Back closes it. + const unwind = nextPop() + await act(async () => { + window.history.back() + await unwind + }) + expect(screen.queryByRole('dialog')).toBeNull() + }) + + it('Forward does not reopen a dialog closed any other way', async () => { + render() + const consume = nextPop() // the released guard consumes its entry + act(pressEscape) + await act(async () => { + await consume + }) + + const reenter = nextPop() + await act(async () => { + window.history.forward() + await reenter + }) + expect(screen.queryByRole('dialog')).toBeNull() + }) + + it('onForwardNavigation preventDefault declines the reopen', async () => { + const { unmount } = render( + event?.preventDefault?.()} + />, + ) + const pop = nextPop() + await act(async () => { + window.history.back() + await pop + }) + + const reenter = nextPop() + await act(async () => { + window.history.forward() + await reenter + }) + expect(screen.queryByRole('dialog')).toBeNull() + + // The decline left the still-watched entry current; unmounting consumes + // it — settle that traversal here, not in the next test. + const consume = nextPop() + unmount() + await act(async () => { + await consume + }) + }) + + // The nested round-trip: closing the outer takes the inner's whole + // registration with it (unmounted with the content that held it), so the + // inner that comes back with the outer is a different machine. It reopens + // anyway — the entry it lost is still its own ground. + const NestedGuards = () => ( + + + + + + open inner + + + + + + + + + + + ) + + const layers = (): string => + `${screen.queryByLabelText('outer') ? 'O' : '-'}${screen.queryByLabelText('inner') ? 'I' : '-'}` + + it('Back unwinds a nested stack one layer per press and Forward restores it the same way', async () => { + const { unmount } = render() + act(() => screen.getByText('open inner').click()) + expect(layers()).toBe('OI') + + const traverse = async (go: () => void): Promise => { + const pop = nextPop() + await act(async () => { + go() + await pop + }) + } + + await traverse(() => window.history.back()) + expect(layers()).toBe('O-') + await traverse(() => window.history.back()) + expect(layers()).toBe('--') + + await traverse(() => window.history.forward()) + expect(layers()).toBe('O-') + await traverse(() => window.history.forward()) + expect(layers()).toBe('OI') + + // Both layers are armed again; unmounting frees their entries one + // traversal at a time — settle both pops here, not in the next test. + const consume = nextPop() + unmount() + await act(async () => { + await consume + await nextPop() + }) + }) + + // Both layers guarded and closed in one commit — a "close all" affordance, + // or a route change that takes the whole stack with it. + const GuardedStack = ({ open }: { open: boolean }) => ( + + + + + + + + + + + + + + + + ) + + it('closing a whole stack at once leaves no entry to swallow a later Back', async () => { + const before: unknown = window.history.state + const { rerender } = render() + expect(window.history.state).not.toEqual(before) + + const consume = nextPop() // the chain spends the entries one pop at a time + rerender() + await act(async () => { + await consume + await nextPop() + }) + expect(window.history.state).toEqual(before) + }) + + it('reopening through the trigger plants a fresh guard, truncating the spent entry', async () => { + render() + const pop = nextPop() + await act(async () => { + window.history.back() + await pop + }) + expect(screen.queryByRole('dialog')).toBeNull() + + openDialog() + expect(screen.queryByRole('dialog')).not.toBeNull() + + const unwind = nextPop() + await act(async () => { + window.history.back() + await unwind + }) + expect(screen.queryByRole('dialog')).toBeNull() + }) }) describe('exit animation', () => { @@ -577,6 +762,33 @@ describe('Dialog', () => { expect(screen.queryByText('Outer')).toBeNull() }) + it('a stack-scoped Escape on the topmost dialog unwinds every layer', () => { + render( + + + + + Outer + + + + + Inner + + + + + + + + , + ) + + act(pressEscape) + expect(screen.queryByText('Inner')).toBeNull() + expect(screen.queryByText('Outer')).toBeNull() + }) + it('hides the dialog beneath the topmost from assistive tech and makes it inert', () => { render() const outer = screen.getByTestId('outer-viewport') @@ -626,7 +838,7 @@ describe('Dialog', () => { rerender() expect(screen.queryByText('Outer')).toBeNull() expect(screen.queryByText('Inner')).toBeNull() - expect(document.body.style.overflow).not.toBe('hidden') + expect(document.body.style.overflowY).not.toBe('hidden') expect(container.hasAttribute('aria-hidden')).toBe(false) expect(container.hasAttribute('inert')).toBe(false) }) diff --git a/packages/react/hooks/use-scroll-lock/tests/use-scroll-lock.test.ts b/packages/react/hooks/use-scroll-lock/tests/use-scroll-lock.test.ts index 858cb24..41e7b5a 100644 --- a/packages/react/hooks/use-scroll-lock/tests/use-scroll-lock.test.ts +++ b/packages/react/hooks/use-scroll-lock/tests/use-scroll-lock.test.ts @@ -8,21 +8,21 @@ import { useScrollLock } from '@dunky.dev/react-use-scroll-lock' describe('useScrollLock', () => { it('locks body scroll while mounted and releases on unmount', () => { const { unmount } = renderHook(() => useScrollLock()) - expect(document.body.style.overflow).toBe('hidden') + expect(document.body.style.overflowY).toBe('hidden') unmount() - expect(document.body.style.overflow).toBe('') + expect(document.body.style.overflowY).toBe('') }) it('does not lock when locked=false', () => { const { unmount } = renderHook(() => useScrollLock(false)) - expect(document.body.style.overflow).toBe('') + expect(document.body.style.overflowY).toBe('') unmount() }) it('locks nothing when target is null — a target not yet resolved', () => { const { unmount } = renderHook(() => useScrollLock(true, null)) - expect(document.body.style.overflow).toBe('') + expect(document.body.style.overflowY).toBe('') unmount() }) }) diff --git a/packages/solid/dialog/SPEC.md b/packages/solid/dialog/SPEC.md index f8f6c1b..d9f9200 100644 --- a/packages/solid/dialog/SPEC.md +++ b/packages/solid/dialog/SPEC.md @@ -78,6 +78,16 @@ Solid-specific notes on top of the core contract: dialog closed any other way consumes its entry, leaving nothing to swallow a later Back; an entry buried under in-app navigation while the dialog is open is left alone (Back then both navigates and closes the dialog). + The entry a Back press spends survives in the forward stack, so the + browser's Forward reopens the dialog it closed (`onForwardNavigation` + fires first; `preventDefault()` vetoes, per the core contract). Reopening + through the trigger instead plants a fresh entry — the browser truncates + the spent one, exactly like navigating after a Back. Two web-mechanics + caveat: a controlled dialog's Back-close is completed by the consumer rather + than by the press itself, so its entry is consumed and Forward has nothing to + re-enter. A nested dialog unmounted along with the parent it was opened from + does come back, and so does one whose page reloaded in between — the entry + remembers the dialog's place in the stack, not the instance that planted it. - Everything ships headless, per the core contract's [Internals](../../core/dialog/SPEC.md#internals). @@ -88,23 +98,24 @@ Solid-specific notes on top of the core contract: The root: owns open/close state, renders no DOM. Accepts the core `DialogOptions`. -| Prop | Type | Default | Description | -| ------------------------ | --------------------------- | ----------------------------------------- | --------------------------------------------------------------------------------------------------------------------- | -| `open` | `boolean` | — | Controlled open state — the dialog follows it alone. Back to `undefined` hands the state over, uncontrolled in place. | -| `defaultOpen` | `boolean` | `false` | Initial open state for the uncontrolled dialog. | -| `onOpenChange` | `(open: boolean) => void` | — | Fired on every open/close transition with the new value. | -| `modal` | `boolean` | `true` | `aria-modal`, focus trap, scroll lock, backdrop. | -| `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. | -| `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). | -| `onBackNavigation` | `(event?) => void` | — | Fired before a back-navigation dismissal; `preventDefault()` vetoes. | -| `onEscapeKeyDown` | `(event) => void` | — | Fired before an Escape dismissal; `preventDefault()` vetoes. | -| `onInteractOutside` | `(event?) => void` | — | Fired before an outside-press dismissal; `preventDefault()` vetoes. | -| `id` | `string` | auto (`createUniqueId`) | Base id for the parts; per-part ids are derived from it. | -| `children` | `JSX.Element` | — | The dialog's parts. | +| Prop | Type | Default | Description | +| ------------------------ | --------------------------- | ----------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- | +| `open` | `boolean` | — | Controlled open state — the dialog follows it alone. Back to `undefined` hands the state over, uncontrolled in place. | +| `defaultOpen` | `boolean` | `false` | Initial open state for the uncontrolled dialog. | +| `onOpenChange` | `(open: boolean) => void` | — | Fired on every open/close transition with the new value. | +| `modal` | `boolean` | `true` | `aria-modal`, focus trap, scroll lock, backdrop. | +| `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. | +| `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. | +| `onForwardNavigation` | `(event?) => void` | — | Fired before a forward-navigation reopen; `preventDefault()` vetoes. | +| `onEscapeKeyDown` | `(event) => void` | — | Fired before an Escape dismissal; `preventDefault()` vetoes. | +| `onInteractOutside` | `(event?) => void` | — | Fired before an outside-press dismissal; `preventDefault()` vetoes. | +| `id` | `string` | auto (`createUniqueId`) | Base id for the parts; per-part ids are derived from it. | +| `children` | `JSX.Element` | — | The dialog's parts. | ### `Dialog.Trigger` diff --git a/packages/solid/dialog/src/dialog.tsx b/packages/solid/dialog/src/dialog.tsx index 45b2269..c6c982b 100644 --- a/packages/solid/dialog/src/dialog.tsx +++ b/packages/solid/dialog/src/dialog.tsx @@ -1,6 +1,7 @@ import { createEffect, omit, + onCleanup, onSettled, untrack, useContext, @@ -20,6 +21,7 @@ import { guardBackNavigation, openDialogLayer, startExitWindow, + type BackNavigationGuard, } from '@dunky.dev/dom-dialog' import { mergeProps, normalize } from '@dunky.dev/solid-state-machine' import { DialogContext, useDialogContext } from './context' @@ -53,18 +55,30 @@ export const Dialog: Component & Parts = props => { const backdropRef: { current: HTMLDivElement | null } = { current: null } // The guard lives on the root — it concerns the dialog's openness, not any - // rendered part. + // rendered part. It spans more than the open state, so it can't be this + // effect's cleanup: a Back-close leaves the registration parked for the + // Forward that may reopen it, and only disposal ends the episode outright. + let guard: BackNavigationGuard | null = null + createEffect( () => api.open, open => { - if (!open || !machine.context.closeOnBack) return - return guardBackNavigation({ + if (!machine.context.closeOnBack) return + guard ??= guardBackNavigation({ backNavigate: () => untrack(() => api.backNavigate()), + forwardNavigate: () => untrack(() => api.forwardNavigate()), isOpen: () => machine.matches('open'), + depth, }) + guard.sync(open) }, ) + onCleanup(() => { + guard?.release() + guard = null + }) + return ( null, backdropRef }}> {props.children} @@ -222,6 +236,7 @@ export const Content: Component = props => { modal: machine.context.modal, backdrop: () => backdropRef.current, initialFocus: untrack(() => resolveInitialFocus(props.initialFocus)), + dismiss: () => machine.send({ type: 'close' }), }) }, ) diff --git a/packages/solid/dialog/stories/dialog.stories.tsx b/packages/solid/dialog/stories/dialog.stories.tsx index 2fea7aa..1053549 100644 --- a/packages/solid/dialog/stories/dialog.stories.tsx +++ b/packages/solid/dialog/stories/dialog.stories.tsx @@ -430,29 +430,98 @@ export const nested: StoryType = { // closeOnBack turns the host's Back into a dismissal: while the dialog is open, // a guard entry sits in the session history, so the browser's Back closes the // dialog instead of leaving the page — what mobile users expect from a -// full-screen overlay. The canvas has no browser chrome, so the in-dialog -// button stands in for a real Back press by calling `history.back()`. +// full-screen overlay. The spent entry survives in the forward stack, so the +// browser's Forward reopens what Back closed. The canvas has no browser +// chrome, so the buttons stand in for real presses by calling +// `history.back()` / `history.forward()`. export const closeOnBack: StoryType = { render: () => ( - - Open dialog - - - - - - Rename board - - The browser's Back closes this dialog instead of navigating away. Press Back — or the - button below, which stands in for it here — and the dialog dismisses while the page - stays put. - -
- -
-
-
-
-
+ <> + + Open dialog + + + + + + Rename board + + The browser's Back closes this dialog instead of navigating away. Press Back — or + the button below, which stands in for it here — and the dialog dismisses while the + page stays put. Forward, from the canvas, reopens it. + +
+ +
+
+
+
+
{' '} + + + ), +} + +// A stack of guards: every open layer plants its own history entry, so Back +// unwinds the stack one layer per press and Forward re-enters it one layer per +// press. Uncontrolled on purpose — a controlled dialog's Back-close is +// completed by the consumer, so its entry is consumed and Forward has nothing +// to re-enter (the `nested` story above is the controlled shape). +// +// Two sequences worth walking, with the in-dialog buttons or the canvas ones +// (the canvas is inert while any modal layer is open): +// +// 1. Both open -> Back closes the inner only -> Forward reopens it. The outer +// never moves. +// 2. Back, Back closes both -> Forward reopens the outer -> Forward again +// reopens the inner. Closing the outer unmounted the inner along with it, +// so the one that comes back is a different machine; it recognizes the +// entry as its own ground by its place in the stack. +const HistoryButtons = () => ( +
+ + +
+) + +export const nestedCloseOnBack: StoryType = { + render: () => ( + <> + + Open outer + + + + + + Outer dialog + + Two guard entries while both layers are open. Back closes the topmost one first. + + + Open inner + + + + + + Inner dialog + + Back closes this layer and leaves the outer alone; Forward brings it back, + guarded again. + + + + + + + + + + + {' '} + {' '} + + ), } diff --git a/packages/solid/dialog/tests/dialog.test.tsx b/packages/solid/dialog/tests/dialog.test.tsx index 6477ad9..5d0a7f3 100644 --- a/packages/solid/dialog/tests/dialog.test.tsx +++ b/packages/solid/dialog/tests/dialog.test.tsx @@ -434,15 +434,15 @@ describe('Dialog', () => { it('locks body scroll while a modal dialog is open', () => { render(() => ) openDialog() - expect(document.body.style.overflow).toBe('hidden') + expect(document.body.style.overflowY).toBe('hidden') pressEscape() - expect(document.body.style.overflow).not.toBe('hidden') + expect(document.body.style.overflowY).not.toBe('hidden') }) it('does not lock scroll when modal=false', () => { render(() => ) - expect(document.body.style.overflow).not.toBe('hidden') + expect(document.body.style.overflowY).not.toBe('hidden') }) it('locks the portal container, not the body, when scoped', () => { @@ -457,11 +457,11 @@ describe('Dialog', () => {
)) - expect(panel.style.overflow).toBe('hidden') - expect(document.body.style.overflow).not.toBe('hidden') + expect(panel.style.overflowY).toBe('hidden') + expect(document.body.style.overflowY).not.toBe('hidden') pressEscape() - expect(panel.style.overflow).not.toBe('hidden') + expect(panel.style.overflowY).not.toBe('hidden') panel.remove() }) }) @@ -505,6 +505,168 @@ describe('Dialog', () => { flush() expect(window.history.state).toEqual(before) }) + + // The traversal, then the commit it caused. + const traverse = async (go: () => void): Promise => { + const pop = nextPop() + go() + await pop + flush() + } + + it('the browser Forward reopens what Back closed, guarded again', async () => { + render(() => ) + flush() + + await traverse(() => window.history.back()) + expect(screen.queryByRole('dialog')).toBeNull() + + await traverse(() => window.history.forward()) + expect(screen.queryByRole('dialog')).not.toBeNull() + + // The reopened dialog is guarded again: the next Back closes it. + await traverse(() => window.history.back()) + expect(screen.queryByRole('dialog')).toBeNull() + }) + + it('Forward does not reopen a dialog closed any other way', async () => { + render(() => ) + flush() + + const consume = nextPop() // the released guard consumes its entry + pressEscape() + await consume + + await traverse(() => window.history.forward()) + expect(screen.queryByRole('dialog')).toBeNull() + }) + + it('onForwardNavigation preventDefault declines the reopen', async () => { + render(() => ( + event?.preventDefault?.()} + /> + )) + flush() + + await traverse(() => window.history.back()) + await traverse(() => window.history.forward()) + expect(screen.queryByRole('dialog')).toBeNull() + + // The decline left the still-watched entry current; disposing consumes + // it — settle that traversal here, not in the next test. + const consume = nextPop() + cleanup() + await consume + }) + + // The nested round-trip: closing the outer takes the inner's whole + // registration with it (unmounted with the content that held it), so the + // inner that comes back with the outer is a different machine. It reopens + // anyway — the entry it lost is still its own ground. + const NestedGuards = () => ( + + + + + + open inner + + + + + + + + + + + ) + + const layers = (): string => + `${screen.queryByLabelText('outer') ? 'O' : '-'}${screen.queryByLabelText('inner') ? 'I' : '-'}` + + it('Back unwinds a nested stack one layer per press and Forward restores it the same way', async () => { + render(() => ) + flush() + press(screen.getByText('open inner')) + expect(layers()).toBe('OI') + + const traverse = async (go: () => void): Promise => { + const pop = nextPop() + go() + await pop + flush() + } + + await traverse(() => window.history.back()) + expect(layers()).toBe('O-') + await traverse(() => window.history.back()) + expect(layers()).toBe('--') + + await traverse(() => window.history.forward()) + expect(layers()).toBe('O-') + await traverse(() => window.history.forward()) + expect(layers()).toBe('OI') + + // Both layers are armed again; disposing frees their entries one + // traversal at a time — settle both pops here, not in the next test. + const consume = nextPop() + cleanup() + await consume + await nextPop() + }) + + // Both layers guarded and closed in one commit — a "close all" affordance, + // or a route change that takes the whole stack with it. + const GuardedStack = (props: { open: boolean }) => ( + + + + + + + + + + + + + + + + ) + + it('closing a whole stack at once leaves no entry to swallow a later Back', async () => { + const before: unknown = window.history.state + const [open, setOpen] = createSignal(true) + render(() => ) + flush() + expect(window.history.state).not.toEqual(before) + + const consume = nextPop() // the chain spends the entries one pop at a time + setOpen(false) + flush() + await consume + await nextPop() + expect(window.history.state).toEqual(before) + }) + + it('reopening through the trigger plants a fresh guard, truncating the spent entry', async () => { + render(() => ) + flush() + + await traverse(() => window.history.back()) + expect(screen.queryByRole('dialog')).toBeNull() + + openDialog() + expect(screen.queryByRole('dialog')).not.toBeNull() + + await traverse(() => window.history.back()) + expect(screen.queryByRole('dialog')).toBeNull() + }) }) describe('exit animation', () => { @@ -595,6 +757,34 @@ describe('Dialog', () => { expect(screen.queryByText('Outer')).toBeNull() }) + it('a stack-scoped Escape on the topmost dialog unwinds every layer', () => { + render(() => ( + + + + + Outer + + + + + Inner + + + + + + + + + )) + flush() + + pressEscape() + expect(screen.queryByText('Inner')).toBeNull() + expect(screen.queryByText('Outer')).toBeNull() + }) + it('hides the dialog beneath the topmost from assistive tech and makes it inert', () => { render(() => ) const outer = screen.getByTestId('outer-viewport') @@ -646,7 +836,7 @@ describe('Dialog', () => { flush() expect(screen.queryByText('Outer')).toBeNull() expect(screen.queryByText('Inner')).toBeNull() - expect(document.body.style.overflow).not.toBe('hidden') + expect(document.body.style.overflowY).not.toBe('hidden') expect(container.hasAttribute('aria-hidden')).toBe(false) expect(container.hasAttribute('inert')).toBe(false) }) diff --git a/packages/solid/hooks/use-scroll-lock/tests/use-scroll-lock.test.ts b/packages/solid/hooks/use-scroll-lock/tests/use-scroll-lock.test.ts index 4026d0d..b595c9a 100644 --- a/packages/solid/hooks/use-scroll-lock/tests/use-scroll-lock.test.ts +++ b/packages/solid/hooks/use-scroll-lock/tests/use-scroll-lock.test.ts @@ -6,21 +6,21 @@ import { useScrollLock } from '@dunky.dev/solid-use-scroll-lock' describe('useScrollLock', () => { it('locks body scroll while mounted and releases on unmount', () => { const { cleanup } = renderHook(() => useScrollLock()) - expect(document.body.style.overflow).toBe('hidden') + expect(document.body.style.overflowY).toBe('hidden') cleanup() - expect(document.body.style.overflow).toBe('') + expect(document.body.style.overflowY).toBe('') }) it('does not lock when locked=false', () => { const { cleanup } = renderHook(() => useScrollLock(false)) - expect(document.body.style.overflow).toBe('') + expect(document.body.style.overflowY).toBe('') cleanup() }) it('locks nothing when target is null — a target not yet resolved', () => { const { cleanup } = renderHook(() => useScrollLock(true, () => null)) - expect(document.body.style.overflow).toBe('') + expect(document.body.style.overflowY).toBe('') cleanup() }) }) diff --git a/packages/solid/vitest.config.ts b/packages/solid/vitest.config.ts index b32027c..4584059 100644 --- a/packages/solid/vitest.config.ts +++ b/packages/solid/vitest.config.ts @@ -12,7 +12,6 @@ export default defineConfig({ globals: false, // node by default; DOM tests opt into jsdom per-file via `@vitest-environment`. environment: 'node', - setupFiles: ['../../vitest.setup.ts'], include: ['**/tests/**/*.test.{ts,tsx}'], }, }) diff --git a/scripts/templates/packages/dom/components/__name__/src/effects.ts b/scripts/templates/packages/dom/components/__name__/src/effects.ts index 86b8383..45d0aae 100644 --- a/scripts/templates/packages/dom/components/__name__/src/effects.ts +++ b/scripts/templates/packages/dom/components/__name__/src/effects.ts @@ -13,8 +13,8 @@ type __Name__Effect = [ // Document-level work every DOM host owns, written once. A listener bound to // `document` or `window` — or anything reading the DOM outside a part's own -// element — belongs here rather than in a substrate: React, Solid, and Vue -// differ in how they schedule the effect, not in what it does. +// element — belongs here rather than in a substrate: React and Solid differ in +// how they schedule the effect, not in what it does. // // See @dunky.dev/dom-dialog for a worked example (the Escape listener, the // open/exit sequences, the outside-press gating). diff --git a/vitest.config.ts b/vitest.config.ts index 6ddcbe8..9f786a4 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -11,7 +11,6 @@ export default defineConfig({ name: 'default', globals: false, environment: 'node', - setupFiles: ['./vitest.setup.ts'], // scripts/templates holds __name__-tokenized stubs (not runnable), // .worktrees/.claude hold local checkouts, packages/native runs on // jest-expo — see packages/native/jest.config.cjs. diff --git a/vitest.setup.ts b/vitest.setup.ts deleted file mode 100644 index 0d5a557..0000000 --- a/vitest.setup.ts +++ /dev/null @@ -1,19 +0,0 @@ -// jsdom doesn't implement Element.checkVisibility() — the platform's -// rendered-element check, which @dunky.dev/dom-focus-trap relies on — so every -// suite that tabs through a trap needs this shim. It reproduces the CSS -// checks: jsdom's UA stylesheet maps the `hidden` attribute to -// `display: none`, and jsdom does no layout, so computed values only reflect -// declared styles. `visibility` is inherited, so the element's own computed -// value suffices; `display` is not, so ancestors are walked. The `Element` -// guard keeps this a no-op for node-environment test files. -if (typeof Element !== 'undefined' && typeof Element.prototype.checkVisibility !== 'function') { - Element.prototype.checkVisibility = function (this: Element): boolean { - const style = getComputedStyle(this) - if (style.visibility === 'hidden' || style.visibility === 'collapse') return false - if (style.display === 'none') return false - for (let ancestor = this.parentElement; ancestor; ancestor = ancestor.parentElement) { - if (getComputedStyle(ancestor).display === 'none') return false - } - return true - } -}