Skip to content

feat(dialog): the browser Forward reopens what Back closed - #39

Merged
ivanbanov merged 17 commits into
mainfrom
fix-dialog-forward-navigation
Aug 25, 2026
Merged

feat(dialog): the browser Forward reopens what Back closed#39
ivanbanov merged 17 commits into
mainfrom
fix-dialog-forward-navigation

Conversation

@ivanbanov

@ivanbanov ivanbanov commented Aug 14, 2026

Copy link
Copy Markdown
Member

What

closeOnBack becomes symmetric: the browser's Back closes the dialog (as before), and the browser's Forward now reopens what Back closed — guarded again for the next Back. No new setting: back-close and forward-reopen are one behavior (the dialog's openness tracking the history position), so the existing closeOnBack gates both.

The reopen follows the shared dismissal contract: a new onForwardNavigation callback fires first and preventDefault() vetoes; a controlled dialog only records the intent.

Both DOM substrates ship it — React and Solid — from the same code.

How

The model that keeps this cheap: the guard entry a Back press pops isn't gone — it survives in the browser's forward stack, still marking the dialog's open ground. A traversal re-entering it is the host's Forward.

  • @dunky.dev/browser-navigationinterceptBackNavigation(onBack, onForward?) grew the optional second callback. A Back-closed guard parks instead of dropping; a popstate landing on its spent entry offers the reopen and re-arms the guard on the entry in place (no new entry planted). The safety invariant: parked entries always sit above every armed entry (every planted entry truncates the forward stack), so ownership of the landing entry — not traversal direction, which popstate doesn't carry — decides reopen vs unwind. Marked ground with no armed owner never unwinds a layer; without that, Forward onto a dead nested-dialog entry would wrongly close its parent. Callers that don't pass onForward behave exactly as before.
  • @dunky.dev/dialoghistory.forward mirrors history.back, gated by the same closeOnBack, reachable from closed and from the animated closing state (Forward interrupts the exit). The connect gains forwardNavigate(), sharing the synthesized veto payload with backNavigate().
  • @dunky.dev/dom-dialog — owns the whole episode, so no host re-derives it. guardBackNavigation returns { sync, release } rather than a disposer: the guard deliberately outlives the open state, so a host reports every change through sync(open) and ends the episode with release(). Whether a close parks the registration (the Back press closed it, its spent entry is still worth watching) or releases it (any other close has no way back) is decided here, once.
  • @dunky.dev/react-dialog, @dunky.dev/solid-dialog — contribute only their host's lifecycle around that: React reports from an effect keyed on the open state and releases on unmount, Solid from a createEffect and onCleanup. Reopening through the trigger plants a fresh entry, truncating the spent one — exactly like navigating after a Back.

Merge with main

main extracted the Dialog's DOM half into @dunky.dev/dom-dialog while this branch grew the Forward reopen inline in the React root — the one conflict. Resolved by moving the fix down a layer rather than keeping it in the host, which is also what gave Solid the behavior for free instead of a second copy of the episode logic.

Synced with main

Merged twice, both times resolving into the shared layer rather than around it. The second sync brought #46's rename (@dunky.dev/dom-navigation -> @dunky.dev/browser-navigation, followed here) and #48's hardening of the very interceptor this branch extends. Both conflicts resolved as unions:

  • detachWhenIdle — the listener now outlives armed guards, parked watchers, in-flight self-caused pops, and undecided releases. Dropping the parked check would end a Forward watch the moment its layer closed.
  • The unwind loop — main removes the guard by identity (onBack may have released it, and a positional pop would evict the guard beneath). Parking now happens only when that removal actually found the guard: a layer that released itself inside onBack tore itself down rather than closing, so there is nothing to offer a reopen to. Covered by a new test and a SPEC clause.

Caveats (spec'd)

  • 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 (URL-held open state remains the answer there, per the navigation SPEC's Reload section).
  • The Forward watch lives in script, so it doesn't survive a reload.
  • Native has no Forward counterpart — the platform offers a Back gesture and no Forward one — so forwardNavigate stays unwired there, recorded in native-dialog's SPEC.

Testing

  • 21 new behavior tests: 6 browser-navigation (reopen + re-arm, decline keeps watching, release ends the watch, a planted entry truncates the watch, stacked guards reopen one per Forward lowest-first, a guard releasing itself inside onBack never parks), 4 core machine/connect, 3 dom-dialog (park through a Back-close, release on any other close, release ends a parked episode), 4 React and 4 Solid (Forward reopens guarded again, no reopen after other closes, veto, trigger-reopen truncates).
  • Full suite, build, typecheck, lint, and format pass. Both closeOnBack stories gained a "Simulate browser Forward" button.

Housekeeping

  • Docs name React and Solid as the substrates instead of Vue, which was the stand-in second framework while Solid didn't exist. Published CHANGELOG entries keep their Vue mention — they are the release record for what shipped.
  • lint-staged moves to .lintstagedrc.ts and filters scripts/templates/**: oxlint and oxfmt both ignore that path and both exit non-zero when every file handed to them is ignored, so a commit touching only scaffold templates failed the pre-commit hook with nothing actually wrong.

🤖 Generated with Claude Code

ivanbanov and others added 17 commits August 14, 2026 22:27
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 <noreply@anthropic.com>
main extracted the Dialog's DOM half into @dunky.dev/dom-dialog, which took
the history guard with it; this branch had grown the Forward reopen inline in
the React root. Resolved by moving the fix down a layer instead of keeping it
in the host: the whole episode is dom-dialog's.

- dom-dialog: guardBackNavigation returns { sync, release } rather than a
  disposer. The guard outlives the open state — that is the Forward watch —
  so a host reports every change through sync(open) and ends the episode with
  release(). Whether a close parks the registration (the Back press closed it,
  the spent entry is still worth watching) or releases it is decided here,
  once for every substrate.
- react-dialog, solid-dialog: both contribute only their host's lifecycle, so
  Solid inherits the Forward reopen with the same tests, spec, and story as
  React.
- native-dialog: records why the Forward half doesn't port — the platform has
  a Back gesture and no Forward one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
main's #46 renamed @dunky.dev/dom-navigation to @dunky.dev/browser-navigation
and #48 hardened the same interceptor this branch extends. Two conflicts, both
unions rather than choices:

- detachWhenIdle: the listener now outlives armed guards, parked watchers,
  in-flight self-caused pops, AND undecided releases. Dropping the parked
  check would end a Forward watch the moment its layer closed.
- The unwind loop: main removes the guard by identity (onBack may have
  released it, and a positional pop would evict the guard beneath). Parking
  now happens only when that removal actually found the guard — a layer that
  released itself inside onBack tore itself down rather than closing, so
  there is nothing to offer a reopen to. New test and SPEC clause for it.

The changeset follows the rename. Full suite (240), typecheck, lint, format,
and build pass; SPECs reconciled against the merged code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…stack

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) <noreply@anthropic.com>
…s ground

- 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…he story

- 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) <noreply@anthropic.com>
…he 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) <noreply@anthropic.com>
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 <noreply@anthropic.com>
…nd summary join the cycle

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 <noreply@anthropic.com>
…, not the topmost layer

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 <noreply@anthropic.com>
… a time; a throwing onBack declines

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 <noreply@anthropic.com>
@ivanbanov
ivanbanov merged commit 5d732d4 into main Aug 25, 2026
6 checks passed
@ivanbanov
ivanbanov requested a lite review from Copilot August 26, 2026 15:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR enhances overlay/dialog navigation and stacking behavior across DOM/React/Solid, while tightening several low-level DOM utilities (overlay containment, focus trap focusables, and scroll-lock overflow restore).

Changes:

  • Add symmetric closeOnBack behavior: browser Forward can reopen what Back closed (new onForwardNavigation, spent-entry claiming/watching).
  • Implement stack-scoped Escape (escapeScope='stack') using layer-stack introspection + dismiss callbacks.
  • Fix DOM utilities: containment follows topmost modal layer, focus trap uses a rendered-ness walk (no checkVisibility shim), and scroll lock restores overflow per-axis.

Reviewed changes

Copilot reviewed 59 out of 59 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
vitest.config.ts Removes global setup file now that focus-trap no longer relies on Element.checkVisibility() shim.
packages/solid/vitest.config.ts Same: removes setupFiles dependency in Solid package tests.
scripts/templates/packages/dom/components/name/src/effects.ts Updates template comment to match currently supported substrates.
packages/solid/hooks/use-scroll-lock/tests/use-scroll-lock.test.ts Aligns assertions with per-axis overflow behavior (overflowY).
packages/solid/dialog/tests/dialog.test.tsx Adds tests for Forward-reopen and stack-scoped Escape; updates scroll-lock assertions.
packages/solid/dialog/stories/dialog.stories.tsx Updates Storybook stories to demonstrate Forward reopen and nested guard behavior.
packages/solid/dialog/src/dialog.tsx Introduces long-lived Back/Forward guard lifecycle (sync + release) and provides dismiss to the overlay layer.
packages/solid/dialog/SPEC.md Documents onForwardNavigation and Forward reopen semantics.
packages/react/hooks/use-scroll-lock/tests/use-scroll-lock.test.ts Aligns assertions with per-axis overflow behavior (overflowY).
packages/react/dialog/tests/dialog.test.tsx Adds tests for Forward-reopen and stack-scoped Escape; updates scroll-lock assertions.
packages/react/dialog/stories/dialog.stories.tsx Updates Storybook stories to demonstrate Forward reopen and nested guard behavior.
packages/react/dialog/src/dialog.tsx Adds Back/Forward guard lifecycle wiring + cleanup; provides dismiss to overlay registration.
packages/react/dialog/SPEC.md Documents onForwardNavigation and Forward reopen semantics.
packages/native/dialog/SPEC.md Documents that Forward reopen has no native analogue and remains unwired.
packages/dom/utils/scroll-lock/src/lock-scroll.ts Saves/hides/restores overflow per-axis (overflow-x/overflow-y) to preserve one-axis declarations.
packages/dom/utils/scroll-lock/tests/lock-scroll.test.ts Expands tests to cover axis-specific restore and avoids asserting shorthand in jsdom.
packages/dom/utils/scroll-lock/SPEC.md Documents per-axis overflow handling rationale.
packages/dom/utils/overlay/src/stack.ts Makes containment follow topmost modal layer; adds dismiss to layer and exports layersBelow.
packages/dom/utils/overlay/src/index.ts Re-exports layersBelow for stack-scoped dismissal features.
packages/dom/utils/overlay/src/hide-outside.ts Generalizes exclude to a list and matches exclusions by containment (portal wrapper aware).
packages/dom/utils/overlay/tests/containment.test.ts Adds coverage for containment behavior when non-modal layers sit above modal layers.
packages/dom/utils/overlay/SPEC.md Updates containment spec + documents layersBelow and containment-based exclusions.
packages/dom/utils/navigation/src/intercept-back-navigation.ts Adds Forward reopen, parked guards, claim-based reopen after teardown, chained consumption, and hardened error behavior.
packages/dom/utils/navigation/src/index.ts Exports new navigation APIs/types (watchSpentEntry, options).
packages/dom/utils/navigation/tests/intercept-back-navigation.test.ts Adds comprehensive tests for Forward reopen, chained consumption, and claim-based behavior.
packages/dom/utils/navigation/SPEC.md Documents new Back/Forward semantics, claims, abandonment, and release consumption behavior.
packages/dom/utils/focus-trap/src/get-focusables.ts Adds iframe and details > summary to focusables; replaces checkVisibility usage with a rendered-ness walk.
packages/dom/utils/focus-trap/tests/trap-focus.test.ts Adds coverage for iframe + details/summary participation in the tab cycle.
packages/dom/utils/focus-trap/SPEC.md Updates internals: removes checkVisibility dependency and shim justification.
packages/dom/components/dialog/src/effects.ts Implements stack-scoped Escape by closing layers below via Layer.dismiss.
packages/dom/components/dialog/src/open-layer.ts Passes optional dismiss into overlay registration for stack-scoped dismissal.
packages/dom/components/dialog/src/back-navigation.ts Reworks guard to long-lived { sync, release }, adds Forward reopen and spent-entry claim/watch.
packages/dom/components/dialog/src/index.ts Exports BackNavigationGuard type alongside options.
packages/dom/components/dialog/tests/dialog.test.ts Adds tests for stack-scoped Escape and Forward reopen in guardBackNavigation.
packages/dom/components/dialog/SPEC.md Updates Escape behavior spec and Back/Forward guard contract documentation.
packages/core/utils/overlay/src/layer-stack.ts Adds ordered() and below() to support modal containment and stack-scoped dismissal ordering.
packages/core/utils/overlay/tests/layer-stack.test.ts Adds test coverage for ordered() and below() behaviors.
packages/core/utils/overlay/SPEC.md Documents new layer-stack APIs and updated containment rationale.
packages/core/dialog/src/types.ts Adds escapeScope, history.forward event, and onForwardNavigation callback types.
packages/core/dialog/src/machine.ts Implements Forward reopen transition when closeOnBack is enabled.
packages/core/dialog/src/connect.ts Adds forwardNavigate() and factors shared history-veto payload logic.
packages/core/dialog/src/index.ts Exports DialogEscapeScope.
packages/core/dialog/tests/machine.test.ts Adds tests for Forward reopen behavior and veto/controlled behavior.
packages/core/dialog/SPEC.md Documents Forward reopen as the mirror of Back-close and clarifies stack-scoped Escape description.
README.md Updates substrate diagram to reflect React/Solid/Native.
CONTRIBUTING.md Updates substrate list (React/Solid/…).
ARCHITECTURE.md Updates substrate references and effect-sharing narrative.
.oxlintrc.json Allows expectOverflow to satisfy vitest/expect-expect.
.lintstagedrc.json Removes JSON lint-staged config (replaced by TS config).
.lintstagedrc.ts Adds lint-staged TS config that avoids failing when only templates are staged.
.changeset/scroll-lock-overflow-per-axis.md Changeset for per-axis overflow fix in scroll-lock.
.changeset/overlay-ordered-layers.md Changeset for ordered layer stack + containment fix.
.changeset/focus-trap-tab-stop-fidelity.md Changeset update reflecting focus-trap behavior changes.
.changeset/focus-trap-rendered-walk.md Changeset for removing checkVisibility dependency and adding focusables.
.changeset/dialog-forward-reopens.md Changeset for symmetric closeOnBack (Forward reopen) across dialog packages.
.changeset/dialog-escape-scope.md Changeset for implementing escapeScope.
.changeset/back-guard-chained-consumption.md Changeset for chained consumption + error hardening in navigation guard.
.changeset/back-guard-batched-release.md Changeset for batched release consumption correctness.
vitest.setup.ts Removes now-unneeded Element.checkVisibility() shim.
Suppressed comments (1)

packages/dom/utils/focus-trap/src/get-focusables.ts:1

  • isRendered checks display: none on the element and its ancestors up to but excluding container. If the container itself is display: none, descendants should be treated as non-rendered, but this loop will return true. Include container in the display check (e.g., loop until node === null or explicitly check container after the loop) so focus cycles never include elements inside a hidden container.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .lintstagedrc.ts
// 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/'
Comment thread .lintstagedrc.ts

const config: Configuration = {
'*.{ts,tsx}': files => {
const checkable = files.filter(file => !file.includes(IGNORED))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants