diff --git a/.changeset/dialog-forward-reopens.md b/.changeset/dialog-forward-reopens.md new file mode 100644 index 0000000..19540c9 --- /dev/null +++ b/.changeset/dialog-forward-reopens.md @@ -0,0 +1,48 @@ +--- +'@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?.() + }} +> +``` + +Under the hood, `interceptBackNavigation(onBack, onForward?)` grew the +optional second callback: 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 no `onForward` +behaves exactly as before. + +`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. + +Web-mechanics caveats, spec'd in the navigation util and both DOM bindings: +a controlled dialog's Back-close is completed by the consumer rather than +the press, so its entry is consumed and Forward has nothing to re-enter; +and the Forward watch lives in script, so it doesn't survive a reload. 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/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..fbbf474 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 @@ -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/machine.ts b/packages/core/dialog/src/machine.ts index ad27a49..7130b78 100644 --- a/packages/core/dialog/src/machine.ts +++ b/packages/core/dialog/src/machine.ts @@ -64,6 +64,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 +96,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..2e0c910 100644 --- a/packages/core/dialog/src/types.ts +++ b/packages/core/dialog/src/types.ts @@ -59,13 +59,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 +82,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 } /** @@ -108,8 +112,9 @@ export interface DialogOptions extends DialogCallbacks { 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/dom/components/dialog/SPEC.md b/packages/dom/components/dialog/SPEC.md index 5f5fa9e..ab96640 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 @@ -81,9 +81,19 @@ 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". +Back into a dismissal, and watches the entry a Back press spends so the host's +Forward reopens what it 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 is an episode, not an open state, so it does not fit a single +lifecycle scope: the substrate reports every change through `sync(open)` and +ends it with `release()`. An open edge (re)arms; a close either parks the +registration — the Back press itself closed the dialog, so the spent entry is +still worth watching — or releases it, because a dialog closed any other way +has no way back. That decision is the reason the episode lives here: a +substrate that scoped the guard to "while open" would drop the Forward watch +with the close. ### Outside presses @@ -103,15 +113,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 episode: `sync(open)` per change, `release()` to 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,9 +129,10 @@ 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. @@ -133,3 +144,4 @@ part is the cycle's last stop wherever it renders. | `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. | diff --git a/packages/dom/components/dialog/src/back-navigation.ts b/packages/dom/components/dialog/src/back-navigation.ts index 1948eb7..2b6e7f1 100644 --- a/packages/dom/components/dialog/src/back-navigation.ts +++ b/packages/dom/components/dialog/src/back-navigation.ts @@ -3,19 +3,69 @@ import { interceptBackNavigation } 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 } +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. + */ + 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. */ -export function guardBackNavigation(options: BackNavigationGuardOptions): () => void { - return interceptBackNavigation(() => { - options.backNavigate() - return !options.isOpen() - }) +export function guardBackNavigation(options: BackNavigationGuardOptions): BackNavigationGuard { + let releaseIntercept: (() => void) | null = null + let closedByBack = false + + const release = (): void => { + releaseIntercept?.() + releaseIntercept = null + } + + 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 + }, + () => { + options.forwardNavigate() + return options.isOpen() + }, + ) + } else if (closedByBack) { + // The registration stays parked in the util, watching the spent entry. + closedByBack = false + } else { + release() + } + }, + release, + } } 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/tests/dialog.test.ts b/packages/dom/components/dialog/tests/dialog.test.ts index 2fdf3a6..8576ab4 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' @@ -233,6 +234,99 @@ 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, + }) + 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/navigation/SPEC.md b/packages/dom/utils/navigation/SPEC.md index dfeec67..7fd39ea 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 @@ -21,8 +23,25 @@ sheet) instead of leaving the page. - **`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 +- **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 (a parked watcher owns it) or does nothing (its layer closed + for good). +- **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 releases itself inside `onBack` + never parks at all — it tore itself down rather than closing, so there is + nothing to offer a reopen to. +- **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. An entry buried under later in-app navigation is unreachable and left alone — Back then both navigates and closes the layer. - **Release then re-register in the same synchronous turn** nets out to zero @@ -37,28 +56,34 @@ sheet) instead of leaving the page. 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. +layer should reopen. The Forward reopen is a session-lifetime watch for the +same reason — it lives in script, not in the entry. A layer that must +survive reload (or be shareable) 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, onForward?)` | Arms a guard; `onBack` fires when the user pops it and returns whether the layer closed. `onForward` fires when a traversal re-enters the popped entry and returns whether the layer reopened. Returns the release for a layer closed by other means or gone for good. | ## 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 in-flight self-caused pop, and no release still + waiting on its deferred consumption. ## 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. | diff --git a/packages/dom/utils/navigation/src/intercept-back-navigation.ts b/packages/dom/utils/navigation/src/intercept-back-navigation.ts index 093a3b4..e512447 100644 --- a/packages/dom/utils/navigation/src/intercept-back-navigation.ts +++ b/packages/dom/utils/navigation/src/intercept-back-navigation.ts @@ -5,6 +5,7 @@ const STATE_KEY = '@dunky.back' interface BackGuard { id: number onBack: () => boolean + onForward: (() => boolean) | undefined } // One shared registry + one popstate listener across every layer: a Back @@ -14,6 +15,13 @@ interface BackGuard { // a drawer under a sheet) unwind one per press with no cross-layer // bookkeeping. const guards: BackGuard[] = [] +// Guards whose entry a Back press already popped, kept for the way back: the +// popped entry survives in the session's forward stack, and a traversal +// re-entering it is the host's Forward — `onForward` asks the layer to +// reopen. Parked entries always sit above every armed one: parking only ever +// pops topmost entries, and any planted entry truncates the forward stack +// they live in (see plantEntry). +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 @@ -32,16 +40,30 @@ function currentGuardId(): number | undefined { return typeof id === 'number' ? id : undefined } -function isRegistered(id: number): boolean { +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(id: number): void { + parked.length = 0 + history.pushState({ [STATE_KEY]: id }, '') +} + +// The listener detaches only when nothing is left to hear: a parked watcher, +// an in-flight self-caused pop (swallow), or an undecided release +// (pendingReleases) all still need it even with every guard released. function detachWhenIdle(): void { - if (guards.length === 0 && swallow === 0 && pendingReleases === 0) { + if (guards.length === 0 && parked.length === 0 && swallow === 0 && pendingReleases === 0) { window.removeEventListener('popstate', onPopState) } } @@ -53,14 +75,37 @@ 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.id) + } + detachWhenIdle() + return + } + const current = currentGuardId() + // A marked entry with no armed owner is forward residue — ground above + // every armed entry (a plant would have truncated it anywhere else), so + // nothing may unwind here whichever way the traversal ran. A parked owner + // means the host re-entered "layer open" ground: offer every crossed guard + // a reopen, lowest first. A decline — vetoed, or a controlled layer that + // hasn't followed — stays parked, so a later landing offers again. No + // owner at all is a dead entry; nothing to do. + if (current !== undefined && !isArmed(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) + } + } } 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 @@ -68,12 +113,18 @@ function onPopState(): void { // 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) + if (index !== -1) { + guards.splice(index, 1) + // The popped entry lives on in the forward stack: park the guard so + // the host's Forward can reopen the layer. A guard that released + // itself inside `onBack` is gone for good — nothing left to reopen. + if (top.onForward !== undefined) parked.push(top) + } continue } // 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 }, '') + plantEntry(top.id) break } detachWhenIdle() @@ -84,9 +135,17 @@ function onPopState(): void { * 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. + * (for a layer closed by any other means, or gone for good) 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. + * + * With `onForward`, a Back-closed layer keeps a way back: its popped entry + * survives 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. A decline keeps the watch for a later + * landing; the watch ends when the layer releases, when a newly planted + * entry truncates the forward stack, or when a new registration adopts the + * entry. * * 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 @@ -95,22 +154,38 @@ function onPopState(): void { * 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. + * that window removes the race instead of compensating for it. The same + * adoption is how a layer reopened by Forward re-registers on its own spent + * entry without a traversal. */ -export function interceptBackNavigation(onBack: () => boolean): () => void { - const guard: BackGuard = { id: ++nextGuardId, onBack } +export function interceptBackNavigation( + onBack: () => boolean, + onForward?: () => boolean, +): () => void { + const guard: BackGuard = { id: ++nextGuardId, onBack, 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 — the ground now + // belongs to this registration. + const stale = parkedIndex(current) + if (stale !== -1) parked.splice(stale, 1) + history.replaceState({ [STATE_KEY]: guard.id }, '') + } else { + plantEntry(guard.id) + } return () => { - const index = guards.indexOf(guard) - if (index === -1) return // already unwound by the Back press itself - guards.splice(index, 1) + const rest = parked.indexOf(guard) + if (rest !== -1) { + parked.splice(rest, 1) + } else { + const index = guards.indexOf(guard) + if (index === -1) return // already unwound by the Back press itself + guards.splice(index, 1) + } pendingReleases++ queueMicrotask(() => { pendingReleases-- 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..140cb37 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 @@ -132,4 +146,100 @@ 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, 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, lowerForward) + const releaseUpper = interceptBackNavigation(() => true, 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/native/dialog/SPEC.md b/packages/native/dialog/SPEC.md index c43a55a..d44e08c 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 diff --git a/packages/react/dialog/SPEC.md b/packages/react/dialog/SPEC.md index eaf2f99..258099e 100644 --- a/packages/react/dialog/SPEC.md +++ b/packages/react/dialog/SPEC.md @@ -73,6 +73,15 @@ 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 + caveats: 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; and the Forward watch lives in script, so it doesn't + survive a reload (the navigation util's SPEC covers why). - Everything ships headless, per the core contract's [Internals](../../core/dialog/SPEC.md#internals). @@ -83,23 +92,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..befc701 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,15 +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'), }) + guardRef.current.sync(api.open) }, [api.open, machine]) + useEffect( + () => () => { + guardRef.current?.release() + guardRef.current = null + }, + [], + ) + return ( {children} diff --git a/packages/react/dialog/stories/dialog.stories.tsx b/packages/react/dialog/stories/dialog.stories.tsx index 12c7dbb..7d36690 100644 --- a/packages/react/dialog/stories/dialog.stories.tsx +++ b/packages/react/dialog/stories/dialog.stories.tsx @@ -427,29 +427,34 @@ 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. + +
+ +
+ + + + {' '} + + ), } diff --git a/packages/react/dialog/tests/dialog.test.tsx b/packages/react/dialog/tests/dialog.test.tsx index d2fd82a..f326d7a 100644 --- a/packages/react/dialog/tests/dialog.test.tsx +++ b/packages/react/dialog/tests/dialog.test.tsx @@ -486,6 +486,98 @@ 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 + }) + }) + + 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', () => { diff --git a/packages/solid/dialog/SPEC.md b/packages/solid/dialog/SPEC.md index f8f6c1b..b119b33 100644 --- a/packages/solid/dialog/SPEC.md +++ b/packages/solid/dialog/SPEC.md @@ -78,6 +78,15 @@ 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 + caveats: 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; and the Forward watch lives in script, so it doesn't + survive a reload (the navigation util's SPEC covers why). - Everything ships headless, per the core contract's [Internals](../../core/dialog/SPEC.md#internals). @@ -88,23 +97,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..9ed56be 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,29 @@ 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'), }) + guard.sync(open) }, ) + onCleanup(() => { + guard?.release() + guard = null + }) + return ( null, backdropRef }}> {props.children} diff --git a/packages/solid/dialog/stories/dialog.stories.tsx b/packages/solid/dialog/stories/dialog.stories.tsx index 2fea7aa..ea02f64 100644 --- a/packages/solid/dialog/stories/dialog.stories.tsx +++ b/packages/solid/dialog/stories/dialog.stories.tsx @@ -430,29 +430,34 @@ 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. + +
+ +
+ + + + {' '} + + ), } diff --git a/packages/solid/dialog/tests/dialog.test.tsx b/packages/solid/dialog/tests/dialog.test.tsx index 6477ad9..e02f0ff 100644 --- a/packages/solid/dialog/tests/dialog.test.tsx +++ b/packages/solid/dialog/tests/dialog.test.tsx @@ -505,6 +505,76 @@ 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 + }) + + 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', () => { 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).