fix(dom-dialog): one Escape press, one answer

.changeset/dialog-escape-once-per-press.md

@@ -0,0 +1,34 @@
+---
+'@dunky.dev/dom-dialog': patch
+'@dunky.dev/react-dialog': patch
+'@dunky.dev/solid-dialog': patch
+---
+
+One Escape press closes at most one dialog — in a real browser too.
+
+Each open dialog listens for Escape on the document and answers only while it
+is the topmost layer. But a browser runs a microtask checkpoint between the
+listeners of a user's key press, and that is where React, Solid, and Svelte
+flush state: the dialog that answered had already left the stack when the next
+dialog's listener asked whether it was topmost now — and it answered the same
+press. One Escape closed a nested dialog together with the one beneath it, or
+ran the outer dialog's `onEscapeKeyDown` guard for the inner dialog's press.
+It took a lower dialog's listener running after the upper one's: a nested
+stack that mounts open in one render (React runs a child's effects first), a
+sibling dialog opened from inside another, or an `onEscapeKeyDown` that
+changes identity after a nested dialog opened. Script-dispatched events run no
+such checkpoint, so tests never saw it.
+
+The dialog that takes a press now marks it answered, and every other dialog
+stands down for it, whatever the answer: a close, a veto from
+`onEscapeKeyDown`, a `closeOnEscape={false}` gate, or a controlled dialog's
+intent that its consumer follows on the next render. A stack-scoped Escape
+(`escapeScope: 'stack'`) still unwinds every layer, once. Copies of the
+package loaded side by side on one page honor each other's answers, from this
+version on. The dialog never calls `preventDefault()` on the event — that
+remains the consumer's veto.
+
+A press another handler already consumed — its default prevented, as popup
+libraries do with the Escape they close on — no longer reaches
+`onEscapeKeyDown` either. It could never close the dialog, but it still ran the
+consumer's guard for a press that belonged to the popup.

packages/dom/components/dialog/SPEC.md

@@ -44,6 +44,18 @@ 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.
 
+A press is answered once, by the dialog that is topmost when it arrives. Each
+dialog has its own listener and asks the stack only when that listener runs —
+and a browser runs a microtask checkpoint between the listeners of a user's key
+press, where the binding flushes the close the previous listener made, so the
+dialog beneath may already be topmost. The dialog that takes a press therefore
+marks it answered, and every other dialog's listener stands down for it,
+whatever the answer turns out to be: a close, a veto, a `closeOnEscape` gate, or
+an intent a controlled consumer has yet to follow. A press whose default another
+handler already prevented — what popup libraries do with the Escape they close
+on — counts as answered too. Copies of this package loaded side by side share
+the mark, from this version on.
+
 A popup inside the dialog that never joined the stack — a listbox or menu from
 another library, a control whose popup is expanded — answers before the dialog
 while it holds focus: the listener stands down until focus is back in the
@@ -177,4 +189,6 @@ still in the window, and Tab is how such a popup is left.
 | 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 press is marked answered when a dialog takes it, not on outcome | A controlled dialog's close lands only when its consumer re-renders — on the very checkpoint that lets the next listener in — so waiting for the outcome would hand that dialog's press to the one beneath.                                                     |
+| The mark is a realm-global `WeakSet`, not `preventDefault`        | The overlay store's idiom: duplicate copies of the package rendezvous on one record. `preventDefault` stays the consumer's veto, and the dialog has no reason to cancel the browser's own handling of the key.                                                  |
 | 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. |

packages/dom/components/dialog/src/effects.ts

@@ -6,12 +6,35 @@ import {
   layersBelow,
 } from '@dunky.dev/dom-overlay'
 
+// The Escape presses a dialog already answered: one press, one answer. Each
+// dialog asks the stack only when its own listener runs, and a user's key
+// press runs a microtask checkpoint between listeners — where the binding
+// flushes the close the previous listener made, so the dialog beneath can be
+// topmost by then. Realm-global, like the overlay store, so a duplicate copy
+// of this package honors it too.
+const ANSWERED_KEY = Symbol.for('@dunky.dev/dom-dialog#answered-escapes')
+
+function answeredEscapes(): WeakSet<Event> {
+  const scope = globalThis as unknown as Record<symbol, WeakSet<Event> | undefined>
+  let answered = scope[ANSWERED_KEY]
+  if (answered === undefined) {
+    answered = new WeakSet()
+    scope[ANSWERED_KEY] = answered
+  }
+  return answered
+}
+
 // Escape is a document-level concern, not a part's — it must work wherever
 // focus is.
 const trackEscape: DialogEffect = [
   (machine, props) => {
     const onKeyDown = (event: KeyboardEvent): void => {
       if (event.key !== 'Escape' || !machine.matches('open')) return
+      // Another dialog took this press, or another handler already consumed
+      // it — preventing its default is what popup libraries do with the
+      // Escape they close on.
+      const answered = answeredEscapes()
+      if (answered.has(event) || event.defaultPrevented) return
       // Only the topmost dialog answers Escape — a nested stack closes one
       // layer at a time, unless this dialog's scope is the whole stack.
       const { id } = machine.context
@@ -20,6 +43,9 @@ const trackEscape: DialogEffect = [
       // while it holds focus — this listener runs in the capture phase, so
       // it would otherwise close the dialog under the popup.
       if (foreignPopupHoldsFocus(id) || expandedPopupControlHoldsFocus(id)) return
+      // The press is this dialog's now, whatever comes of it: a close, a veto,
+      // a gate, or an intent a controlled consumer has yet to follow.
+      answered.add(event)
       props.onEscapeKeyDown?.(event)
       if (event.defaultPrevented) return
       // Read the stack before the send: closing this layer releases it, and

packages/dom/components/dialog/tests/dialog.test.ts

@@ -1,6 +1,6 @@
 // @vitest-environment jsdom
 // The DOM half of the Dialog, driven directly — no substrate, no framework.
-import { afterEach, describe, expect, it, vi } from 'vitest'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import { machine, type Machine } from '@dunky.dev/state-machine'
 import { dialogMachine } from '@dunky.dev/dialog'
 import type {
@@ -33,10 +33,12 @@ const build = (options: DialogOptions = {}): DialogService => {
 // disposed after the test: the listener is document-level, so one left over
 // — a vetoing one above all — would answer the next test's Escape too.
 const armed: (() => void)[] = []
-const armEscape = (service: DialogService, props: DialogOptions = {}): (() => void) => {
-  const [effect] = domDialogEffects[
-    domDialogEffects.length - 1
-  ] as (typeof domDialogEffects)[number]
+const armEscape = (
+  service: DialogService,
+  props: DialogOptions = {},
+  effects = domDialogEffects,
+): (() => void) => {
+  const [effect] = effects[effects.length - 1] as (typeof effects)[number]
   const dispose = effect(service, props) ?? ((): void => {})
   armed.push(dispose)
   return dispose
@@ -45,10 +47,82 @@ const armEscape = (service: DialogService, props: DialogOptions = {}): (() => vo
 const pressEscape = (): boolean =>
   document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', cancelable: true }))
 
+// A user's key press, as a browser delivers it: one listener at a time, with a
+// microtask checkpoint after each, where a binding flushes the close the
+// listener made. jsdom's dispatchEvent runs listeners back to back, so the
+// press drives the document's keydown listeners by hand, in the order added.
+const keydownListeners: EventListenerOrEventListenerObject[] = []
+const captureKeydownListeners = (): void => {
+  keydownListeners.length = 0
+  const add = document.addEventListener.bind(document)
+  const remove = document.removeEventListener.bind(document)
+  vi.spyOn(document, 'addEventListener').mockImplementation((type, listener, options) => {
+    if (type === 'keydown') keydownListeners.push(listener)
+    add(type, listener, options)
+  })
+  vi.spyOn(document, 'removeEventListener').mockImplementation((type, listener, options) => {
+    const index = keydownListeners.indexOf(listener)
+    if (type === 'keydown' && index !== -1) keydownListeners.splice(index, 1)
+    remove(type, listener, options)
+  })
+}
+
+const pressEscapeAsUser = async (): Promise<void> => {
+  const event = new KeyboardEvent('keydown', { key: 'Escape', cancelable: true })
+  for (const listener of keydownListeners.slice()) {
+    // A listener removed mid-press is skipped, as in a real dispatch.
+    if (!keydownListeners.includes(listener)) continue
+    if (typeof listener === 'function') listener(event)
+    else listener.handleEvent(event)
+    // The checkpoint drains every microtask, those queued while draining too
+    // (a re-render queuing the release); a task boundary is how a test gets
+    // that far.
+    await new Promise(resolve => setTimeout(resolve))
+  }
+}
+
 // The layer stack is a realm-global that outlives a test — every registration
 // has to be undone or the next test inherits a stale topmost.
 const registered: (() => void)[] = []
 
+// A dialog run the way a binding runs it: open, its window in the stack — and
+// out of it on the microtask after the machine leaves `open`, when the
+// framework flushes the change, never synchronously.
+const openLayer = (options: DialogOptions & { id: string }, depth: number): DialogService => {
+  const service = build({ defaultOpen: true, ...options })
+  const content = document.createElement('div')
+  document.body.append(content)
+  const release = registerLayer({
+    id: options.id,
+    depth,
+    element: content,
+    modal: true,
+    backdrop: () => null,
+    dismiss: () => service.send({ type: 'close' }),
+  })
+  registered.push(release)
+  const stop = service.subscribe(() => {
+    if (service.matches('open')) return
+    stop()
+    queueMicrotask(release)
+  })
+  return service
+}
+
+// A stack of dialogs, bottom first, whose Escape listeners are armed in
+// `order` — the order a browser runs them in.
+type ListenerOrder = 'top-first' | 'bottom-first'
+const openStack = (
+  order: ListenerOrder,
+  ...dialogs: Array<DialogOptions & { id: string }>
+): DialogService[] => {
+  const stack = dialogs.map((options, index) => openLayer(options, index + 1))
+  const arming = [...stack.keys()]
+  if (order === 'top-first') arming.reverse()
+  for (const index of arming) armEscape(stack[index], dialogs[index])
+  return stack
+}
+
 // A layer, mounted and registered, standing in for a rendered dialog window.
 const mountLayer = (id: string, depth: number, html = '', dismiss?: () => void): HTMLElement => {
   const content = document.createElement('div')
@@ -88,38 +162,33 @@ describe('domDialogEffects — Escape', () => {
     expect(service.matches('open')).toBe(true)
   })
 
-  it('is ignored by a dialog that is not topmost — one layer per press', () => {
-    const service = build({ defaultOpen: true })
-    mountLayer('dlg', 1)
-    mountLayer('above', 2)
-    armEscape(service)
-
-    pressEscape()
-    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', () => {
+  // escapeScope: 'stack' — the receiving dialog gates and vetoes, and only an
+  // Escape it allowed gives the layers beneath their plain close.
+  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)
+    armEscape(upper, { onEscapeKeyDown: event => event.preventDefault?.() })
 
     pressEscape()
-    expect([upper.matches('open'), lower.matches('open')]).toEqual([false, false])
+    expect([upper.matches('open'), lower.matches('open')]).toEqual([true, true])
   })
 
-  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?.() })
+  // A popup library prevents the default of the Escape it closes on; the press
+  // was the popup's, so it never reaches the consumer's guard.
+  it('stands down for a press another handler already consumed', () => {
+    const onEscapeKeyDown = vi.fn()
+    const service = build({ defaultOpen: true })
+    mountLayer('dlg', 1)
+    const consume = (event: KeyboardEvent): void => event.preventDefault()
+    window.addEventListener('keydown', consume, true)
+    armed.push(() => window.removeEventListener('keydown', consume, true))
+    armEscape(service, { onEscapeKeyDown })
 
     pressEscape()
-    expect([upper.matches('open'), lower.matches('open')]).toEqual([true, true])
+    expect(service.matches('open')).toBe(true)
+    expect(onEscapeKeyDown).not.toHaveBeenCalled()
   })
 
   it('detaches its listener on dispose', () => {
@@ -181,6 +250,110 @@ describe('domDialogEffects — Escape', () => {
   })
 })
 
+// One press, one answer — from the dialog topmost when it arrived, however
+// soon a closed dialog leaves the stack.
+describe('domDialogEffects — one answer per Escape press', () => {
+  beforeEach(captureKeydownListeners)
+
+  // Top-first from here on: the topmost dialog's listener runs first, so its
+  // close has flushed by the time the one beneath asks who is topmost.
+  it('closes only the topmost dialog — the one beneath never hears the press', async () => {
+    const lowerEscape = vi.fn()
+    const [lower, upper] = openStack(
+      'top-first',
+      { id: 'lower', onEscapeKeyDown: lowerEscape },
+      { id: 'upper' },
+    )
+
+    await pressEscapeAsUser()
+    expect([upper.matches('open'), lower.matches('open')]).toEqual([false, true])
+    expect(lowerEscape).not.toHaveBeenCalled()
+  })
+
+  // A confirm-then-close guard: it vetoes the dismissal and closes its dialog
+  // itself, which leaves the stack all the same.
+  it('ends a vetoed press at the topmost dialog', async () => {
+    const lowerEscape = vi.fn()
+    const [lower, upper] = openStack(
+      'top-first',
+      { id: 'lower', onEscapeKeyDown: lowerEscape },
+      {
+        id: 'upper',
+        onEscapeKeyDown: event => {
+          event.preventDefault?.()
+          upper.send({ type: 'close' })
+        },
+      },
+    )
+
+    await pressEscapeAsUser()
+    expect(lower.matches('open')).toBe(true)
+    expect(lowerEscape).not.toHaveBeenCalled()
+  })
+
+  // A controlled dialog only records the intent; it closes when its consumer
+  // echoes `open={false}` back — on the checkpoint, as a re-render would.
+  it('ends the press at a controlled topmost dialog, before its consumer follows', async () => {
+    const lowerEscape = vi.fn()
+    const [lower, upper] = openStack(
+      'top-first',
+      { id: 'lower', onEscapeKeyDown: lowerEscape },
+      {
+        id: 'upper',
+        open: true,
+        onEscapeKeyDown: () =>
+          queueMicrotask(() => upper.send({ type: 'controlled.sync', value: false })),
+      },
+    )
+
+    await pressEscapeAsUser()
+    expect([upper.matches('open'), lower.matches('open')]).toEqual([false, true])
+    expect(lowerEscape).not.toHaveBeenCalled()
+  })
+
+  // The layers beneath get a plain close, never the press itself.
+  it('unwinds a stack-scoped press through every layer, once', async () => {
+    const beneathEscape = vi.fn()
+    const stack = openStack(
+      'top-first',
+      { id: 'bottom', onEscapeKeyDown: beneathEscape },
+      { id: 'middle', onEscapeKeyDown: beneathEscape },
+      { id: 'top', escapeScope: 'stack' },
+    )
+
+    await pressEscapeAsUser()
+    expect(stack.map(dialog => dialog.matches('open'))).toEqual([false, false, false])
+    expect(beneathEscape).not.toHaveBeenCalled()
+  })
+
+  // Only the dialog that takes the press marks it: a lower dialog that hears
+  // it first, before anything closed, must leave it to the topmost.
+  it('leaves the press to the topmost dialog when the one beneath hears it first', async () => {
+    const [lower, upper] = openStack('bottom-first', { id: 'lower' }, { id: 'upper' })
+
+    await pressEscapeAsUser()
+    expect([upper.matches('open'), lower.matches('open')]).toEqual([false, true])
+  })
+})
+
+describe('domDialogEffects — a duplicate copy of the package', () => {
+  beforeEach(captureKeydownListeners)
+
+  // A monorepo or micro-frontend can load the package twice: a press one copy
+  // answered must count as answered for the other too.
+  it('honors a press another copy of the package already answered', async () => {
+    vi.resetModules()
+    const copy = await import('@dunky.dev/dom-dialog')
+    const lower = openLayer({ id: 'lower' }, 1)
+    const upper = openLayer({ id: 'upper' }, 2)
+    armEscape(upper)
+    armEscape(lower, {}, copy.domDialogEffects)
+
+    await pressEscapeAsUser()
+    expect([upper.matches('open'), lower.matches('open')]).toEqual([false, true])
+  })
+})
+
 describe('openDialogLayer', () => {
   const options = { id: 'dlg', depth: 1, modal: true, backdrop: () => null }