diff --git a/.changeset/dialog-press-gates.md b/.changeset/dialog-press-gates.md new file mode 100644 index 0000000..210e669 --- /dev/null +++ b/.changeset/dialog-press-gates.md @@ -0,0 +1,23 @@ +--- +'@dunky.dev/dom-dialog': minor +'@dunky.dev/react-dialog': patch +'@dunky.dev/solid-dialog': patch +--- + +`acceptsBackdropPress` / `acceptsViewportPress` are replaced by +`gateBackdropPress(id, bindings)` / `gateViewportPress(id, bindings)`. The +predicates left every DOM substrate writing the same `onClick` wrapper around +them by hand — destructure the press out of the part's normalized bindings, +re-wrap it behind the check — which is exactly the kind of shared DOM behavior +this package exists to hold. The gate now takes the normalized bindings and +returns them with the press gated; a substrate just passes the result to its +`mergeProps`: + +```tsx +
+``` + +The gating rules are unchanged: only the topmost dialog of a stack answers an +outside press, and a viewport press must have started on the viewport itself +rather than bubbled up from the content. React and Solid dialogs use the gates +internally — no consumer-facing change there. diff --git a/.changeset/navigation-shared-registry.md b/.changeset/navigation-shared-registry.md new file mode 100644 index 0000000..db53b8d --- /dev/null +++ b/.changeset/navigation-shared-registry.md @@ -0,0 +1,14 @@ +--- +'@dunky.dev/browser-navigation': patch +--- + +The guard registry now anchors on a realm-global keyed by `Symbol.for`, +matching `@dunky.dev/dom-overlay` and `@dunky.dev/dom-scroll-lock`. A monorepo +or micro-frontend can load more than one copy of this module into the same +page; each copy previously kept its own registry and `popstate` listener while +all of them shared the one real session history, so a Back could be answered +by the wrong copy — a swallowed press, or an entry planted twice. Every +duplicate copy now rendezvouses on the same registry, and only one listener is +ever attached (the store remembers it — the browser's listener dedupe can't +span copies, since each copy's function has its own identity). Resolved lazily +on first use, so `sideEffects: false` still holds. diff --git a/.changeset/overlay-below-reuses-ordered.md b/.changeset/overlay-below-reuses-ordered.md new file mode 100644 index 0000000..15f4558 --- /dev/null +++ b/.changeset/overlay-below-reuses-ordered.md @@ -0,0 +1,8 @@ +--- +'@dunky.dev/overlay': patch +--- + +Internal cleanup: `below(id)` now derives "the layers beneath" from the same +`ordered()` ranking the rest of the stack uses — everything after the layer in +topmost-first order — instead of carrying a second copy of the depth/open-order +comparison. One ranking rule, written once. No behavior change. diff --git a/.changeset/overlay-hide-tracker.md b/.changeset/overlay-hide-tracker.md new file mode 100644 index 0000000..e8b2336 --- /dev/null +++ b/.changeset/overlay-hide-tracker.md @@ -0,0 +1,11 @@ +--- +'@dunky.dev/dom-overlay': patch +--- + +Internal cleanup: containment (`hideOutside`) and the exit window +(`hideExitingLayer`) now share one hide/undo tracker instead of each keeping +its own copy of the bookkeeping. The two rules the copies could have let +drift — what counts as author-hidden (an existing `inert`, a truthy +`aria-hidden`; `aria-hidden="false"` asserts visible and doesn't count) and +that the undo restores exactly the authored value — are now written once. No +behavior change. diff --git a/AGENTS.md b/AGENTS.md index ae55113..7099d87 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -138,6 +138,17 @@ verified across all scopes. If something's off, loop back to SPEC; if not, ship ## Code +### Principles + +Every change is held to these four, in this order — simplest thing that +works, written once, built only when needed, behaving as promised: + +| Principle | Meaning | +| --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **KISS** | Keep it simple. Prefer the plain solution over the clever one; complexity must earn its keep with a need the simple version can't meet. | +| **DRY** | Don't repeat yourself. A rule two places must agree on is written once and shared — duplication is where the copies drift apart. | +| **YAGNI** | You aren't gonna need it. Build for the requirement in front of you, not the one imagined; speculative machinery is deleted-on-sight, not kept just in case. | + ### Naming Descriptive names everywhere. Short names are fine for local variables diff --git a/packages/core/utils/overlay/src/layer-stack.ts b/packages/core/utils/overlay/src/layer-stack.ts index 95dbe4d..aa088a8 100644 --- a/packages/core/utils/overlay/src/layer-stack.ts +++ b/packages/core/utils/overlay/src/layer-stack.ts @@ -78,16 +78,11 @@ export function createLayerStack(): LayerStack { 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) + // "Beneath" is the same ranking `ordered` already answers: everything + // after the layer in topmost-first order. + const ranked = ordered() + const at = ranked.findIndex(layer => layer.id === id) + return at === -1 ? [] : ranked.slice(at + 1) }, } } diff --git a/packages/dom/components/dialog/SPEC.md b/packages/dom/components/dialog/SPEC.md index 89c39f4..6a3b84a 100644 --- a/packages/dom/components/dialog/SPEC.md +++ b/packages/dom/components/dialog/SPEC.md @@ -118,10 +118,12 @@ dialogs at the same place can't be told apart, and then neither reopens. ### Outside presses A press dismisses only when it is genuinely outside and genuinely this -dialog's to answer: +dialog's to answer. The gates take the part's normalized bindings and return +them with the press wrapped — the wrap every DOM substrate would otherwise +write identically: -- `acceptsBackdropPress` — the topmost dialog of a stack answers, nobody else. -- `acceptsViewportPress` — content presses bubble to the viewport, so the +- `gateBackdropPress` — the topmost dialog of a stack answers, nobody else. +- `gateViewportPress` — content presses bubble to the viewport, so the press must have started on the viewport itself, and then the same topmost rule applies. @@ -139,8 +141,8 @@ part is the cycle's last stop wherever it renders. | `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. | +| `gateBackdropPress(id, bindings)` | The backdrop bindings with the press gated to this dialog's outside turn. | +| `gateViewportPress(id, bindings)` | Same for the viewport, ignoring presses that bubbled from the content. | | `dialogTrapOptions(machine, closeId)` | `TrapFocusOptions` for the dialog window. | ## Constraints @@ -163,7 +165,8 @@ part is the cycle's last stop wherever it renders. | 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. | +| Press gating reads 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 gates wrap the bindings rather than exposing a predicate | Every DOM substrate would write the same `onClick` wrapper around the predicate; wrapping here keeps a substrate's contribution to lifecycle only. | | 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/index.ts b/packages/dom/components/dialog/src/index.ts index cecf6bd..8ce5acd 100644 --- a/packages/dom/components/dialog/src/index.ts +++ b/packages/dom/components/dialog/src/index.ts @@ -6,5 +6,5 @@ export { type BackNavigationGuard, type BackNavigationGuardOptions, } from './back-navigation' -export { acceptsBackdropPress, acceptsViewportPress } from './press' +export { gateBackdropPress, gateViewportPress } from './press' export { dialogTrapOptions } from './focus-trap' diff --git a/packages/dom/components/dialog/src/press.ts b/packages/dom/components/dialog/src/press.ts index 5301984..d0ecdcc 100644 --- a/packages/dom/components/dialog/src/press.ts +++ b/packages/dom/components/dialog/src/press.ts @@ -1,27 +1,49 @@ import { isTopmostLayer } from '@dunky.dev/dom-overlay' -// The parts of a DOM press event these predicates read — narrower than the -// host's synthetic event type, so React and Solid both satisfy it. +// The parts of a DOM press event the gates read — narrower than the host's +// synthetic event type, so React and Solid both satisfy it. interface PressTarget { target: EventTarget | null currentTarget: EventTarget | null } +// Every DOM substrate would wrap the normalized part's onClick identically, +// so the wrap lives here: fire the consumer-visible press only when it is +// this dialog's outside interaction, swallow it otherwise. +function gate( + bindings: Record, + accepts: (event: PressTarget) => boolean, +): Record { + const { onClick, ...rest } = bindings as { onClick?: (event: PressTarget) => void } & Record< + string, + unknown + > + rest.onClick = (event: PressTarget) => { + if (accepts(event)) onClick?.(event) + } + return rest +} + /** - * Whether a backdrop press is this dialog's outside interaction. Only the - * topmost dialog of a stack answers one — a nested stack dismisses one layer - * at a time, the same rule Escape follows. + * The backdrop part's bindings with the press gated: only the topmost dialog + * of a stack answers an outside interaction — a nested stack dismisses one + * layer at a time, the same rule Escape follows. */ -export function acceptsBackdropPress(id: string): boolean { - return isTopmostLayer(id) +export function gateBackdropPress( + id: string, + bindings: Record, +): Record { + return gate(bindings, () => isTopmostLayer(id)) } /** - * Whether a viewport press is this dialog's outside interaction. Content - * presses bubble up to the viewport, so only a press that started on the - * viewport itself counts — and then only for the topmost dialog. + * The viewport part's bindings with the press gated: content presses bubble + * up to the viewport, so only a press that started on the viewport itself + * counts — and then only for the topmost dialog. */ -export function acceptsViewportPress(id: string, event: PressTarget): boolean { - if (event.target !== event.currentTarget) return false - return isTopmostLayer(id) +export function gateViewportPress( + id: string, + bindings: Record, +): Record { + return gate(bindings, event => event.target === event.currentTarget && isTopmostLayer(id)) } diff --git a/packages/dom/components/dialog/tests/dialog.test.ts b/packages/dom/components/dialog/tests/dialog.test.ts index 984516d..c0a0426 100644 --- a/packages/dom/components/dialog/tests/dialog.test.ts +++ b/packages/dom/components/dialog/tests/dialog.test.ts @@ -11,10 +11,10 @@ import type { } from '@dunky.dev/dialog' import { registerLayer } from '@dunky.dev/dom-overlay' import { - acceptsBackdropPress, - acceptsViewportPress, dialogTrapOptions, domDialogEffects, + gateBackdropPress, + gateViewportPress, guardBackNavigation, openDialogLayer, startExitWindow, @@ -403,21 +403,36 @@ describe('guardBackNavigation', () => { }) describe('outside-press gating', () => { - it('lets only the topmost dialog answer a backdrop press', () => { + type GatedPress = { + onClick: (event: { target: Element | null; currentTarget: Element | null }) => void + } + + it('lets only the topmost dialog answer a backdrop press, passing the other bindings through', () => { mountLayer('dlg', 1) - expect(acceptsBackdropPress('dlg')).toBe(true) + const onClick = vi.fn() + const gated = gateBackdropPress('dlg', { onClick, 'data-state': 'open' }) as GatedPress + expect(gated).toMatchObject({ 'data-state': 'open' }) + + gated.onClick({ target: null, currentTarget: null }) + expect(onClick).toHaveBeenCalledTimes(1) mountLayer('above', 2) - expect(acceptsBackdropPress('dlg')).toBe(false) + gated.onClick({ target: null, currentTarget: null }) + expect(onClick).toHaveBeenCalledTimes(1) }) it('ignores a viewport press that bubbled up from the content', () => { mountLayer('dlg', 1) + const onClick = vi.fn() + const gated = gateViewportPress('dlg', { onClick }) as GatedPress const viewport = document.createElement('div') const content = document.createElement('div') - expect(acceptsViewportPress('dlg', { target: viewport, currentTarget: viewport })).toBe(true) - expect(acceptsViewportPress('dlg', { target: content, currentTarget: viewport })).toBe(false) + gated.onClick({ target: viewport, currentTarget: viewport }) + expect(onClick).toHaveBeenCalledTimes(1) + + gated.onClick({ target: content, currentTarget: viewport }) + expect(onClick).toHaveBeenCalledTimes(1) }) }) diff --git a/packages/dom/utils/navigation/SPEC.md b/packages/dom/utils/navigation/SPEC.md index 4fe6d77..844de55 100644 --- a/packages/dom/utils/navigation/SPEC.md +++ b/packages/dom/utils/navigation/SPEC.md @@ -175,8 +175,9 @@ outcome. ## Constraints -- One shared registry and one `popstate` listener module-wide — the - one-pop-one-guard ordering is the whole unwinding contract. +- One shared registry and one `popstate` listener realm-wide — even across + duplicate copies of this module — the one-pop-one-guard ordering is the + whole unwinding contract. - 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. @@ -196,12 +197,13 @@ outcome. ## 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. | -| 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. | +| 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. | +| The registry anchors on a realm-global keyed by `Symbol.for`, resolved lazily | Duplicate module copies (monorepo, micro-frontend) would fork the registry while sharing the one real session history, answering pops from the wrong copy — the duplicate-singleton bug class of radix-ui/primitives#2815. The store also remembers the attached listener: each copy's function differs, so the browser's (type, listener) dedupe can't span copies. Lazy keeps `sideEffects: false` honest. | +| Consumption is deferred a microtask | A queued `history.back()` is not reliably delivered once another entry is pushed before it lands; letting a same-turn re-register adopt the entry removes the race instead of compensating for it. | +| Self-caused pops are counted, and re-arm a live guard whose entry they consumed | The browser reports them through the same `popstate` as a user's Back; uncounted, one release would unwind another layer. | +| A Back-closed guard parks instead of dropping; ownership of the landing entry — not traversal direction — decides reopen vs unwind | `popstate` carries no direction. A parked or stale marker can only be forward residue above the armed guards (pushes truncate it everywhere else), so landing on one must never unwind — it would close layers on a Forward. | +| Reopening re-arms the guard on the spent entry in place | The traversal already made the entry current; planting another would truncate the remaining forward stack and stack junk entries. | +| Sibling releases consume entries one traversal at a time, not one `history.go(-n)` | Entries below the current one are opaque, so a multi-step jump could cross navigation this package doesn't own; chaining single pops — each landing continuing the chain — stops at the first entry that isn't ours to spend. Call order still can't matter: the pending set is order-free. | +| Built on the History API, not the Navigation API | The Navigation API answers natively what this module reconstructs — whose traversal it was and which direction it ran — dissolving the self-caused-pop counting and the direction inference. It is not cross-browser yet (Chromium ships it; Safari and Firefox don't fully); once it is, this module should be rebuilt on it. | diff --git a/packages/dom/utils/navigation/src/intercept-back-navigation.ts b/packages/dom/utils/navigation/src/intercept-back-navigation.ts index 6c3101b..e62a777 100644 --- a/packages/dom/utils/navigation/src/intercept-back-navigation.ts +++ b/packages/dom/utils/navigation/src/intercept-back-navigation.ts @@ -27,11 +27,6 @@ interface ClaimWatcher { 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 */ @@ -40,22 +35,67 @@ export interface ReleaseOptions { // One shared registry + one popstate listener: a Back pops exactly one entry, // so only the guard whose entry vanished answers — stacked layers unwind one -// per press with no cross-layer bookkeeping. -const guards: BackGuard[] = [] -// Guards whose entry a Back press popped, kept so the host's Forward can -// reopen the layer. Parked entries always sit above every armed one. -const parked: BackGuard[] = [] -let nextGuardId = 0 -// Pops this module caused itself — counted so they are never read as a user's -// Back and unwind another layer. -let swallow = 0 -// Entries whose guards released but whose consumption hasn't happened yet. -// Sibling releases from one turn all land here; consumption then chains one -// traversal at a time (each pop surfaces the next spent entry) instead of one -// history.go(-n) jump, because entries below the current one are opaque — a -// multi-step jump could cross navigation this module doesn't own. -const pendingConsumption = new Set() -let consumptionScheduled = false +// per press with no cross-layer bookkeeping. The registry is anchored on a +// realm-global keyed by `Symbol.for` rather than module-level variables: a +// monorepo or micro-frontend can load more than one copy of this module into +// the same page, and forked registries sharing the one real session history +// would answer pops from the wrong copy (the same duplicate-singleton class +// of bug as Radix's focus-scope stack, radix-ui/primitives#2815). Resolved +// lazily on first use so the module keeps its `sideEffects: false` contract. +const STORE_KEY = Symbol.for('@dunky.dev/browser-navigation#navigation-store') + +interface NavigationStore { + guards: BackGuard[] + // Guards whose entry a Back press popped, kept so the host's Forward can + // reopen the layer. Parked entries always sit above every armed one. + parked: BackGuard[] + 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. + abandoned: Set + // 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. + pendingConsumption: Set + nextGuardId: number + // Pops this module caused itself — counted so they are never read as a + // user's Back and unwind another layer. + swallow: number + consumptionScheduled: boolean + // The attached popstate listener. Each module copy has its own listener + // function, so the browser's (type, listener) dedupe can't span copies — + // the store remembers the attached one instead. Any copy's listener + // behaves identically: all state lives here. + listener?: () => void +} + +function getStore(): NavigationStore { + const scope = globalThis as unknown as Record + let store = scope[STORE_KEY] + if (store === undefined) { + store = { + guards: [], + parked: [], + watchers: [], + abandoned: new Set(), + pendingConsumption: new Set(), + nextGuardId: 0, + swallow: 0, + consumptionScheduled: false, + } + scope[STORE_KEY] = store + } + return store +} + +function attachListener(store: NavigationStore): void { + if (store.listener !== undefined) return + store.listener = onPopState + window.addEventListener('popstate', onPopState) +} function currentGuardId(): number | undefined { const state: unknown = history.state @@ -74,13 +114,13 @@ function currentClaim(): string | 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 resolveClaim(): void { +function resolveClaim(store: NavigationStore): void { const id = currentGuardId() - if (id !== undefined && abandoned.has(id)) return + if (id !== undefined && store.abandoned.has(id)) return const claim = currentClaim() if (claim === undefined) return let candidate: ClaimWatcher | undefined - for (const watcher of watchers) { + for (const watcher of store.watchers) { if (watcher.claim !== claim) continue if (candidate !== undefined) return candidate = watcher @@ -88,31 +128,31 @@ function resolveClaim(): void { candidate?.reopen() } -function isArmed(id: number): boolean { - for (const guard of guards) if (guard.id === id) return true +function isArmed(store: NavigationStore, id: number): boolean { + for (const guard of store.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 +function parkedIndex(store: NavigationStore, id: number): number { + for (let index = 0; index < store.parked.length; index++) { + if ((store.parked[index] as BackGuard).id === id) return index } return -1 } // Every planted entry truncates the forward stack, taking every parked entry // with it — the guards watching them have nothing left to hear. -function plantEntry(guard: BackGuard): void { - parked.length = 0 +function plantEntry(store: NavigationStore, guard: BackGuard): void { + store.parked.length = 0 history.pushState({ [STATE_KEY]: guard.id, [CLAIM_KEY]: guard.claim }, '') } // Consume the current entry if its guard released: one history.back() whose // pop re-enters onPopState and continues the chain from there. -function consumeCurrentIfPending(): void { +function consumeCurrentIfPending(store: NavigationStore): void { const current = currentGuardId() - if (current !== undefined && pendingConsumption.delete(current)) { - swallow++ + if (current !== undefined && store.pendingConsumption.delete(current)) { + store.swallow++ history.back() } } @@ -121,32 +161,36 @@ function consumeCurrentIfPending(): void { // 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 { +function detachWhenIdle(store: NavigationStore): void { if ( - guards.length === 0 && - parked.length === 0 && - watchers.length === 0 && - swallow === 0 && - !consumptionScheduled + store.guards.length === 0 && + store.parked.length === 0 && + store.watchers.length === 0 && + store.swallow === 0 && + !store.consumptionScheduled ) { - pendingConsumption.clear() - window.removeEventListener('popstate', onPopState) + store.pendingConsumption.clear() + if (store.listener !== undefined) { + window.removeEventListener('popstate', store.listener) + store.listener = undefined + } } } function onPopState(): void { - if (swallow > 0) { - swallow-- + const store = getStore() + if (store.swallow > 0) { + store.swallow-- // Self-heal: if our own pop consumed an entry a live guard still needs // (it adopted the entry while the traversal was in flight), re-arm it. - const top = guards[guards.length - 1] + const top = store.guards[store.guards.length - 1] if (top !== undefined && top.id !== currentGuardId()) { - plantEntry(top) + plantEntry(store, top) } else { // The pop may have surfaced the next spent sibling entry — continue. - consumeCurrentIfPending() + consumeCurrentIfPending(store) } - detachWhenIdle() + detachWhenIdle(store) return } const current = currentGuardId() @@ -156,28 +200,28 @@ function onPopState(): void { // 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 (current !== undefined && !isArmed(store, current) && !store.pendingConsumption.has(current)) { + const landed = parkedIndex(store, current) if (landed !== -1) { - for (let index = parked.length - 1; index >= landed; index--) { - const guard = parked[index] as BackGuard + for (let index = store.parked.length - 1; index >= landed; index--) { + const guard = store.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) + store.parked.splice(index, 1) + store.guards.push(guard) } } } else { - resolveClaim() + resolveClaim(store) } - detachWhenIdle() + detachWhenIdle(store) return } // Unwind every guard the traversal jumped over, topmost first — a Back // press covers one; a multi-entry jump (history.go(-n)) covers several. - while (guards.length > 0) { - const top = guards[guards.length - 1] as BackGuard + while (store.guards.length > 0) { + const top = store.guards[store.guards.length - 1] as BackGuard if (top.id === current) break let closed = false try { @@ -186,23 +230,23 @@ function onPopState(): void { // 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) plantEntry(store, 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) + const index = store.guards.indexOf(top) if (index !== -1) { - guards.splice(index, 1) + store.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) + if (top.onForward !== undefined) store.parked.push(top) } } // 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() + consumeCurrentIfPending(store) + detachWhenIdle(store) } /** @@ -223,62 +267,62 @@ export function interceptBackNavigation( onBack: () => boolean, options: BackNavigationOptions = {}, ): (releaseOptions?: ReleaseOptions) => void { + const store = getStore() const guard: BackGuard = { - id: ++nextGuardId, + id: ++store.nextGuardId, claim: options.claim, onBack, onForward: options.onForward, } - // Identical (type, listener) pairs dedupe, so attaching is idempotent. - window.addEventListener('popstate', onPopState) + attachListener(store) const current = currentGuardId() - guards.push(guard) - if (current !== undefined && !isArmed(current)) { + store.guards.push(guard) + if (current !== undefined && !isArmed(store, current)) { // Adoption steals the entry from a parked watcher too, and withdraws it // from any pending consumption — the ground now belongs to this // registration, so no traversal may spend it. - const stale = parkedIndex(current) - if (stale !== -1) parked.splice(stale, 1) - pendingConsumption.delete(current) + const stale = parkedIndex(store, current) + if (stale !== -1) store.parked.splice(stale, 1) + store.pendingConsumption.delete(current) history.replaceState({ [STATE_KEY]: guard.id, [CLAIM_KEY]: guard.claim }, '') } else { - plantEntry(guard) + plantEntry(store, guard) } return (releaseOptions: ReleaseOptions = {}) => { - if (releaseOptions.keepClaim !== true) abandoned.add(guard.id) + if (releaseOptions.keepClaim !== true) store.abandoned.add(guard.id) - const rest = parked.indexOf(guard) + const rest = store.parked.indexOf(guard) if (rest !== -1) { - parked.splice(rest, 1) + store.parked.splice(rest, 1) // A parked guard's entry sits in the forward stack — not the chain's to // spend — unless a declined reopen left it current: consume that one, // or it swallows the next Back. if (currentGuardId() === guard.id) { - pendingConsumption.add(guard.id) - scheduleConsumption() + store.pendingConsumption.add(guard.id) + scheduleConsumption(store) } else { - detachWhenIdle() + detachWhenIdle(store) } return } - const index = guards.indexOf(guard) + const index = store.guards.indexOf(guard) if (index === -1) return // already unwound by the Back press itself - guards.splice(index, 1) - pendingConsumption.add(guard.id) - scheduleConsumption() + store.guards.splice(index, 1) + store.pendingConsumption.add(guard.id) + scheduleConsumption(store) } } // 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 +function scheduleConsumption(store: NavigationStore): void { + if (store.consumptionScheduled) return + store.consumptionScheduled = true queueMicrotask(() => { - consumptionScheduled = false - consumeCurrentIfPending() - detachWhenIdle() + store.consumptionScheduled = false + consumeCurrentIfPending(store) + detachWhenIdle(store) }) } @@ -288,14 +332,14 @@ function scheduleConsumption(): void { * claim, neither answers. */ export function watchSpentEntry(claim: string, reopen: () => boolean): () => void { + const store = getStore() const watcher: ClaimWatcher = { claim, reopen } - // Identical (type, listener) pairs dedupe, so attaching is idempotent. - window.addEventListener('popstate', onPopState) - watchers.push(watcher) + attachListener(store) + store.watchers.push(watcher) return () => { - const index = watchers.indexOf(watcher) + const index = store.watchers.indexOf(watcher) if (index === -1) return - watchers.splice(index, 1) - detachWhenIdle() + store.watchers.splice(index, 1) + detachWhenIdle(store) } } 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 b4c1bfe..eba9cb7 100644 --- a/packages/dom/utils/navigation/tests/intercept-back-navigation.test.ts +++ b/packages/dom/utils/navigation/tests/intercept-back-navigation.test.ts @@ -321,3 +321,20 @@ describe('interceptBackNavigation', () => { await releaseAndSettle(releaseLower) }) }) + +describe('registry global anchoring', () => { + it('anchors its registry on the realm global so duplicate module copies share it', async () => { + // A second bundled copy of this module resolves the same registry through + // this well-known global symbol; forked registries sharing the one real + // session history would answer pops from the wrong copy. + const release = interceptBackNavigation(() => true) + + const store = (globalThis as unknown as Record)[ + Symbol.for('@dunky.dev/browser-navigation#navigation-store') + ] + expect(store?.guards).toHaveLength(1) + + await releaseAndSettle(release) + expect(store?.guards).toHaveLength(0) + }) +}) diff --git a/packages/dom/utils/overlay/src/hide-exiting-layer.ts b/packages/dom/utils/overlay/src/hide-exiting-layer.ts index 00a4f0e..af04208 100644 --- a/packages/dom/utils/overlay/src/hide-exiting-layer.ts +++ b/packages/dom/utils/overlay/src/hide-exiting-layer.ts @@ -1,3 +1,5 @@ +import { createHideTracker } from './hide-tracker' + /** * A closing overlay has already left the stack — the page beneath is live * again — but its layer keeps painting until the exit visual finishes. Take @@ -22,25 +24,9 @@ export function hideExitingLayer( // the content itself. if (root.parentElement === null) root = content - const targets: Element[] = [root] - if (backdrop != null && !root.contains(backdrop)) targets.push(backdrop) - - const hidden: Array<[Element, string | null]> = [] - for (const element of targets) { - // `aria-hidden="false"` asserts visible — the opposite of author-hidden — - // so only a truthy value counts as the author's. - const ariaHidden = element.getAttribute('aria-hidden') - if ((ariaHidden !== null && ariaHidden !== 'false') || element.hasAttribute('inert')) continue - element.setAttribute('aria-hidden', 'true') - element.setAttribute('inert', '') - hidden.push([element, ariaHidden]) - } + const tracker = createHideTracker() + tracker.hide(root) + if (backdrop != null && !root.contains(backdrop)) tracker.hide(backdrop) - return () => { - for (const [element, previousAriaHidden] of hidden) { - if (previousAriaHidden === null) element.removeAttribute('aria-hidden') - else element.setAttribute('aria-hidden', previousAriaHidden) - element.removeAttribute('inert') - } - } + return tracker.undo } diff --git a/packages/dom/utils/overlay/src/hide-outside.ts b/packages/dom/utils/overlay/src/hide-outside.ts index 7e38e8c..7850d7b 100644 --- a/packages/dom/utils/overlay/src/hide-outside.ts +++ b/packages/dom/utils/overlay/src/hide-outside.ts @@ -1,3 +1,5 @@ +import { createHideTracker } from './hide-tracker' + // Never hide these: they carry no rendered content, or must stay announced. const HIDE_SKIP = /^(SCRIPT|STYLE|LINK|TEMPLATE)$/ @@ -47,7 +49,7 @@ export function hideOutside(target: HTMLElement, exclude?: readonly Element[]): return false } - const hidden: Array<[Element, string | null]> = [] + const tracker = createHideTracker() function hideOutsideOf(parent: Element): void { for (const child of Array.from(parent.children)) { @@ -59,31 +61,14 @@ export function hideOutside(target: HTMLElement, exclude?: readonly Element[]): hideOutsideOf(child) continue } - // Skip 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 and the undo restores the authored value. - const ariaHidden = child.getAttribute('aria-hidden') - if ( - HIDE_SKIP.test(child.tagName) || - (ariaHidden !== null && ariaHidden !== 'false') || - child.hasAttribute('inert') - ) { - continue - } - child.setAttribute('aria-hidden', 'true') - child.setAttribute('inert', '') - hidden.push([child, ariaHidden]) + // Content-less tags never need hiding; author-hidden elements are the + // tracker's own skip. + if (HIDE_SKIP.test(child.tagName)) continue + tracker.hide(child) } } hideOutsideOf(document.body) - return () => { - for (const [element, previousAriaHidden] of hidden) { - if (previousAriaHidden === null) element.removeAttribute('aria-hidden') - else element.setAttribute('aria-hidden', previousAriaHidden) - element.removeAttribute('inert') - } - } + return tracker.undo } diff --git a/packages/dom/utils/overlay/src/hide-tracker.ts b/packages/dom/utils/overlay/src/hide-tracker.ts new file mode 100644 index 0000000..c68688d --- /dev/null +++ b/packages/dom/utils/overlay/src/hide-tracker.ts @@ -0,0 +1,34 @@ +// The one hide/undo bookkeeping shared by containment (`hideOutside`) and the +// exit window (`hideExitingLayer`): what counts as author-hidden and what the +// undo restores must not drift apart between the two. + +export interface HideTracker { + /** Marks the element `aria-hidden` + `inert` — unless the author already + * hides it: 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 and the undo restores the authored value.) */ + hide: (element: Element) => void + /** Removes exactly what `hide` added. */ + undo: () => void +} + +export function createHideTracker(): HideTracker { + const hidden: Array<[Element, string | null]> = [] + return { + hide(element) { + const ariaHidden = element.getAttribute('aria-hidden') + if ((ariaHidden !== null && ariaHidden !== 'false') || element.hasAttribute('inert')) return + element.setAttribute('aria-hidden', 'true') + element.setAttribute('inert', '') + hidden.push([element, ariaHidden]) + }, + undo() { + for (const [element, previousAriaHidden] of hidden) { + if (previousAriaHidden === null) element.removeAttribute('aria-hidden') + else element.setAttribute('aria-hidden', previousAriaHidden) + element.removeAttribute('inert') + } + hidden.length = 0 + }, + } +} diff --git a/packages/react/dialog/src/dialog.tsx b/packages/react/dialog/src/dialog.tsx index ad2d978..b80eb9a 100644 --- a/packages/react/dialog/src/dialog.tsx +++ b/packages/react/dialog/src/dialog.tsx @@ -6,7 +6,6 @@ import { useRef, type ComponentPropsWithoutRef, type ForwardRefExoticComponent, - type MouseEvent, type ReactNode, type RefAttributes, type RefObject, @@ -17,9 +16,9 @@ import { useScrollLock } from '@dunky.dev/react-use-scroll-lock' import type { DialogOptions } from '@dunky.dev/dialog' import { - acceptsBackdropPress, - acceptsViewportPress, dialogTrapOptions, + gateBackdropPress, + gateViewportPress, guardBackNavigation, openDialogLayer, startExitWindow, @@ -140,16 +139,10 @@ export const Backdrop: PartComponent = forw >((props, forwardedRef) => { const { api, machine, backdropRef } = useDialogContext() useImperativeHandle(forwardedRef, () => backdropRef.current as HTMLDivElement) - const { onClick, ...bindings } = normalize(api.parts.backdrop) as { - onClick?: (event: MouseEvent) => void - } & Record - - const merged = mergeProps(props, { - ...bindings, - onClick: (event: MouseEvent) => { - if (acceptsBackdropPress(machine.context.id)) onClick?.(event) - }, - }) + const merged = mergeProps( + props, + gateBackdropPress(machine.context.id, normalize(api.parts.backdrop)), + ) // Only a modal dialog dims the page — non-modal coexists with it. if (!machine.context.modal) return null @@ -168,16 +161,10 @@ export const Viewport: PartComponent = forw DialogViewportProps >((props, forwardedRef) => { const { api, machine } = useDialogContext() - const { onClick, ...bindings } = normalize(api.parts.viewport) as { - onClick?: (event: MouseEvent) => void - } & Record - - const merged = mergeProps(props, { - ...bindings, - onClick: (event: MouseEvent) => { - if (acceptsViewportPress(machine.context.id, event)) onClick?.(event) - }, - }) + const merged = mergeProps( + props, + gateViewportPress(machine.context.id, normalize(api.parts.viewport)), + ) return
}) diff --git a/packages/solid/dialog/src/dialog.tsx b/packages/solid/dialog/src/dialog.tsx index 6be3851..d5be6aa 100644 --- a/packages/solid/dialog/src/dialog.tsx +++ b/packages/solid/dialog/src/dialog.tsx @@ -15,9 +15,9 @@ import { useScrollLock } from '@dunky.dev/solid-use-scroll-lock' import type { DialogOptions } from '@dunky.dev/dialog' import { - acceptsBackdropPress, - acceptsViewportPress, dialogTrapOptions, + gateBackdropPress, + gateViewportPress, guardBackNavigation, openDialogLayer, startExitWindow, @@ -149,23 +149,14 @@ export const Backdrop: Component = props => { const rest = omit(props, 'ref', 'children') onSettled(() => () => (backdropRef.current = null)) - const bindings = (): Record => { - const { onClick, ...attrs } = normalize(api.parts.backdrop) as { - onClick?: (event: MouseEvent) => void - } & Record - return { - ...attrs, - onClick: (event: MouseEvent) => { - if (acceptsBackdropPress(machine.context.id)) onClick?.(event) - }, - } - } - return ( // Only a modal dialog dims the page — non-modal coexists with it.
(rest, bindings())} + {...mergeProps( + rest, + gateBackdropPress(machine.context.id, normalize(api.parts.backdrop)), + )} ref={element => { backdropRef.current = element applyConsumerRef(props.ref, element) @@ -187,19 +178,16 @@ export const Viewport: Component = props => { const { api, machine } = useDialogContext() const rest = omit(props, 'children') - const bindings = (): Record => { - const { onClick, ...attrs } = normalize(api.parts.viewport) as { - onClick?: (event: MouseEvent) => void - } & Record - return { - ...attrs, - onClick: (event: MouseEvent) => { - if (acceptsViewportPress(machine.context.id, event)) onClick?.(event) - }, - } - } - - return
(rest, bindings())}>{props.children}
+ return ( +
( + rest, + gateViewportPress(machine.context.id, normalize(api.parts.viewport)), + )} + > + {props.children} +
+ ) } // =============================================================================