From da982c0520c64d092be9aa87b4fc5b70a2852842 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Fri, 4 Sep 2026 17:08:21 +0200 Subject: [PATCH 1/4] feat(dialog): a popup inside the dialog owns Tab and Escape while it holds focus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A select menu, popover, menu, or combobox list inside a modal dialog never joins the layer stack, so the dialog kept treating itself as topmost: its capture-phase Escape closed the dialog under the popup, and its trap cancelled Tab inside the popup and pulled focus back into the window. - dom-overlay: `foreignPopupHoldsFocus(id)` and `expandedPopupControlHoldsFocus(id)` read the real keyboard owner from ARIA — the popup roles (aria-haspopup's values) and an expanded control with aria-haspopup — telling a registered layer from a popup that never registered. No cooperation from the popup is needed. - dom-dialog: the Escape listener stands down for either; the trap stands down for a foreign popup only (Tab is how an expanded control's popup is left). - SPECs: core behavior contract, dom-dialog, dom-overlay. Stories: a hand-rolled listbox inside the dialog, mirrored in React and Solid. Co-Authored-By: Claude Fable 5.1 --- .changeset/dialog-inner-popups.md | 15 ++ .changeset/overlay-popup-focus.md | 19 +++ packages/core/dialog/SPEC.md | 9 +- packages/dom/components/dialog/SPEC.md | 13 +- packages/dom/components/dialog/src/effects.ts | 14 +- .../dom/components/dialog/src/focus-trap.ts | 10 +- .../components/dialog/tests/dialog.test.ts | 45 ++++++ packages/dom/utils/overlay/SPEC.md | 26 ++++ packages/dom/utils/overlay/src/index.ts | 1 + packages/dom/utils/overlay/src/popup-focus.ts | 46 ++++++ packages/dom/utils/overlay/src/stack.ts | 16 +++ .../utils/overlay/tests/popup-focus.test.ts | 133 ++++++++++++++++++ .../react/dialog/stories/dialog.stories.tsx | 111 ++++++++++++++- .../solid/dialog/stories/dialog.stories.tsx | 113 ++++++++++++++- 14 files changed, 562 insertions(+), 9 deletions(-) create mode 100644 .changeset/dialog-inner-popups.md create mode 100644 .changeset/overlay-popup-focus.md create mode 100644 packages/dom/utils/overlay/src/popup-focus.ts create mode 100644 packages/dom/utils/overlay/tests/popup-focus.test.ts diff --git a/.changeset/dialog-inner-popups.md b/.changeset/dialog-inner-popups.md new file mode 100644 index 0000000..a642f33 --- /dev/null +++ b/.changeset/dialog-inner-popups.md @@ -0,0 +1,15 @@ +--- +'@dunky.dev/dom-dialog': patch +--- + +A popup opened inside the dialog owns Tab and Escape while it holds focus. + +A select menu, a popover, a menu, or a combobox list inside a modal dialog — +from any library, as long as it carries ARIA popup semantics — is not a layer +the stack knows, so the dialog kept treating itself as topmost: its +capture-phase Escape closed the dialog together with the popup, and its focus +trap cancelled Tab inside the popup and pulled focus back into the window. Both +now stand down while such a popup holds focus and resume once focus is back in +the window, so one Escape closes the popup and the next reaches the dialog. A +control whose popup is expanded (a combobox input) hands over Escape only; Tab +is how its popup is left, so the trap keeps it. diff --git a/.changeset/overlay-popup-focus.md b/.changeset/overlay-popup-focus.md new file mode 100644 index 0000000..777f9f4 --- /dev/null +++ b/.changeset/overlay-popup-focus.md @@ -0,0 +1,19 @@ +--- +'@dunky.dev/dom-overlay': minor +--- + +Two queries tell a layer when a popup it cannot see owns the keyboard: +`foreignPopupHoldsFocus(id)` and `expandedPopupControlHoldsFocus(id)`. + +A popup can hold focus inside a layer without ever registering in the stack — +a third-party listbox or menu, a combobox's list. The stack still names the +layer topmost, so the layer would keep answering Escape and trapping Tab under +the popup. The queries read the real owner from ARIA instead: focus in an +element with a popup role (`aria-haspopup`'s values) that is neither the +layer's window nor a registered layer, or on a control inside the window whose +popup is expanded (`aria-expanded` with `aria-haspopup`). No cooperation from +the popup is required. + +```ts +enabled: () => isTopmostLayer(id) && !foreignPopupHoldsFocus(id) +``` diff --git a/packages/core/dialog/SPEC.md b/packages/core/dialog/SPEC.md index 2ca4129..57190d7 100644 --- a/packages/core/dialog/SPEC.md +++ b/packages/core/dialog/SPEC.md @@ -150,6 +150,12 @@ Per the [APG modal-dialog keyboard interaction](https://www.w3.org/WAI/ARIA/apg/ always the cycle's last stop, wherever it renders — the dismissal affordance follows the content instead of interrupting it. Focus never tabs out of the dialog. +- **Inner popups**: a popup opened inside the dialog that is not itself a + dialog — a select menu, a popover, a menu, a combobox list, from this + library or any other — owns the keyboard while it holds focus: Tab stays in + it and Escape closes it alone. The dialog's trap and Escape resume once + focus is back in the window. The dialog recognizes the popup by its ARIA + popup semantics, so no cooperation from the popup is needed. - **No focusables**: Tab is a no-op; focus stays on the dialog window. - **On close**: focus returns to the element focused before opening (normally the Trigger). @@ -170,7 +176,8 @@ stack of dialogs only the topmost one exists until it closes. - **Escape**: lands only on the topmost dialog, subject to that dialog's own dismissal settings and veto. Its reach is that dialog's escape scope: one layer (the default — the stack unwinds one layer per press) or the whole - stack. + stack. An inner popup (select menu, popover, menu, combobox list) counts as + a layer of its own: one press closes it, the next reaches the dialog. - **Outside press**: pressing around the topmost dialog is an outside interaction for that dialog alone, following its own dismissal settings; the dialogs beneath are unaffected. diff --git a/packages/dom/components/dialog/SPEC.md b/packages/dom/components/dialog/SPEC.md index 89c39f4..95cacc2 100644 --- a/packages/dom/components/dialog/SPEC.md +++ b/packages/dom/components/dialog/SPEC.md @@ -44,6 +44,14 @@ 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 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 +window, so one press closes the popup and the next reaches the dialog. The +stack cannot name such a layer, so the answer is read from ARIA through the +overlay util's focus queries (`foreignPopupHoldsFocus`, +`expandedPopupControlHoldsFocus`). + How far an allowed Escape reaches is that dialog's `escapeScope`: itself, so a nested stack unwinds one dialog per press, or the whole stack at once. Either way the dialog that received the press is the only one that gates or vetoes @@ -129,7 +137,10 @@ dialog's to answer: `dialogTrapOptions` is the trap configuration the substrate hands to its `trapFocus` wrapper: a modal dialog traps while it is topmost, and the Close -part is the cycle's last stop wherever it renders. +part is the cycle's last stop wherever it renders. The trap stands down while +an unregistered popup inside the dialog holds focus — Tab is the popup's for +as long as it does. A control whose popup is expanded keeps the trap: focus is +still in the window, and Tab is how such a popup is left. ## API diff --git a/packages/dom/components/dialog/src/effects.ts b/packages/dom/components/dialog/src/effects.ts index 285b6d1..f225449 100644 --- a/packages/dom/components/dialog/src/effects.ts +++ b/packages/dom/components/dialog/src/effects.ts @@ -1,5 +1,10 @@ import { dialogEffects, type DialogEffect } from '@dunky.dev/dialog' -import { isTopmostLayer, layersBelow } from '@dunky.dev/dom-overlay' +import { + expandedPopupControlHoldsFocus, + foreignPopupHoldsFocus, + isTopmostLayer, + layersBelow, +} from '@dunky.dev/dom-overlay' // Escape is a document-level concern, not a part's — it must work wherever // focus is. @@ -9,7 +14,12 @@ const trackEscape: DialogEffect = [ if (event.key !== 'Escape' || !machine.matches('open')) 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. - if (!isTopmostLayer(machine.context.id)) return + const { id } = machine.context + if (!isTopmostLayer(id)) return + // A popup inside the dialog that never joined the stack answers first + // 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 props.onEscapeKeyDown?.(event) if (event.defaultPrevented) return // Read the stack before the send: closing this layer releases it, and diff --git a/packages/dom/components/dialog/src/focus-trap.ts b/packages/dom/components/dialog/src/focus-trap.ts index e5ab97f..3caea49 100644 --- a/packages/dom/components/dialog/src/focus-trap.ts +++ b/packages/dom/components/dialog/src/focus-trap.ts @@ -1,6 +1,6 @@ import type { DialogMachine } from '@dunky.dev/dialog' import type { TrapFocusOptions } from '@dunky.dev/dom-focus-trap' -import { isTopmostLayer } from '@dunky.dev/dom-overlay' +import { foreignPopupHoldsFocus, isTopmostLayer } from '@dunky.dev/dom-overlay' /** * The trap configuration for a dialog window, for whichever hook the substrate @@ -11,8 +11,12 @@ import { isTopmostLayer } from '@dunky.dev/dom-overlay' export function dialogTrapOptions(machine: DialogMachine, closeId: () => string): TrapFocusOptions { return { // Only a modal dialog traps, and only while topmost — a nested dialog - // owns focus while open. - enabled: () => machine.context.modal && isTopmostLayer(machine.context.id), + // owns focus while open, and so does a popup inside the dialog that holds + // it without having joined the stack. + enabled: () => + machine.context.modal && + isTopmostLayer(machine.context.id) && + !foreignPopupHoldsFocus(machine.context.id), // The Close part is the cycle's last stop wherever it renders (core // SPEC); found by its derived id. last: () => document.getElementById(closeId()), diff --git a/packages/dom/components/dialog/tests/dialog.test.ts b/packages/dom/components/dialog/tests/dialog.test.ts index 984516d..b864994 100644 --- a/packages/dom/components/dialog/tests/dialog.test.ts +++ b/packages/dom/components/dialog/tests/dialog.test.ts @@ -124,6 +124,36 @@ describe('domDialogEffects — Escape', () => { pressEscape() expect(service.matches('open')).toBe(true) }) + + // A popup inside the dialog owns Escape while it holds focus — read from + // ARIA, so a popup that never registers in the stack counts too. + it('stands down while a popup inside the dialog holds focus', () => { + const service = build({ defaultOpen: true }) + const content = mountLayer( + 'dlg', + 1, + '', + ) + ;(content.querySelector('#option') as HTMLElement).focus() + armEscape(service) + + pressEscape() + expect(service.matches('open')).toBe(true) + }) + + it('stands down while a control with an expanded popup holds focus', () => { + const service = build({ defaultOpen: true }) + const content = mountLayer( + 'dlg', + 1, + '', + ) + ;(content.querySelector('#combo') as HTMLElement).focus() + armEscape(service) + + pressEscape() + expect(service.matches('open')).toBe(true) + }) }) describe('openDialogLayer', () => { @@ -440,6 +470,21 @@ describe('dialogTrapOptions', () => { expect(enabled?.()).toBe(false) }) + it('stands down while a popup inside the dialog holds focus', () => { + const service = build({ defaultOpen: true }) + const content = mountLayer( + 'dlg', + 1, + '', + ) + const { enabled } = dialogTrapOptions(service, () => 'dlg-close') + + ;(content.querySelector('#option') as HTMLElement).focus() + expect(enabled?.()).toBe(false) + content.focus() + expect(enabled?.()).toBe(true) + }) + it('resolves Close as the cycle’s last stop, wherever it renders', () => { const service = build({ defaultOpen: true }) mountLayer('dlg', 1, '') diff --git a/packages/dom/utils/overlay/SPEC.md b/packages/dom/utils/overlay/SPEC.md index 139b251..ab364cb 100644 --- a/packages/dom/utils/overlay/SPEC.md +++ b/packages/dom/utils/overlay/SPEC.md @@ -54,6 +54,30 @@ against each other. - The backdrop is resolved through a getter, not a snapshot: a re-hide (a layer above closing) sees the element current at that moment. +### Popups the stack never sees + +A popup can hold focus inside a layer without registering — a listbox or menu +from another library, a combobox's list. The stack still names the layer +topmost, so the layer would keep answering Escape and trapping Tab under the +popup. Two queries read the real owner from ARIA instead, and a layer consults +them to stand down: + +- `foreignPopupHoldsFocus(id)` — focus sits in a popup that is neither the + layer's own window nor a registered layer: an element with a popup role + (`aria-haspopup`'s values — listbox, menu, tree, grid, dialog), wherever it + renders, or anything outside the window that is in no popup role at all — + the page is inert while a modal layer is open, so whatever holds focus out + there is a layer. Focus on the body doesn't count: the layer re-enters from + there. Registered layers never count as foreign — a layer beneath is inert, + and focus reported there re-enters the topmost layer's trap. +- `expandedPopupControlHoldsFocus(id)` — focus sits on a control inside the + window whose popup is expanded (`aria-expanded="true"` with `aria-haspopup`), + the way a combobox keeps focus on its input while its list is open. + `aria-expanded` alone is a disclosure, which has no popup to close. + +No cooperation from the popup is required — any well-formed ARIA popup works. +A popup that does register is simply topmost, and the stack answers as usual. + ### Initial focus The strict rule is only that focus moves into the overlay: an overlay that @@ -97,6 +121,8 @@ again — but keeps painting until its exit visual finishes: | `Layer` | `OverlayLayer` + `element`, `modal`, an optional `backdrop` getter, and an optional `dismiss`. | | `isTopmostLayer(id)` | Whether the layer owns Escape and the focus trap right now. | | `layersBelow(id)` | The layers stacked beneath, topmost first — the unwinding order for a stack-scoped dismissal. | +| `foreignPopupHoldsFocus(id)` | Whether focus sits in a popup that is neither the layer's window nor a registered layer. | +| `expandedPopupControlHoldsFocus(id)` | Whether focus sits on a control inside the layer's window whose popup is expanded. | | `getInitialFocus(content, designated?)` | The element to focus on open: `designated`, else first form field, else the overlay window — each step filtered for renderedness. | | `hideExitingLayer(content, boundary, backdrop?)` | Inerts the still-painting layer for the exit window; returns the undo. | | `watchExitAnimation(element, onComplete)` | Reports the exit visual's end once; returns the cancel. | diff --git a/packages/dom/utils/overlay/src/index.ts b/packages/dom/utils/overlay/src/index.ts index 1753d05..867b9f1 100644 --- a/packages/dom/utils/overlay/src/index.ts +++ b/packages/dom/utils/overlay/src/index.ts @@ -1,4 +1,5 @@ export { registerLayer, isTopmostLayer, layersBelow, type Layer } from './stack' +export { foreignPopupHoldsFocus, expandedPopupControlHoldsFocus } from './popup-focus' export { getInitialFocus } from './get-initial-focus' export { watchExitAnimation } from './watch-exit-animation' export { hideExitingLayer } from './hide-exiting-layer' diff --git a/packages/dom/utils/overlay/src/popup-focus.ts b/packages/dom/utils/overlay/src/popup-focus.ts new file mode 100644 index 0000000..42f8b2e --- /dev/null +++ b/packages/dom/utils/overlay/src/popup-focus.ts @@ -0,0 +1,46 @@ +import { getLayer, layerContaining } from './stack' + +// A popup can hold focus inside a layer without ever joining the stack — a +// listbox or menu from another library, a combobox's list — so the stack still +// names the layer topmost while the popup owns the keyboard. Ownership is read +// from ARIA instead: the popup roles (`aria-haspopup`'s values) mark the layer +// focus sits in, and the stack tells a registered layer from a popup that +// never registered. No cooperation is required — any well-formed ARIA popup +// works. +const POPUP_SELECTOR = + '[role="listbox"], [role="menu"], [role="tree"], [role="grid"], [role="dialog"], [role="alertdialog"]' + +/** + * Whether focus sits in a popup that is neither the layer's own window nor a + * registered layer — wherever it renders, inside the window or portalled + * beside it. Outside every registered window and in no popup role counts too: + * the page is inert while a modal layer is open, so whatever holds focus out + * there is a layer. The body (focus in browser chrome, or nowhere) doesn't: the + * layer re-enters from there. + */ +export function foreignPopupHoldsFocus(id: string): boolean { + const layer = getLayer(id) + if (layer === undefined) return false + const active = document.activeElement + if (active === null || active === document.body) return false + const popup = active.closest(POPUP_SELECTOR) + if (popup === null) return layerContaining(active) === undefined + return popup !== layer.element && layerContaining(popup)?.element !== popup +} + +/** + * Whether focus sits on a control inside the layer's window whose popup is + * expanded — a combobox keeps focus on its input while its list is open — so + * the popup owns Escape. `aria-expanded` alone isn't enough: a disclosure or + * accordion trigger is expanded too and has no popup to close, so + * `aria-haspopup` must name one. + */ +export function expandedPopupControlHoldsFocus(id: string): boolean { + const layer = getLayer(id) + if (layer === undefined) return false + const active = document.activeElement + if (active === null || !layer.element.contains(active)) return false + if (active.getAttribute('aria-expanded') !== 'true') return false + const popup = active.getAttribute('aria-haspopup') + return popup !== null && popup !== 'false' +} diff --git a/packages/dom/utils/overlay/src/stack.ts b/packages/dom/utils/overlay/src/stack.ts index c0c8a90..b4dcac8 100644 --- a/packages/dom/utils/overlay/src/stack.ts +++ b/packages/dom/utils/overlay/src/stack.ts @@ -102,3 +102,19 @@ export function isTopmostLayer(id: string): boolean { export function layersBelow(id: string): Layer[] { return getStore().stack.below(id) } + +export function getLayer(id: string): Layer | undefined { + for (const layer of getStore().stack.ordered()) { + if (layer.id === id) return layer + } + return undefined +} + +// The registered layer whose window holds `node` — what tells a sibling in +// the stack from a popup that never registered. +export function layerContaining(node: Node): Layer | undefined { + for (const layer of getStore().stack.ordered()) { + if (layer.element.contains(node)) return layer + } + return undefined +} diff --git a/packages/dom/utils/overlay/tests/popup-focus.test.ts b/packages/dom/utils/overlay/tests/popup-focus.test.ts new file mode 100644 index 0000000..3c349f4 --- /dev/null +++ b/packages/dom/utils/overlay/tests/popup-focus.test.ts @@ -0,0 +1,133 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it } from 'vitest' +import { + expandedPopupControlHoldsFocus, + foreignPopupHoldsFocus, + registerLayer, +} from '@dunky.dev/dom-overlay' + +const registered: Array<() => void> = [] + +// A registered layer window with the given markup inside it. +const mountLayer = (id: string, depth: number, html = ''): HTMLElement => { + const content = document.createElement('div') + content.tabIndex = -1 + content.innerHTML = html + document.body.append(content) + registered.push(registerLayer({ id, depth, element: content, modal: true })) + return content +} + +// Markup appended beside the layers, the way a portalled popup lands. +const mountBeside = (html: string): HTMLElement => { + const host = document.createElement('div') + host.innerHTML = html + document.body.append(host) + return host +} + +const focus = (root: ParentNode, selector: string): void => { + ;(root.querySelector(selector) as HTMLElement).focus() +} + +afterEach(() => { + for (const unregister of registered) unregister() + registered.length = 0 + document.body.innerHTML = '' +}) + +describe('foreignPopupHoldsFocus', () => { + it('is false while focus sits in the layer window itself', () => { + const content = mountLayer('dlg', 1, '') + focus(content, '#action') + + expect(foreignPopupHoldsFocus('dlg')).toBe(false) + }) + + it('is true while focus sits in a popup rendered inside the window', () => { + const content = mountLayer( + 'dlg', + 1, + '', + ) + focus(content, '#option') + + expect(foreignPopupHoldsFocus('dlg')).toBe(true) + }) + + it('is true while focus sits in a popup portalled beside the window', () => { + mountLayer('dlg', 1) + const beside = mountBeside('
') + focus(beside, '#inside') + + expect(foreignPopupHoldsFocus('dlg')).toBe(true) + }) + + // Outside the window and not in a popup role: the page is inert while a + // modal layer is open, so whatever holds focus out there is a layer. + it('is true while focus sits outside the window in an element with no popup role', () => { + mountLayer('dlg', 1) + const beside = mountBeside('
') + focus(beside, '#floating') + + expect(foreignPopupHoldsFocus('dlg')).toBe(true) + }) + + it('is false while focus sits in another registered layer', () => { + const lower = mountLayer('lower', 1, '') + mountLayer('upper', 2) + focus(lower, '#beneath') + + expect(foreignPopupHoldsFocus('upper')).toBe(false) + }) + + it('is false while focus sits on the body', () => { + mountLayer('dlg', 1) + ;(document.activeElement as HTMLElement | null)?.blur() + + expect(foreignPopupHoldsFocus('dlg')).toBe(false) + }) + + it('is false for an id the stack does not know', () => { + const beside = mountBeside('
') + focus(beside, '#item') + + expect(foreignPopupHoldsFocus('nobody')).toBe(false) + }) +}) + +describe('expandedPopupControlHoldsFocus', () => { + it('is true while focus sits on a control inside the window whose popup is expanded', () => { + const content = mountLayer( + 'dlg', + 1, + '', + ) + focus(content, '#combo') + + expect(expandedPopupControlHoldsFocus('dlg')).toBe(true) + }) + + it('is false once the popup collapses', () => { + const content = mountLayer( + 'dlg', + 1, + '', + ) + focus(content, '#combo') + + expect(expandedPopupControlHoldsFocus('dlg')).toBe(false) + }) + + // Expanded, but with no popup to close: a disclosure or accordion trigger. + it('is false for an expanded control without a popup', () => { + const content = mountLayer( + 'dlg', + 1, + '
', + ) + focus(content, '#disclosure') + + expect(expandedPopupControlHoldsFocus('dlg')).toBe(false) + }) +}) diff --git a/packages/react/dialog/stories/dialog.stories.tsx b/packages/react/dialog/stories/dialog.stories.tsx index aa022ed..750edd1 100644 --- a/packages/react/dialog/stories/dialog.stories.tsx +++ b/packages/react/dialog/stories/dialog.stories.tsx @@ -1,4 +1,4 @@ -import { useRef, useState, type CSSProperties } from 'react' +import { useEffect, useRef, useState, type CSSProperties } from 'react' import type { Meta, StoryObj } from '@storybook/react-vite' import { Dialog } from '@dunky.dev/react-dialog' @@ -69,6 +69,24 @@ const input: CSSProperties = { borderRadius: 6, font: 'inherit', } +const listbox: CSSProperties = { + position: 'absolute', + top: '100%', + left: 0, + minWidth: 220, + margin: '4px 0 0', + padding: 4, + listStyle: 'none', + background: 'white', + border: '1px solid #ccc', + borderRadius: 6, + boxShadow: '0 4px 16px rgba(0, 0, 0, 0.16)', +} +const option: CSSProperties = { + padding: '6px 10px', + borderRadius: 4, + cursor: 'pointer', +} // A scoped dialog opens inside a container instead of over the whole page: it // portals into that element, and its overlay layers switch from `fixed` // (viewport-pinned) to `absolute` (container-pinned). @@ -424,6 +442,97 @@ export const nested: StoryType = { render: () => , } +// A popup inside the dialog that never joins the layer stack — a third-party +// listbox stands in. While it holds focus, Tab and Escape are its: the +// dialog's trap and Escape stand down until focus is back in the window, so +// one Escape closes the listbox and the next closes the dialog. +const editors = ['Team members', 'Anyone with the link', 'Only me'] +const Listbox = () => { + const [open, setOpen] = useState(false) + const [value, setValue] = useState(editors[0]) + const buttonRef = useRef(null) + const listRef = useRef(null) + const close = () => { + setOpen(false) + buttonRef.current?.focus() + } + const pick = (editor: string) => { + setValue(editor) + close() + } + useEffect(() => { + if (open) listRef.current?.querySelector('[role="option"]')?.focus() + }, [open]) + return ( +
+ + {open && ( +
    { + if (event.key === 'Escape') close() + if (event.key === 'Tab') setOpen(false) + }} + > + {editors.map(editor => ( +
  • pick(editor)} + onKeyDown={event => { + if (event.key === 'Enter' || event.key === ' ') pick(editor) + }} + > + {editor} +
    • + ))} +
