Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changeset/dialog-press-gates.md
Original file line number Diff line number Diff line change
@@ -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
<div {...mergeProps(props, gateBackdropPress(machine.context.id, normalize(api.parts.backdrop)))} />
```

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.
14 changes: 14 additions & 0 deletions .changeset/navigation-shared-registry.md
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 8 additions & 0 deletions .changeset/overlay-below-reuses-ordered.md
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 11 additions & 0 deletions .changeset/overlay-hide-tracker.md
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 11 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 5 additions & 10 deletions packages/core/utils/overlay/src/layer-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,11 @@ export function createLayerStack<T extends OverlayLayer>(): LayerStack<T> {
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)
},
}
}
15 changes: 9 additions & 6 deletions packages/dom/components/dialog/SPEC.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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
Expand All @@ -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. |
2 changes: 1 addition & 1 deletion packages/dom/components/dialog/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
48 changes: 35 additions & 13 deletions packages/dom/components/dialog/src/press.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown>,
accepts: (event: PressTarget) => boolean,
): Record<string, unknown> {
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<string, unknown>,
): Record<string, unknown> {
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<string, unknown>,
): Record<string, unknown> {
return gate(bindings, event => event.target === event.currentTarget && isTopmostLayer(id))
}
29 changes: 22 additions & 7 deletions packages/dom/components/dialog/tests/dialog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
})
})

Expand Down
Loading
Loading