From 6b51d8de4d1069863a56c7ac5f74cb3c8dfaa20c Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Fri, 14 Aug 2026 22:27:16 +0200 Subject: [PATCH 01/15] feat(dialog): the browser Forward reopens what Back closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closeOnBack becomes symmetric. The guard entry a Back press pops survives in the forward stack, still marking the dialog's open ground: a traversal re-entering it is the host's Forward and reopens the dialog, guarded again for the next Back. One setting gates both directions. - dom-navigation: interceptBackNavigation(onBack, onForward?) — a Back-closed guard parks instead of dropping; a landing on its spent entry offers the reopen and re-arms the guard on the entry in place. Ownership of the landing entry, not traversal direction, decides reopen vs unwind: marked ground with no armed owner is forward residue and never unwinds a layer. - dialog (core): history.forward mirrors history.back, gated by the same closeOnBack, reachable from closed and from the animated closing state (Forward interrupts the exit); forwardNavigate() on the api with a new onForwardNavigation veto callback, per the shared dismissal contract. - react-dialog: one registration spans the whole episode — armed while open, parked through a Back-close, released on any other close or on unmount. Reopening through the trigger plants a fresh entry, truncating the spent one, exactly like navigating after a Back. Co-Authored-By: Claude Fable 5 --- .changeset/dialog-forward-reopens.md | 38 +++++++ packages/core/dialog/SPEC.md | 38 ++++--- packages/core/dialog/src/connect.ts | 35 ++++-- packages/core/dialog/src/machine.ts | 12 ++ packages/core/dialog/src/types.ts | 15 ++- packages/core/dialog/tests/machine.test.ts | 43 +++++++ packages/dom/utils/navigation/SPEC.md | 55 ++++++--- .../src/intercept-back-navigation.ts | 106 +++++++++++++++--- .../tests/intercept-back-navigation.test.ts | 94 ++++++++++++++++ packages/react/dialog/SPEC.md | 44 +++++--- packages/react/dialog/src/dialog.tsx | 54 +++++++-- .../react/dialog/stories/dialog.stories.tsx | 51 +++++---- packages/react/dialog/tests/dialog.test.tsx | 92 +++++++++++++++ 13 files changed, 565 insertions(+), 112 deletions(-) create mode 100644 .changeset/dialog-forward-reopens.md diff --git a/.changeset/dialog-forward-reopens.md b/.changeset/dialog-forward-reopens.md new file mode 100644 index 0000000..9be393f --- /dev/null +++ b/.changeset/dialog-forward-reopens.md @@ -0,0 +1,38 @@ +--- +'@dunky.dev/dom-navigation': minor +'@dunky.dev/dialog': minor +'@dunky.dev/react-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. + +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. Layers that don't pass `onForward` +(the Vue dialog, for now) behave exactly as before. + +Web-mechanics caveats, spec'd in the navigation util and the React dialog: +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/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/utils/navigation/SPEC.md b/packages/dom/utils/navigation/SPEC.md index 8044afe..5b49291 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,23 @@ 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. +- **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,27 +54,33 @@ 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 and no - in-flight self-caused pop. +- 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, and no in-flight self-caused pop. ## 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 a12cf33..c898fd3 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 @@ -27,15 +35,29 @@ 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 } +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: an in-flight // self-caused pop (swallow) still needs it even with every guard released. function detachWhenIdle(): void { - if (guards.length === 0 && swallow === 0) { + if (guards.length === 0 && parked.length === 0 && swallow === 0) { window.removeEventListener('popstate', onPopState) } } @@ -47,24 +69,50 @@ 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 if (top.onBack()) { guards.pop() + // The popped entry lives on in the forward stack: park the guard so + // the host's Forward can reopen the layer. + 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() @@ -75,9 +123,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 @@ -86,22 +142,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) + } queueMicrotask(() => { // Still ours and still current: nobody adopted it and no Back popped // it — consume the entry. The listener stays until the pop lands. 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 64d2432..6ba243a 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 @@ -92,4 +106,84 @@ 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 + }) + + 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/react/dialog/SPEC.md b/packages/react/dialog/SPEC.md index a23d558..686d2a3 100644 --- a/packages/react/dialog/SPEC.md +++ b/packages/react/dialog/SPEC.md @@ -67,6 +67,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). @@ -77,23 +86,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 7f1d39e..8c1f3d7 100644 --- a/packages/react/dialog/src/dialog.tsx +++ b/packages/react/dialog/src/dialog.tsx @@ -53,18 +53,54 @@ export const Dialog: ((props: DialogProps) => ReactNode) & Parts = ({ children, apiRef.current = api // closeOnBack: while open, a guard entry in the session history turns the - // host's Back into a dismissal instead of a navigation. Every decision - // (gate, veto, controlled) lives in the core's backNavigate; this effect - // only wires the web mechanics. It lives on the root — the guard concerns - // the dialog's openness, not any rendered part. + // host's Back into a dismissal instead of a navigation — and the entry a + // Back press pops survives in the forward stack, so Forward reopens what + // Back closed. Every decision (gate, veto, controlled) lives in the core's + // backNavigate/forwardNavigate; this only wires the web mechanics. It lives + // on the root — the guard concerns the dialog's openness, not any rendered + // part. One registration spans the whole episode: armed while open, parked + // in the util through a Back-close (releasing there would end the Forward + // watch), released when the dialog closes any other way — or unmounts, + // whichever phase the registration is in. + const releaseGuardRef = useRef<(() => void) | null>(null) + const closedByBackRef = useRef(false) + useEffect(() => { - if (!api.open || !machine.context.closeOnBack) return - return interceptBackNavigation(() => { - apiRef.current.backNavigate() - return !machine.matches('open') - }) + if (!machine.context.closeOnBack) return + if (api.open) { + // (Re)arm on every open edge. Opened 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, like the browser + // does for any navigation after a Back). + releaseGuardRef.current?.() + releaseGuardRef.current = interceptBackNavigation( + () => { + apiRef.current.backNavigate() + const closed = !machine.matches('open') + closedByBackRef.current = closed + return closed + }, + () => { + apiRef.current.forwardNavigate() + return machine.matches('open') + }, + ) + } else if (closedByBackRef.current) { + closedByBackRef.current = false + } else { + releaseGuardRef.current?.() + releaseGuardRef.current = null + } }, [api.open, machine]) + useEffect( + () => () => { + releaseGuardRef.current?.() + releaseGuardRef.current = null + }, + [], + ) + return ( {children} diff --git a/packages/react/dialog/stories/dialog.stories.tsx b/packages/react/dialog/stories/dialog.stories.tsx index 14f8446..878c0f9 100644 --- a/packages/react/dialog/stories/dialog.stories.tsx +++ b/packages/react/dialog/stories/dialog.stories.tsx @@ -427,29 +427,36 @@ 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()`. +// (The Vue substrate's story stays Back-only for now: its binding doesn't +// wire the Forward reopen yet.) 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 d82dbe3..4186a34 100644 --- a/packages/react/dialog/tests/dialog.test.tsx +++ b/packages/react/dialog/tests/dialog.test.tsx @@ -484,6 +484,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', () => { From 3fe5d545760f4cd48edb0b372f102af8a641ced9 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Sat, 22 Aug 2026 16:57:40 +0200 Subject: [PATCH 02/15] docs: name React and Solid as the substrates, not Vue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vue was the stand-in second framework in the docs while Solid didn't exist. Solid ships now, so the examples name the substrates that are actually here — including the DOM layer's "who schedules the effect" argument, which reads better with two real hosts than with a hypothetical one. Published CHANGELOG entries keep their Vue mention: they are the release record for what shipped, not current docs. Co-Authored-By: Claude Opus 5 (1M context) --- ARCHITECTURE.md | 6 +++--- CONTRIBUTING.md | 2 +- README.md | 2 +- packages/dom/components/dialog/SPEC.md | 2 +- .../packages/dom/components/__name__/src/effects.ts | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) 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/dom/components/dialog/SPEC.md b/packages/dom/components/dialog/SPEC.md index e041bf1..0ae592a 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 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). From e460e053d2ed058413d41d2d844fad404544b060 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Sun, 23 Aug 2026 15:57:53 +0200 Subject: [PATCH 03/15] chore: keep the pre-commit hook alive on template-only commits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit oxlint and oxfmt both ignore `scripts/templates/**`, and both exit non-zero when every file handed to them is ignored rather than treating it as a no-op. A commit touching only scaffold templates therefore failed the hook on "no files to check" — nothing was wrong with the code. lint-staged's config becomes `.lintstagedrc.ts` (auto-discovered, and the same TS-config style as knip/tsdown/vitest) so it can filter those paths out and skip the tasks entirely when nothing checkable is staged. Co-Authored-By: Claude Opus 5 (1M context) --- .lintstagedrc.json | 3 --- .lintstagedrc.ts | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) delete mode 100644 .lintstagedrc.json create mode 100644 .lintstagedrc.ts 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 From 95c6b4aba346ee2c72bfbb7805f02564054a5dc6 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 11:29:38 +0200 Subject: [PATCH 04/15] =?UTF-8?q?feat(overlay):=20below(id)=20=E2=80=94=20?= =?UTF-8?q?the=20layers=20beneath=20one,=20in=20unwinding=20order?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dismissal that reaches past its own layer gets the ones beneath topmost first, by the same rule that decides the topmost, so closing a whole stack at once ends up where closing it one layer at a time would have. Co-Authored-By: Claude Opus 5 (1M context) --- packages/core/utils/overlay/SPEC.md | 13 ++++++++----- packages/core/utils/overlay/src/layer-stack.ts | 16 ++++++++++++++++ .../core/utils/overlay/tests/layer-stack.test.ts | 12 ++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/packages/core/utils/overlay/SPEC.md b/packages/core/utils/overlay/SPEC.md index 49a17bb..1ddbfdf 100644 --- a/packages/core/utils/overlay/SPEC.md +++ b/packages/core/utils/overlay/SPEC.md @@ -31,14 +31,17 @@ 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. +- 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()`, `isTopmost(id)`, `below(id)`. | ## Constraints diff --git a/packages/core/utils/overlay/src/layer-stack.ts b/packages/core/utils/overlay/src/layer-stack.ts index 3033fe9..6443a65 100644 --- a/packages/core/utils/overlay/src/layer-stack.ts +++ b/packages/core/utils/overlay/src/layer-stack.ts @@ -22,6 +22,10 @@ export interface LayerStack { // The topmost layer, or undefined when the stack is empty. topmost: () => T | undefined 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 @@ -59,5 +63,17 @@ export function createLayerStack(): LayerStack { 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..3eb0e7e 100644 --- a/packages/core/utils/overlay/tests/layer-stack.test.ts +++ b/packages/core/utils/overlay/tests/layer-stack.test.ts @@ -33,6 +33,18 @@ describe('createLayerStack', () => { expect(stack.isTopmost('anything')).toBe(false) }) + 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', () => { const a = createLayerStack() const b = createLayerStack() From 6b0b11d37e77c8d4a9aadce3e850df60a4690486 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 11:29:40 +0200 Subject: [PATCH 05/15] feat(dom-overlay): layersBelow(id) and an opt-in Layer.dismiss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DOM half of a stack-scoped dismissal: a layer registers how to close it from above, and the layer that received the intent unwinds the ones beneath through it. A layer that registers no dismiss opts out and stays open — a stack mixing primitives is never closed out from under them. Co-Authored-By: Claude Opus 5 (1M context) --- packages/dom/utils/overlay/SPEC.md | 17 +++++++++-------- packages/dom/utils/overlay/src/index.ts | 2 +- packages/dom/utils/overlay/src/stack.ts | 13 +++++++++++++ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/packages/dom/utils/overlay/SPEC.md b/packages/dom/utils/overlay/SPEC.md index 19992dd..de13083 100644 --- a/packages/dom/utils/overlay/SPEC.md +++ b/packages/dom/utils/overlay/SPEC.md @@ -60,14 +60,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 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..e75874e 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 @@ -66,3 +72,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) +} From a33e1496a5e2440c968002efc493f850ea9db26a Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 11:30:02 +0200 Subject: [PATCH 06/15] feat(browser-navigation): batched release + spent-entry claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes to the interceptor: - fix: a whole stack released in one turn (close-all, an unmounting subtree) now consumes every entry it planted in a single traversal. Before, only the topmost guard's entry was current and got consumed; each entry beneath stayed behind and silently swallowed a later Back. - interceptBackNavigation(onBack, { onForward, claim }): the claim names the layer's ground and is stamped into the entry, outliving the registration (and a reload). watchSpentEntry(claim, reopen) is the way back for a layer whose guard is gone — a nested layer unmounted with the parent that held it. Only a sole claimant answers; a layer that closed (rather than being torn down) gives its ground up, so Forward never undoes a deliberate close. release({ keepClaim: true }) marks the teardown case. Also records in the SPEC why this is built on the History API rather than the Navigation API, and what dissolves once that API is cross-browser. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/back-guard-batched-release.md | 23 ++ .changeset/dialog-forward-reopens.md | 30 ++- packages/dom/utils/navigation/SPEC.md | 78 ++++-- packages/dom/utils/navigation/src/index.ts | 7 +- .../src/intercept-back-navigation.ts | 242 ++++++++++++------ .../tests/intercept-back-navigation.test.ts | 47 +++- 6 files changed, 303 insertions(+), 124 deletions(-) create mode 100644 .changeset/back-guard-batched-release.md diff --git a/.changeset/back-guard-batched-release.md b/.changeset/back-guard-batched-release.md new file mode 100644 index 0000000..39a2a36 --- /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 — the freed run was consumed as one traversal +``` + +Release order doesn't matter (the run is read from the entry order), and an +entry genuinely buried under later in-app navigation is still left alone. diff --git a/.changeset/dialog-forward-reopens.md b/.changeset/dialog-forward-reopens.md index 19540c9..c90ea8b 100644 --- a/.changeset/dialog-forward-reopens.md +++ b/.changeset/dialog-forward-reopens.md @@ -29,11 +29,23 @@ and a controlled dialog only records the intent: > ``` -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. +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 @@ -42,7 +54,7 @@ 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. +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/packages/dom/utils/navigation/SPEC.md b/packages/dom/utils/navigation/SPEC.md index 7fd39ea..6c4b93f 100644 --- a/packages/dom/utils/navigation/SPEC.md +++ b/packages/dom/utils/navigation/SPEC.md @@ -31,19 +31,32 @@ it reopens the layer. 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). + 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 releases itself inside `onBack` - never parks at all — it tore itself down rather than closing, so there is - nothing to offer a reopen to. + 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. An entry buried under later in-app navigation is unreachable and left alone — Back then both navigates and closes the layer. +- **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. @@ -54,18 +67,28 @@ it reopens the layer. ### 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. 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. +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, 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. | +| 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 @@ -74,16 +97,21 @@ itself from it — Back then closes for free and needs no interceptor. - 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. +- 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. ## 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. | -| 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. | +| 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. | +| Consumption is batched per turn, over a snapshot of the entry order | A multi-entry `go` is one traversal and one `popstate`, so a freed run costs the same as a single entry. The snapshot is taken before the first release splices, so the call order of sibling releases can't change the run. | +| 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 e512447..73e399e 100644 --- a/packages/dom/utils/navigation/src/intercept-back-navigation.ts +++ b/packages/dom/utils/navigation/src/intercept-back-navigation.ts @@ -1,37 +1,62 @@ // 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 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). +// 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(). +// Releases whose deferred consumption hasn't run — the listener must outlive +// them, or one release's idle check could detach it under a sibling's pop. let pendingReleases = 0 +// A turn's releases, consumed together: a stack closing at once has only its +// topmost entry current, so one-at-a-time would strand every entry beneath. +// `order` snapshots the entry order before the first release splices. +let batch: Set | null = null +let order: number[] | null = null function currentGuardId(): number | undefined { const state: unknown = history.state @@ -40,6 +65,30 @@ function currentGuardId(): number | undefined { return typeof id === 'number' ? id : undefined } +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 @@ -54,16 +103,48 @@ function parkedIndex(id: number): number { // 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 { +function plantEntry(guard: BackGuard): void { parked.length = 0 - history.pushState({ [STATE_KEY]: id }, '') + history.pushState({ [STATE_KEY]: guard.id, [CLAIM_KEY]: guard.claim }, '') +} + +// Once per turn, after every release in it: consume the freed run of entries — +// current one down, while contiguous — in a single traversal. +function consumeBatch(): void { + const released = batch as Set + const entryOrder = order as number[] + batch = null + order = null + pendingReleases = 0 + + const current = currentGuardId() + // Nothing of ours is current: buried under later navigation, or adopted by a + // same-turn re-register. Unreachable entries are left alone. + if (current === undefined || !released.has(current)) { + detachWhenIdle() + return + } + // A guard that both registered and released inside this turn isn't in the + // snapshot; its own entry is still the one to consume. + const top = entryOrder.indexOf(current) + let count = 1 + for (let index = top - 1; index >= 0 && released.has(entryOrder[index] as number); index--) { + count++ + } + swallow++ + history.go(-count) } -// 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. +// Detach only when nothing is left to hear — parked guards, claim watchers, +// in-flight self-caused pops, and undecided releases all still need it. function detachWhenIdle(): void { - if (guards.length === 0 && parked.length === 0 && swallow === 0 && pendingReleases === 0) { + if ( + guards.length === 0 && + parked.length === 0 && + watchers.length === 0 && + swallow === 0 && + pendingReleases === 0 + ) { window.removeEventListener('popstate', onPopState) } } @@ -75,19 +156,16 @@ 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()) { - plantEntry(top.id) + plantEntry(top) } 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. + // A marked entry with no armed owner 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. if (current !== undefined && !isArmed(current)) { const landed = parkedIndex(current) if (landed !== -1) { @@ -100,6 +178,8 @@ function onPopState(): void { guards.push(guard) } } + } else { + offerToClaimant() } detachWhenIdle() return @@ -115,54 +195,42 @@ function onPopState(): void { const index = guards.indexOf(top) 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. + // 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) } 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. - plantEntry(top.id) + plantEntry(top) break } 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, 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. + * 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. The same - * adoption is how a layer reopened by Forward re-registers on its own spent - * entry without a traversal. + * Consumption is deferred a microtask so a same-turn release + re-register + * adopts the entry in place with zero traversals — 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, - onForward?: () => boolean, -): () => void { - const guard: BackGuard = { id: ++nextGuardId, onBack, onForward } + 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() @@ -172,12 +240,20 @@ export function interceptBackNavigation( // belongs to this registration. const stale = parkedIndex(current) if (stale !== -1) parked.splice(stale, 1) - history.replaceState({ [STATE_KEY]: guard.id }, '') + history.replaceState({ [STATE_KEY]: guard.id, [CLAIM_KEY]: guard.claim }, '') } else { - plantEntry(guard.id) + plantEntry(guard) } - return () => { + return (releaseOptions: ReleaseOptions = {}) => { + if (releaseOptions.keepClaim !== true) abandoned.add(guard.id) + // Snapshot the entry order before this turn's first release splices it. + if (batch === null) { + batch = new Set() + order = [...guards.map(entry => entry.id), ...parked.map(entry => entry.id)] + queueMicrotask(consumeBatch) + } + const rest = parked.indexOf(guard) if (rest !== -1) { parked.splice(rest, 1) @@ -186,17 +262,25 @@ export function interceptBackNavigation( if (index === -1) return // already unwound by the Back press itself guards.splice(index, 1) } + batch.add(guard.id) 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. - if (currentGuardId() === guard.id) { - swallow++ - history.back() - } else { - 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 140cb37..0ba5fd6 100644 --- a/packages/dom/utils/navigation/tests/intercept-back-navigation.test.ts +++ b/packages/dom/utils/navigation/tests/intercept-back-navigation.test.ts @@ -103,6 +103,30 @@ 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 consuming one at a time would leave + // every entry beneath behind, each swallowing a later Back. Release order is + // irrelevant: the run is read from the entry order, not the call order. + 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() // one traversal for the whole run + releaseUpper() + releaseLower() + await pop + expect(history.state).toEqual(before) + + const outerFirst = interceptBackNavigation(() => true) + const innerFirst = interceptBackNavigation(() => true) + const second = nextPop() + outerFirst() + innerFirst() + await second + 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) @@ -150,7 +174,7 @@ describe('interceptBackNavigation', () => { 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) + const release = interceptBackNavigation(onBack, { onForward }) await pressBack() expect(onBack).toHaveBeenCalledTimes(1) @@ -168,7 +192,7 @@ describe('interceptBackNavigation', () => { 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) + const release = interceptBackNavigation(() => true, { onForward }) await pressBack() await pressForward() @@ -183,7 +207,7 @@ describe('interceptBackNavigation', () => { it('release while parked ends the Forward watch', async () => { const onForward = vi.fn(() => true) - const release = interceptBackNavigation(() => true, onForward) + const release = interceptBackNavigation(() => true, { onForward }) await pressBack() release() await new Promise(resolve => queueMicrotask(resolve)) @@ -198,10 +222,13 @@ describe('interceptBackNavigation', () => { 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) + release = interceptBackNavigation( + () => { + release() + return true + }, + { onForward }, + ) await pressBack() await pressForward() // re-enters the spent entry, nobody watching @@ -211,7 +238,7 @@ describe('interceptBackNavigation', () => { it('a newly planted entry ends the Forward watch of the layer before it', async () => { const firstForward = vi.fn(() => true) - interceptBackNavigation(() => true, firstForward) + interceptBackNavigation(() => true, { onForward: firstForward }) await pressBack() // parked, entry in the forward stack const second = vi.fn(() => true) @@ -227,8 +254,8 @@ describe('interceptBackNavigation', () => { 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) + const releaseLower = interceptBackNavigation(() => true, { onForward: lowerForward }) + const releaseUpper = interceptBackNavigation(() => true, { onForward: upperForward }) await pressBack() await pressBack() From ffa4fada7719daa8661adab52c20952f3d8d7559 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 11:30:03 +0200 Subject: [PATCH 07/15] =?UTF-8?q?feat(dialog):=20escapeScope=20option=20?= =?UTF-8?q?=E2=80=94=20one=20layer=20per=20press,=20or=20the=20whole=20sta?= =?UTF-8?q?ck?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SPECs promised escapeScope but nothing implemented it. The core now carries the option ('layer' | 'stack', default 'layer') in context; only the dialog that receives the Escape gates and vetoes it, per the existing stack-closing contract. The SPEC's claim of a stack-scoped Close press is removed — nothing implements it. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/dialog-escape-scope.md | 33 +++++++++++++++++++++++++++++ packages/core/dialog/SPEC.md | 2 +- packages/core/dialog/src/index.ts | 1 + packages/core/dialog/src/machine.ts | 1 + packages/core/dialog/src/types.ts | 8 +++++++ 5 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 .changeset/dialog-escape-scope.md 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/packages/core/dialog/SPEC.md b/packages/core/dialog/SPEC.md index fbbf474..2ca4129 100644 --- a/packages/core/dialog/SPEC.md +++ b/packages/core/dialog/SPEC.md @@ -178,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 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 7130b78..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', diff --git a/packages/core/dialog/src/types.ts b/packages/core/dialog/src/types.ts index 2e0c910..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 @@ -107,6 +110,11 @@ 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 From ea429d920208f3a2497219799eeccd36c17634db Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 11:30:23 +0200 Subject: [PATCH 08/15] feat(dom-dialog): stack-scoped Escape + the guard episode reclaims its ground MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - escapeScope 'stack': an allowed Escape on the topmost dialog dismisses every layer beneath through the dismiss each one registered, top-down. The stack is read before the machine moves (closing releases the layer, taking the answer with it), and nothing unwinds unless this dialog's own Escape was actually allowed — a veto leaves the whole stack standing. - guardBackNavigation claims its ground by nesting depth, so a nested dialog unmounted with its parent reopens on the Forward that re-enters its spent entry. Depth, not id: the auto-generated id doesn't survive the remount. A close surrenders the ground; only a teardown keeps it claimable. Co-Authored-By: Claude Opus 5 (1M context) --- packages/dom/components/dialog/SPEC.md | 78 +++++++++++-------- .../components/dialog/src/back-navigation.ts | 53 ++++++++++--- packages/dom/components/dialog/src/effects.ts | 15 +++- .../dom/components/dialog/src/open-layer.ts | 4 + .../components/dialog/tests/dialog.test.ts | 31 +++++++- 5 files changed, 134 insertions(+), 47 deletions(-) diff --git a/packages/dom/components/dialog/SPEC.md b/packages/dom/components/dialog/SPEC.md index ab96640..a2321b1 100644 --- a/packages/dom/components/dialog/SPEC.md +++ b/packages/dom/components/dialog/SPEC.md @@ -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,20 +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, 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 +`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 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. +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 @@ -113,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)` | 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. | +| 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 @@ -138,10 +152,12 @@ part is the cycle's last stop wherever it renders. ## 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. | -| 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. | +| 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 2b6e7f1..6554e70 100644 --- a/packages/dom/components/dialog/src/back-navigation.ts +++ b/packages/dom/components/dialog/src/back-navigation.ts @@ -1,4 +1,8 @@ -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. */ @@ -7,13 +11,20 @@ export interface BackNavigationGuardOptions { 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. + * 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. */ @@ -29,15 +40,30 @@ export interface BackNavigationGuard { * 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. + * 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): BackNavigationGuard { - let releaseIntercept: (() => void) | null = null + 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?.() + releaseIntercept?.({ keepClaim: true }) releaseIntercept = null + releaseWatch?.() + releaseWatch = null + } + + const reopen = (): boolean => { + options.forwardNavigate() + return options.isOpen() } return { @@ -54,16 +80,21 @@ export function guardBackNavigation(options: BackNavigationGuardOptions): BackNa closedByBack = !options.isOpen() return closedByBack }, - () => { - options.forwardNavigate() - return options.isOpen() - }, + { onForward: reopen, claim }, ) } else if (closedByBack) { - // The registration stays parked in the util, watching the spent entry. + // The registration stays parked in the util, watching the spent entry + // it still owns — a stronger claim than the one below. closedByBack = false } else { - release() + // 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/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 8576ab4..94377e0 100644 --- a/packages/dom/components/dialog/tests/dialog.test.ts +++ b/packages/dom/components/dialog/tests/dialog.test.ts @@ -45,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 } @@ -90,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) @@ -257,6 +283,7 @@ describe('guardBackNavigation', () => { backNavigate: () => void (opened = false), forwardNavigate: () => void (opened = true), isOpen: () => opened, + depth: 1, }) const report = (): void => { if (opened === reported) return From 923325395a1d4f1ac5938f1aaa476d5507b48bcd Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 11:30:24 +0200 Subject: [PATCH 09/15] docs(native-dialog): escapeScope parity row; Forward stays unwired escapeScope joins closeOnEscape as cross-substrate parity props nothing reads on touch, and the SPEC records why the core's Forward half has no counterpart here: the platform offers a Back gesture and no Forward one. Co-Authored-By: Claude Opus 5 (1M context) --- packages/native/dialog/SPEC.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/native/dialog/SPEC.md b/packages/native/dialog/SPEC.md index d44e08c..71d77f2 100644 --- a/packages/native/dialog/SPEC.md +++ b/packages/native/dialog/SPEC.md @@ -79,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. | From 5650f031cdec54599b9f14f4fc9fb13b66ea4304 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 11:30:42 +0200 Subject: [PATCH 10/15] feat(react-dialog): escapeScope, the nested Forward round-trip, and the story MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - passes the layer's dismiss and its depth to the DOM package — lifecycle only; the decisions live below this binding. - new nestedCloseOnBack story: two guarded layers, Back unwinds one per press, Forward restores the same way — including the inner dialog that was unmounted with its parent. - tests: the nested round-trip (OI -> O- -> -- -> O- -> OI), close-all leaves no entry to swallow a later Back, stack-scoped Escape unwinds every layer. Co-Authored-By: Claude Opus 5 (1M context) --- packages/react/dialog/SPEC.md | 9 +- packages/react/dialog/src/dialog.tsx | 4 +- .../react/dialog/stories/dialog.stories.tsx | 64 ++++++++++ packages/react/dialog/tests/dialog.test.tsx | 118 ++++++++++++++++++ 4 files changed, 190 insertions(+), 5 deletions(-) diff --git a/packages/react/dialog/SPEC.md b/packages/react/dialog/SPEC.md index 258099e..1b39064 100644 --- a/packages/react/dialog/SPEC.md +++ b/packages/react/dialog/SPEC.md @@ -78,10 +78,11 @@ React-specific notes on top of the core contract: 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). + 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). diff --git a/packages/react/dialog/src/dialog.tsx b/packages/react/dialog/src/dialog.tsx index befc701..18d1136 100644 --- a/packages/react/dialog/src/dialog.tsx +++ b/packages/react/dialog/src/dialog.tsx @@ -65,9 +65,10 @@ export const Dialog: ((props: DialogProps) => ReactNode) & Parts = ({ children, backNavigate: () => apiRef.current.backNavigate(), forwardNavigate: () => apiRef.current.forwardNavigate(), isOpen: () => machine.matches('open'), + depth, }) guardRef.current.sync(api.open) - }, [api.open, machine]) + }, [api.open, machine, depth]) useEffect( () => () => { @@ -214,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 7d36690..9b27a00 100644 --- a/packages/react/dialog/stories/dialog.stories.tsx +++ b/packages/react/dialog/stories/dialog.stories.tsx @@ -458,3 +458,67 @@ export const closeOnBack: StoryType = { ), } + +// 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 f326d7a..2866df6 100644 --- a/packages/react/dialog/tests/dialog.test.tsx +++ b/packages/react/dialog/tests/dialog.test.tsx @@ -559,6 +559,97 @@ describe('Dialog', () => { }) }) + // 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 in one + // traversal — settle it here, not in the next test. + const consume = nextPop() + unmount() + await act(async () => { + await consume + }) + }) + + // 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() // one traversal for both entries + rerender() + await act(async () => { + await consume + }) + expect(window.history.state).toEqual(before) + }) + it('reopening through the trigger plants a fresh guard, truncating the spent entry', async () => { render() const pop = nextPop() @@ -669,6 +760,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') From 55c768244064fda9bb1e67af46c5301ce03e5c87 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 11:30:43 +0200 Subject: [PATCH 11/15] feat(solid-dialog): escapeScope, the nested Forward round-trip, and the story The Solid mirror of the React commit: dismiss + depth wiring in the binding, the nestedCloseOnBack story, and the same three test scenarios. Co-Authored-By: Claude Opus 5 (1M context) --- packages/solid/dialog/SPEC.md | 9 +- packages/solid/dialog/src/dialog.tsx | 2 + .../solid/dialog/stories/dialog.stories.tsx | 64 ++++++++++ packages/solid/dialog/tests/dialog.test.tsx | 118 ++++++++++++++++++ 4 files changed, 189 insertions(+), 4 deletions(-) diff --git a/packages/solid/dialog/SPEC.md b/packages/solid/dialog/SPEC.md index b119b33..d9f9200 100644 --- a/packages/solid/dialog/SPEC.md +++ b/packages/solid/dialog/SPEC.md @@ -83,10 +83,11 @@ Solid-specific notes on top of the core contract: 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). + 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). diff --git a/packages/solid/dialog/src/dialog.tsx b/packages/solid/dialog/src/dialog.tsx index 9ed56be..c6c982b 100644 --- a/packages/solid/dialog/src/dialog.tsx +++ b/packages/solid/dialog/src/dialog.tsx @@ -68,6 +68,7 @@ export const Dialog: Component & Parts = props => { backNavigate: () => untrack(() => api.backNavigate()), forwardNavigate: () => untrack(() => api.forwardNavigate()), isOpen: () => machine.matches('open'), + depth, }) guard.sync(open) }, @@ -235,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 ea02f64..1053549 100644 --- a/packages/solid/dialog/stories/dialog.stories.tsx +++ b/packages/solid/dialog/stories/dialog.stories.tsx @@ -461,3 +461,67 @@ export const closeOnBack: StoryType = { ), } + +// 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 e02f0ff..6d3de0e 100644 --- a/packages/solid/dialog/tests/dialog.test.tsx +++ b/packages/solid/dialog/tests/dialog.test.tsx @@ -562,6 +562,96 @@ describe('Dialog', () => { 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 in one + // traversal — settle it here, not in the next test. + const consume = nextPop() + cleanup() + await consume + }) + + // 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() // one traversal for both entries + setOpen(false) + flush() + await consume + expect(window.history.state).toEqual(before) + }) + it('reopening through the trigger plants a fresh guard, truncating the spent entry', async () => { render(() => ) flush() @@ -665,6 +755,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') From 9d93cfc638c75e8ee8c48a1ad51b2537645680cf Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 18:31:26 +0200 Subject: [PATCH 12/15] fix(dom-scroll-lock): save, hide, and restore overflow per axis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overflow shorthand serializes back to '' unless both longhands are set, so a container scrolling on one axis (overflow-y: auto) saved as unset and release removed the consumer's own declaration for good. All three operations now run on the overflow-x/overflow-y longhands, and the tests assert both axes by name while locked — the only point where jsdom can tell the two implementations apart. Co-Authored-By: Claude Fable 5 --- .changeset/scroll-lock-overflow-per-axis.md | 16 ++++++ .oxlintrc.json | 3 +- packages/dom/utils/scroll-lock/SPEC.md | 1 + .../dom/utils/scroll-lock/src/lock-scroll.ts | 17 +++++-- .../scroll-lock/tests/lock-scroll.test.ts | 51 ++++++++++++++----- packages/react/dialog/tests/dialog.test.tsx | 14 ++--- .../tests/use-scroll-lock.test.ts | 8 +-- packages/solid/dialog/tests/dialog.test.tsx | 14 ++--- .../tests/use-scroll-lock.test.ts | 8 +-- 9 files changed, 93 insertions(+), 39 deletions(-) create mode 100644 .changeset/scroll-lock-overflow-per-axis.md 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/.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/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/react/dialog/tests/dialog.test.tsx b/packages/react/dialog/tests/dialog.test.tsx index 2866df6..60b12fe 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() }) }) @@ -836,7 +836,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/tests/dialog.test.tsx b/packages/solid/dialog/tests/dialog.test.tsx index 6d3de0e..a5bcd4d 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() }) }) @@ -834,7 +834,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() }) }) From 560d539dac5d2ed4b318b9ddad08f9717ddb8f00 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 18:31:41 +0200 Subject: [PATCH 13/15] fix(dom-focus-trap): rendered check without checkVisibility; iframe and summary join the cycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Element.checkVisibility() 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 the API the throw left Tab dead entirely. Visibility is now a computed-style walk (spec behavior everywhere), which also retires the test-environment shim: vitest.setup.ts existed only for it and is gone. The selector also gains iframe and details > summary:first-of-type — the browser tabs to both, and the trap stepping past them made them keyboard-unreachable while a dialog was open. Co-Authored-By: Claude Fable 5 --- .changeset/focus-trap-rendered-walk.md | 17 ++++++++++++++ .changeset/focus-trap-tab-stop-fidelity.md | 5 ++-- packages/dom/utils/focus-trap/SPEC.md | 10 ++++---- .../utils/focus-trap/src/get-focusables.ts | 23 ++++++++++++++++++- .../utils/focus-trap/tests/trap-focus.test.ts | 19 +++++++++++++++ packages/solid/vitest.config.ts | 1 - vitest.config.ts | 1 - vitest.setup.ts | 19 --------------- 8 files changed, 65 insertions(+), 30 deletions(-) create mode 100644 .changeset/focus-trap-rendered-walk.md delete mode 100644 vitest.setup.ts 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/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/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/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 - } -} From 4698e0c5f24182e050473cd68faf2e60d2b66630 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 18:31:52 +0200 Subject: [PATCH 14/15] fix(overlay,dom-overlay): containment follows the topmost modal layer, not the topmost layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A non-modal layer opening above a modal one — a select menu, a combobox list, a tooltip — released the modal layer's containment: the page behind an open dialog became reachable exactly while someone was interacting. The stack now exposes ordered() (topmost first; modality stays the host's vocabulary), syncContainment hides around the topmost modal layer, and every layer stacked above it is held out of the hiding by ancestry — non-modal layers portal to the body, so they surface as siblings through their portal wrappers, not descendants. Escape and the focus trap still follow the topmost layer; only containment decouples. Co-Authored-By: Claude Fable 5 --- .changeset/overlay-ordered-layers.md | 27 ++++++++ packages/core/utils/overlay/SPEC.md | 30 +++++---- .../core/utils/overlay/src/layer-stack.ts | 38 +++++++---- .../utils/overlay/tests/layer-stack.test.ts | 10 +++ packages/dom/utils/overlay/SPEC.md | 23 +++++-- .../dom/utils/overlay/src/hide-outside.ts | 36 +++++++--- packages/dom/utils/overlay/src/stack.ts | 35 ++++++++-- .../utils/overlay/tests/containment.test.ts | 67 +++++++++++++++++++ 8 files changed, 220 insertions(+), 46 deletions(-) create mode 100644 .changeset/overlay-ordered-layers.md 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/packages/core/utils/overlay/SPEC.md b/packages/core/utils/overlay/SPEC.md index 1ddbfdf..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,17 +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)`, `below(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 @@ -51,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 6443a65..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,6 +22,11 @@ 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 @@ -36,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++ } @@ -60,6 +73,7 @@ export function createLayerStack(): LayerStack { } }, topmost, + ordered, isTopmost(id) { return topmost()?.id === id }, diff --git a/packages/core/utils/overlay/tests/layer-stack.test.ts b/packages/core/utils/overlay/tests/layer-stack.test.ts index 3eb0e7e..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,16 @@ 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', () => { diff --git a/packages/dom/utils/overlay/SPEC.md b/packages/dom/utils/overlay/SPEC.md index de13083..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 @@ -86,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/stack.ts b/packages/dom/utils/overlay/src/stack.ts index e75874e..c0c8a90 100644 --- a/packages/dom/utils/overlay/src/stack.ts +++ b/packages/dom/utils/overlay/src/stack.ts @@ -47,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 { 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 From 8ae32b422e66b9bf9810a85d299a730205f2f1ca Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Tue, 25 Aug 2026 18:32:04 +0200 Subject: [PATCH 15/15] fix(browser-navigation): chain sibling-release consumption one pop at a time; a throwing onBack declines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Entries below the current one are opaque, so the batched history.go(-n) could cross navigation the app planted itself. Released entries are now spent through a chain of single history.back() pops — each landing continues the chain and stops at the first entry that isn't ours. A released entry buried beneath a live layer no longer swallows the Back that surfaces it: the press unwinds the crossed guard and the chain consumes the dead entry. A parked guard's entry joins the chain only when a declined reopen left it current — its ground otherwise lives in the forward stack and is not the chain's to spend. An onBack that throws now counts as a decline: the guard re-arms in a finally so the next Back still reaches the layer while the error propagates, instead of aborting the unwind with no entry re-planted. Co-Authored-By: Claude Fable 5 --- .changeset/back-guard-batched-release.md | 6 +- .changeset/back-guard-chained-consumption.md | 16 ++ packages/dom/utils/navigation/SPEC.md | 22 ++- .../src/intercept-back-navigation.ts | 155 ++++++++++-------- .../tests/intercept-back-navigation.test.ts | 65 +++++++- packages/react/dialog/tests/dialog.test.tsx | 8 +- packages/solid/dialog/tests/dialog.test.tsx | 8 +- 7 files changed, 189 insertions(+), 91 deletions(-) create mode 100644 .changeset/back-guard-chained-consumption.md diff --git a/.changeset/back-guard-batched-release.md b/.changeset/back-guard-batched-release.md index 39a2a36..f98470f 100644 --- a/.changeset/back-guard-batched-release.md +++ b/.changeset/back-guard-batched-release.md @@ -16,8 +16,8 @@ releaseOuter() history.back() // before: ❌ nothing happens — spent on outer's leftover entry -// after: ✅ leaves the page — the freed run was consumed as one traversal +// after: ✅ leaves the page — every freed entry was consumed ``` -Release order doesn't matter (the run is read from the entry order), and an -entry genuinely buried under later in-app navigation is still left alone. +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/packages/dom/utils/navigation/SPEC.md b/packages/dom/utils/navigation/SPEC.md index 6c4b93f..a14e490 100644 --- a/packages/dom/utils/navigation/SPEC.md +++ b/packages/dom/utils/navigation/SPEC.md @@ -22,7 +22,9 @@ it reopens the layer. 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. + 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 @@ -50,9 +52,12 @@ it reopens the layer. 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. An entry buried under later - in-app navigation is unreachable and left alone — Back then both navigates - and closes the layer. + 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 @@ -103,6 +108,13 @@ Back then closes for free and needs no interceptor. - 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 @@ -113,5 +125,5 @@ Back then closes for free and needs no interceptor. | 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. | -| Consumption is batched per turn, over a snapshot of the entry order | A multi-entry `go` is one traversal and one `popstate`, so a freed run costs the same as a single entry. The snapshot is taken before the first release splices, so the call order of sibling releases can't change the run. | +| 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/intercept-back-navigation.ts b/packages/dom/utils/navigation/src/intercept-back-navigation.ts index 73e399e..a9ecea0 100644 --- a/packages/dom/utils/navigation/src/intercept-back-navigation.ts +++ b/packages/dom/utils/navigation/src/intercept-back-navigation.ts @@ -49,14 +49,13 @@ let nextGuardId = 0 // 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 — the listener must outlive -// them, or one release's idle check could detach it under a sibling's pop. -let pendingReleases = 0 -// A turn's releases, consumed together: a stack closing at once has only its -// topmost entry current, so one-at-a-time would strand every entry beneath. -// `order` snapshots the entry order before the first release splices. -let batch: Set | null = null -let order: number[] | null = null +// 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 @@ -108,43 +107,29 @@ function plantEntry(guard: BackGuard): void { history.pushState({ [STATE_KEY]: guard.id, [CLAIM_KEY]: guard.claim }, '') } -// Once per turn, after every release in it: consume the freed run of entries — -// current one down, while contiguous — in a single traversal. -function consumeBatch(): void { - const released = batch as Set - const entryOrder = order as number[] - batch = null - order = null - pendingReleases = 0 - +// 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() - // Nothing of ours is current: buried under later navigation, or adopted by a - // same-turn re-register. Unreachable entries are left alone. - if (current === undefined || !released.has(current)) { - detachWhenIdle() - return - } - // A guard that both registered and released inside this turn isn't in the - // snapshot; its own entry is still the one to consume. - const top = entryOrder.indexOf(current) - let count = 1 - for (let index = top - 1; index >= 0 && released.has(entryOrder[index] as number); index--) { - count++ + if (current !== undefined && pendingConsumption.delete(current)) { + swallow++ + history.back() } - swallow++ - history.go(-count) } // Detach only when nothing is left to hear — parked guards, claim watchers, -// in-flight self-caused pops, and undecided releases all still need it. +// 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 && parked.length === 0 && watchers.length === 0 && swallow === 0 && - pendingReleases === 0 + !consumptionScheduled ) { + pendingConsumption.clear() window.removeEventListener('popstate', onPopState) } } @@ -157,16 +142,21 @@ function onPopState(): void { const top = guards[guards.length - 1] if (top !== undefined && top.id !== currentGuardId()) { 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 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. - if (current !== undefined && !isArmed(current)) { + // 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--) { @@ -189,23 +179,29 @@ function onPopState(): void { 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) - // 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) - } - 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. - plantEntry(top) - 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() } @@ -217,9 +213,11 @@ function onPopState(): void { * entry in place. * * Consumption is deferred a microtask so a same-turn release + re-register - * adopts the entry in place with zero traversals — 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. + * 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, @@ -236,10 +234,12 @@ export function interceptBackNavigation( const current = currentGuardId() guards.push(guard) if (current !== undefined && !isArmed(current)) { - // Adoption steals the entry from a parked watcher too — the ground now - // belongs to this registration. + // 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) @@ -247,26 +247,41 @@ export function interceptBackNavigation( return (releaseOptions: ReleaseOptions = {}) => { if (releaseOptions.keepClaim !== true) abandoned.add(guard.id) - // Snapshot the entry order before this turn's first release splices it. - if (batch === null) { - batch = new Set() - order = [...guards.map(entry => entry.id), ...parked.map(entry => entry.id)] - queueMicrotask(consumeBatch) - } 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) + // 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) { + pendingConsumption.add(guard.id) + scheduleConsumption() + } else { + detachWhenIdle() + } + return } - batch.add(guard.id) - pendingReleases++ + 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 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 0ba5fd6..b4c1bfe 100644 --- a/packages/dom/utils/navigation/tests/intercept-back-navigation.test.ts +++ b/packages/dom/utils/navigation/tests/intercept-back-navigation.test.ts @@ -84,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) @@ -94,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) @@ -104,18 +105,19 @@ describe('interceptBackNavigation', () => { }) // A whole stack freed in one commit — close-all, or an unmounting subtree. - // Only the topmost entry is current, so consuming one at a time would leave - // every entry beneath behind, each swallowing a later Back. Release order is - // irrelevant: the run is read from the entry order, not the call order. + // 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() // one traversal for the whole run + const pop = nextPop() releaseUpper() releaseLower() await pop + await nextPop() expect(history.state).toEqual(before) const outerFirst = interceptBackNavigation(() => true) @@ -124,6 +126,55 @@ describe('interceptBackNavigation', () => { 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) }) diff --git a/packages/react/dialog/tests/dialog.test.tsx b/packages/react/dialog/tests/dialog.test.tsx index 60b12fe..bac6a27 100644 --- a/packages/react/dialog/tests/dialog.test.tsx +++ b/packages/react/dialog/tests/dialog.test.tsx @@ -608,12 +608,13 @@ describe('Dialog', () => { await traverse(() => window.history.forward()) expect(layers()).toBe('OI') - // Both layers are armed again; unmounting frees their entries in one - // traversal — settle it here, not in the next test. + // 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() }) }) @@ -642,10 +643,11 @@ describe('Dialog', () => { const { rerender } = render() expect(window.history.state).not.toEqual(before) - const consume = nextPop() // one traversal for both entries + 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) }) diff --git a/packages/solid/dialog/tests/dialog.test.tsx b/packages/solid/dialog/tests/dialog.test.tsx index a5bcd4d..5d0a7f3 100644 --- a/packages/solid/dialog/tests/dialog.test.tsx +++ b/packages/solid/dialog/tests/dialog.test.tsx @@ -611,11 +611,12 @@ describe('Dialog', () => { await traverse(() => window.history.forward()) expect(layers()).toBe('OI') - // Both layers are armed again; disposing frees their entries in one - // traversal — settle it here, not in the next test. + // 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, @@ -645,10 +646,11 @@ describe('Dialog', () => { flush() expect(window.history.state).not.toEqual(before) - const consume = nextPop() // one traversal for both entries + 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) })