+ )} +
+ ) +} + +export const innerPopup: StoryType = { + render: () => ( + + Open dialog + + + + + + Board settings + + Open the listbox, then press Tab and Escape: the popup answers first, the dialog only + once it is closed. + + +
+ +
+ + + + + ), +} + // closeOnBack turns the host's Back into a dismissal: while the dialog is open, // a guard entry sits in the session history, so the browser's Back closes the // dialog instead of leaving the page — what mobile users expect from a diff --git a/packages/solid/dialog/stories/dialog.stories.tsx b/packages/solid/dialog/stories/dialog.stories.tsx index a1c71ba..b2cf3b9 100644 --- a/packages/solid/dialog/stories/dialog.stories.tsx +++ b/packages/solid/dialog/stories/dialog.stories.tsx @@ -1,4 +1,4 @@ -import { createSignal, Repeat } from 'solid-js' +import { createEffect, createSignal, For, Repeat, Show } from 'solid-js' import type { JSX } from '@solidjs/web' import type { Meta, StoryObj } from 'storybook-solidjs-vite' import { Dialog } from '@dunky.dev/solid-dialog' @@ -101,6 +101,25 @@ const scopedViewport: JSX.CSSProperties = { ...viewport, position: 'absolute' } // through state — see the alertDialog story. const closableContent: JSX.CSSProperties = { ...content, position: 'relative' } +const listbox: JSX.CSSProperties = { + position: 'absolute', + top: '100%', + left: '0', + 'min-width': '220px', + margin: '4px 0 0', + padding: '4px', + 'list-style': 'none', + background: 'white', + border: '1px solid #ccc', + 'border-radius': '6px', + 'box-shadow': '0 4px 16px rgba(0, 0, 0, 0.16)', +} +const option: JSX.CSSProperties = { + padding: '6px 10px', + 'border-radius': '4px', + cursor: 'pointer', +} + const CloseButton = () => ( × @@ -427,6 +446,98 @@ export const nested: StoryType = { render: () => , } +// A popup inside the dialog that never joins the layer stack — a third-party +// listbox stands in. While it holds focus, Tab and Escape are its: the +// dialog's trap and Escape stand down until focus is back in the window, so +// one Escape closes the listbox and the next closes the dialog. +const editors = ['Team members', 'Anyone with the link', 'Only me'] +const Listbox = () => { + const [open, setOpen] = createSignal(false) + const [value, setValue] = createSignal(editors[0]) + const [button, setButton] = createSignal() + const [list, setList] = createSignal() + const close = () => { + setOpen(false) + button()?.focus() + } + const pick = (editor: string) => { + setValue(editor) + close() + } + createEffect(() => { + if (open()) list()?.querySelector('[role="option"]')?.focus() + }) + return ( +
+ + +
    { + if (event.key === 'Escape') close() + if (event.key === 'Tab') setOpen(false) + }} + > + + {editor => ( +
  • pick(editor)} + onKeyDown={event => { + if (event.key === 'Enter' || event.key === ' ') pick(editor) + }} + > + {editor} +
    • + )} +     +
+ +
+ ) +} + +export const innerPopup: StoryType = { + render: () => ( + + Open dialog + + + + + + Board settings + + Open the listbox, then press Tab and Escape: the popup answers first, the dialog only + once it is closed. + + +
+ +
+ + + + + ), +} + // closeOnBack turns the host's Back into a dismissal: while the dialog is open, // a guard entry sits in the session history, so the browser's Back closes the // dialog instead of leaving the page — what mobile users expect from a From ff2616f74a62640292464cb4a70ab3588898f96f Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Sun, 4 Oct 2026 11:57:25 +0200 Subject: [PATCH 2/4] feat(dom-overlay): export POPUP_SELECTOR The selector naming the popup roles the focus queries read ownership from, so a consumer can match the same set the queries do. Co-Authored-By: Claude Fable 5.1 --- .changeset/overlay-popup-focus.md | 3 +++ packages/dom/utils/overlay/SPEC.md | 1 + packages/dom/utils/overlay/src/index.ts | 6 +++++- packages/dom/utils/overlay/src/popup-focus.ts | 2 +- 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.changeset/overlay-popup-focus.md b/.changeset/overlay-popup-focus.md index 777f9f4..d827cdd 100644 --- a/.changeset/overlay-popup-focus.md +++ b/.changeset/overlay-popup-focus.md @@ -14,6 +14,9 @@ layer's window nor a registered layer, or on a control inside the window whose popup is expanded (`aria-expanded` with `aria-haspopup`). No cooperation from the popup is required. +`POPUP_SELECTOR` — the selector naming those popup roles — is exported too, so +a consumer can match the same set the queries do. + ```ts enabled: () => isTopmostLayer(id) && !foreignPopupHoldsFocus(id) ``` diff --git a/packages/dom/utils/overlay/SPEC.md b/packages/dom/utils/overlay/SPEC.md index ab364cb..74136c7 100644 --- a/packages/dom/utils/overlay/SPEC.md +++ b/packages/dom/utils/overlay/SPEC.md @@ -121,6 +121,7 @@ again — but keeps painting until its exit visual finishes: | `Layer` | `OverlayLayer` + `element`, `modal`, an optional `backdrop` getter, and an optional `dismiss`. | | `isTopmostLayer(id)` | Whether the layer owns Escape and the focus trap right now. | | `layersBelow(id)` | The layers stacked beneath, topmost first — the unwinding order for a stack-scoped dismissal. | +| `POPUP_SELECTOR` | The CSS selector naming the popup roles (`aria-haspopup`'s values) the queries read ownership from. | | `foreignPopupHoldsFocus(id)` | Whether focus sits in a popup that is neither the layer's window nor a registered layer. | | `expandedPopupControlHoldsFocus(id)` | Whether focus sits on a control inside the layer's window whose popup is expanded. | | `getInitialFocus(content, designated?)` | The element to focus on open: `designated`, else first form field, else the overlay window — each step filtered for renderedness. | diff --git a/packages/dom/utils/overlay/src/index.ts b/packages/dom/utils/overlay/src/index.ts index 867b9f1..004d8d5 100644 --- a/packages/dom/utils/overlay/src/index.ts +++ b/packages/dom/utils/overlay/src/index.ts @@ -1,5 +1,9 @@ export { registerLayer, isTopmostLayer, layersBelow, type Layer } from './stack' -export { foreignPopupHoldsFocus, expandedPopupControlHoldsFocus } from './popup-focus' +export { + POPUP_SELECTOR, + foreignPopupHoldsFocus, + expandedPopupControlHoldsFocus, +} from './popup-focus' export { getInitialFocus } from './get-initial-focus' export { watchExitAnimation } from './watch-exit-animation' export { hideExitingLayer } from './hide-exiting-layer' diff --git a/packages/dom/utils/overlay/src/popup-focus.ts b/packages/dom/utils/overlay/src/popup-focus.ts index 42f8b2e..8a22e5c 100644 --- a/packages/dom/utils/overlay/src/popup-focus.ts +++ b/packages/dom/utils/overlay/src/popup-focus.ts @@ -7,7 +7,7 @@ import { getLayer, layerContaining } from './stack' // focus sits in, and the stack tells a registered layer from a popup that // never registered. No cooperation is required — any well-formed ARIA popup // works. -const POPUP_SELECTOR = +export const POPUP_SELECTOR = '[role="listbox"], [role="menu"], [role="tree"], [role="grid"], [role="dialog"], [role="alertdialog"]' /** From 1953cc7675bcc0f7d66ef43e0229f622463cbdb1 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Sun, 4 Oct 2026 11:57:27 +0200 Subject: [PATCH 3/4] fix(dom-overlay): a non-modal layer keeps Escape while focus is on the page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit foreignPopupHoldsFocus counted focus outside every registered window, in no popup role, as a foreign popup — on the premise that the page is inert under an open layer. That holds for a modal layer only: a non-modal dialog leaves the page live, so focus on any page element made its Escape stand down, where it used to answer wherever focus is. The branch now applies to modal layers only. Also disposes every armed Escape listener in the dom-dialog tests' afterEach: the document-level listener from the veto test survived into later tests and cancelled their Escape, masking any test that expected a close. Co-Authored-By: Claude Fable 5.1 --- .../components/dialog/tests/dialog.test.ts | 29 +++++++++++++++++-- packages/dom/utils/overlay/SPEC.md | 12 ++++---- packages/dom/utils/overlay/src/popup-focus.ts | 11 +++---- .../utils/overlay/tests/popup-focus.test.ts | 14 +++++++-- 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/packages/dom/components/dialog/tests/dialog.test.ts b/packages/dom/components/dialog/tests/dialog.test.ts index b864994..81755f8 100644 --- a/packages/dom/components/dialog/tests/dialog.test.ts +++ b/packages/dom/components/dialog/tests/dialog.test.ts @@ -29,12 +29,17 @@ const build = (options: DialogOptions = {}): DialogService => { } // The Escape listener is the last effect in the list; the ones before it are -// the core's, covered by the core's own tests. +// the core's, covered by the core's own tests. Every armed listener is +// 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] - return effect(service, props) ?? ((): void => {}) + const dispose = effect(service, props) ?? ((): void => {}) + armed.push(dispose) + return dispose } const pressEscape = (): boolean => @@ -57,6 +62,7 @@ const mountLayer = (id: string, depth: number, html = '', dismiss?: () => void): } afterEach(() => { + while (armed.length > 0) (armed.pop() as () => void)() while (registered.length > 0) (registered.pop() as () => void)() document.body.innerHTML = '' vi.restoreAllMocks() @@ -154,6 +160,25 @@ describe('domDialogEffects — Escape', () => { pressEscape() expect(service.matches('open')).toBe(true) }) + + // Escape answers wherever focus is. A non-modal dialog leaves the page live, + // so focus out there is the page's — not a popup the dialog must yield to. + it('closes a non-modal dialog while focus sits on the page', () => { + const service = build({ defaultOpen: true, modal: false }) + const content = document.createElement('div') + content.tabIndex = -1 + document.body.append(content) + registered.push( + registerLayer({ id: 'dlg', depth: 1, element: content, modal: false, backdrop: () => null }), + ) + const page = document.createElement('button') + document.body.append(page) + page.focus() + armEscape(service) + + pressEscape() + expect(service.matches('open')).toBe(false) + }) }) describe('openDialogLayer', () => { diff --git a/packages/dom/utils/overlay/SPEC.md b/packages/dom/utils/overlay/SPEC.md index 74136c7..f1dae10 100644 --- a/packages/dom/utils/overlay/SPEC.md +++ b/packages/dom/utils/overlay/SPEC.md @@ -65,11 +65,13 @@ them to stand down: - `foreignPopupHoldsFocus(id)` — focus sits in a popup that is neither the layer's own window nor a registered layer: an element with a popup role (`aria-haspopup`'s values — listbox, menu, tree, grid, dialog), wherever it - renders, or anything outside the window that is in no popup role at all — - the page is inert while a modal layer is open, so whatever holds focus out - there is a layer. Focus on the body doesn't count: the layer re-enters from - there. Registered layers never count as foreign — a layer beneath is inert, - and focus reported there re-enters the topmost layer's trap. + renders, or — for a modal layer — anything outside the window that is in no + popup role at all: its page is inert, so whatever holds focus out there is a + layer. A non-modal layer leaves the page live, so focus out there is the + page's and the layer keeps answering. Focus on the body never counts: the + layer re-enters from there. Registered layers never count as foreign — a + layer beneath is inert, and focus reported there re-enters the topmost + layer's trap. - `expandedPopupControlHoldsFocus(id)` — focus sits on a control inside the window whose popup is expanded (`aria-expanded="true"` with `aria-haspopup`), the way a combobox keeps focus on its input while its list is open. diff --git a/packages/dom/utils/overlay/src/popup-focus.ts b/packages/dom/utils/overlay/src/popup-focus.ts index 8a22e5c..fe72d64 100644 --- a/packages/dom/utils/overlay/src/popup-focus.ts +++ b/packages/dom/utils/overlay/src/popup-focus.ts @@ -13,10 +13,11 @@ export const POPUP_SELECTOR = /** * Whether focus sits in a popup that is neither the layer's own window nor a * registered layer — wherever it renders, inside the window or portalled - * beside it. Outside every registered window and in no popup role counts too: - * the page is inert while a modal layer is open, so whatever holds focus out - * there is a layer. The body (focus in browser chrome, or nowhere) doesn't: the - * layer re-enters from there. + * beside it. For a modal layer, outside every registered window and in no + * popup role counts too: its page is inert, so whatever holds focus out there + * is a layer. A non-modal layer leaves the page live, so focus out there is + * the page's and the layer keeps answering. The body (focus in browser chrome, + * or nowhere) never counts: the layer re-enters from there. */ export function foreignPopupHoldsFocus(id: string): boolean { const layer = getLayer(id) @@ -24,7 +25,7 @@ export function foreignPopupHoldsFocus(id: string): boolean { const active = document.activeElement if (active === null || active === document.body) return false const popup = active.closest(POPUP_SELECTOR) - if (popup === null) return layerContaining(active) === undefined + if (popup === null) return layer.modal && layerContaining(active) === undefined return popup !== layer.element && layerContaining(popup)?.element !== popup } diff --git a/packages/dom/utils/overlay/tests/popup-focus.test.ts b/packages/dom/utils/overlay/tests/popup-focus.test.ts index 3c349f4..5b7904a 100644 --- a/packages/dom/utils/overlay/tests/popup-focus.test.ts +++ b/packages/dom/utils/overlay/tests/popup-focus.test.ts @@ -9,12 +9,12 @@ import { const registered: Array<() => void> = [] // A registered layer window with the given markup inside it. -const mountLayer = (id: string, depth: number, html = ''): HTMLElement => { +const mountLayer = (id: string, depth: number, html = '', modal = true): HTMLElement => { const content = document.createElement('div') content.tabIndex = -1 content.innerHTML = html document.body.append(content) - registered.push(registerLayer({ id, depth, element: content, modal: true })) + registered.push(registerLayer({ id, depth, element: content, modal })) return content } @@ -73,6 +73,16 @@ describe('foreignPopupHoldsFocus', () => { expect(foreignPopupHoldsFocus('dlg')).toBe(true) }) + // A non-modal layer leaves the page live: focus out there is the page's, + // not a popup's. + it('is false while focus sits outside a non-modal layer in an element with no popup role', () => { + mountLayer('dlg', 1, '', false) + const beside = mountBeside('
') + focus(beside, '#page') + + expect(foreignPopupHoldsFocus('dlg')).toBe(false) + }) + it('is false while focus sits in another registered layer', () => { const lower = mountLayer('lower', 1, '') mountLayer('upper', 2) From 995cce02d0098bfd61786e3756b2397d69c52c74 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Sun, 4 Oct 2026 12:02:27 +0200 Subject: [PATCH 4/4] chore: format README and the dialog substrate SPECs Pre-existing on main: the restoreFocus rows outgrew their props tables, and the README used a star emphasis the formatter rewrites. Whitespace and emphasis style only. Co-Authored-By: Claude Fable 5.1 --- README.md | 2 +- packages/react/dialog/SPEC.md | 8 ++++---- packages/solid/dialog/SPEC.md | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 13f1a4c..51c3a18 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ up there with the same behavior and the same a11y. The engine that runs the core machines lives in its own repo: [dunky-dev/state-machine](https://github.com/dunky-dev/state-machine). Here it -is just a dependency — this repo defines *what* each primitive does and how it +is just a dependency — this repo defines _what_ each primitive does and how it renders, not how machines execute. ## Layout diff --git a/packages/react/dialog/SPEC.md b/packages/react/dialog/SPEC.md index f874feb..4e7b816 100644 --- a/packages/react/dialog/SPEC.md +++ b/packages/react/dialog/SPEC.md @@ -149,11 +149,11 @@ The positioning + scroll layer around the dialog window. The dialog window; renders a `
` with the `dialog` role. -| Prop | Type | Default | Description | -| -------------- | -------------------------------- | ----------------- | ------------------------------------------- | -| `initialFocus` | `RefObject` | the dialog window | The element to focus when the dialog opens. | +| Prop | Type | Default | Description | +| -------------- | -------------------------------- | ----------------- | ---------------------------------------------------------------------------------------------------------------------------------- | +| `initialFocus` | `RefObject` | the dialog window | The element to focus when the dialog opens. | | `restoreFocus` | `RefObject` | — | Focused on close when nothing meaningful held focus before opening (the body, or an element since removed). Typically the trigger. | -| `...props` | `ComponentProps<'div'>` | — | Forwarded to the rendered `
`. | +| `...props` | `ComponentProps<'div'>` | — | Forwarded to the rendered `
`. | ### `Dialog.Title` diff --git a/packages/solid/dialog/SPEC.md b/packages/solid/dialog/SPEC.md index f0f98ea..4166469 100644 --- a/packages/solid/dialog/SPEC.md +++ b/packages/solid/dialog/SPEC.md @@ -155,11 +155,11 @@ The positioning + scroll layer around the dialog window. The dialog window; renders a `
` with the `dialog` role. -| Prop | Type | Default | Description | -| -------------- | --------------------------------------------------------- | ----------------- | ------------------------------------------------------------------- | -| `initialFocus` | `HTMLElement \| (() => HTMLElement \| null \| undefined)` | the dialog window | The element to focus when the dialog opens — resolved at open time. | +| Prop | Type | Default | Description | +| -------------- | --------------------------------------------------------- | ----------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `initialFocus` | `HTMLElement \| (() => HTMLElement \| null \| undefined)` | the dialog window | The element to focus when the dialog opens — resolved at open time. | | `restoreFocus` | `HTMLElement \| (() => HTMLElement \| null \| undefined)` | — | Focused on close when nothing meaningful held focus before opening (the body, or an element since removed) — resolved at close time. Typically the trigger. | -| `...props` | `ComponentProps<'div'>` | — | Forwarded to the rendered `
`. | +| `...props` | `ComponentProps<'div'>` | — | Forwarded to the rendered `
`. | ### `Dialog.Title`