From 772a7dfe18a58d070c1872b48ef8e6acec180723 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Wed, 26 Aug 2026 14:58:55 +0200 Subject: [PATCH 1/5] fix(dom-overlay): hide by descent from the body, not an ancestor walk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Containment matched its retained elements by ancestry, so a branch that contained one was skipped whole. `container` on the Portal parts puts a layer wherever the consumer wants, and when that branch also held page content the whole branch went unhidden. Descend from the body instead: a branch holding a retained root is recursed into rather than spared, so content beside a portalled layer is hidden individually while the layer stays reachable. A target at or above the body no-ops — the old walk got that from its loop condition. Co-Authored-By: Claude Opus 5 (1M context) --- .../overlay-portal-branch-containment.md | 25 +++++ packages/dom/utils/overlay/SPEC.md | 19 ++-- .../dom/utils/overlay/src/hide-outside.ts | 93 ++++++++++++------- .../utils/overlay/tests/containment.test.ts | 31 +++++++ 4 files changed, 129 insertions(+), 39 deletions(-) create mode 100644 .changeset/overlay-portal-branch-containment.md diff --git a/.changeset/overlay-portal-branch-containment.md b/.changeset/overlay-portal-branch-containment.md new file mode 100644 index 0000000..26c33d5 --- /dev/null +++ b/.changeset/overlay-portal-branch-containment.md @@ -0,0 +1,25 @@ +--- +'@dunky.dev/dom-overlay': patch +--- + +Page content beside a layer portalled into an app branch is now hidden by a +modal layer's containment. + +Containment holds a few elements out of the hiding — the topmost modal layer, +its backdrop, and the layers stacked above it — and it matched them by +ancestry, so a branch that _contained_ one was skipped whole. Where a layer +sits is the consumer's choice: `container` on the Portal part lets it land +anywhere, and when that branch also held page content, the entire branch went +unhidden — the page reachable by pointer, keyboard, and screen reader for as +long as the layer was open. + +```tsx +// The menu lands inside the app branch, beside the page content. + +``` + +Hiding now descends from the body instead of walking up from the layer. A +branch that holds one of those retained elements is descended into rather than +spared, so the content beside it is hidden individually while the layer itself +stays reachable. A layer at or above the body is a no-op — nothing sits +outside it. diff --git a/packages/dom/utils/overlay/SPEC.md b/packages/dom/utils/overlay/SPEC.md index d2a40e1..27b6253 100644 --- a/packages/dom/utils/overlay/SPEC.md +++ b/packages/dom/utils/overlay/SPEC.md @@ -23,11 +23,18 @@ against each other. it has one — its backdrop. Topmost follows the core stack's rule. - Containment follows the **topmost modal layer**, not the topmost layer. Everything outside that layer's subtree is hidden from assistive tech and - taken out of pointer and keyboard reach (`aria-hidden` + `inert` on the - siblings of its ancestor path). Two kinds of element are held out of it: - the layer's own backdrop — rendered outside the content's subtree yet part - of the layer — so an outside press can still dismiss, and every layer - stacked above it. + taken out of pointer and keyboard reach (`aria-hidden` + `inert`). Two + kinds of element are held out of it — together the _retained roots_: the + layer's own backdrop — rendered outside the content's subtree yet part of + the layer — so an outside press can still dismiss, and every layer stacked + above it. +- Hiding descends from the body rather than walking up from the layer, and a + branch that holds a retained root is descended into rather than spared + whole. Portalling is the consumer's choice — every overlay exposes + `container` on its Portal part — so a layer can land on an app branch that + holds page content beside it; sparing the branch would leave that content + reachable. A layer at or above the body is a no-op: nothing sits outside + it. - A non-modal layer above a modal one does not release the modal layer's containment. The ordinary layers — a select menu, a combobox list, a tooltip, a context menu — are non-modal, and living inside a dialog is @@ -98,6 +105,6 @@ again — but keeps painting until its exit visual finishes: | -------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | The store anchors on a realm-global keyed by `Symbol.for`, resolved lazily | A monorepo or micro-frontend can load duplicate copies of this module; separate stores drift apart (the duplicate-singleton bug class of radix-ui/primitives#2815). Lazy keeps `sideEffects: false` honest. | | Containment re-runs from scratch on every stack change | Undo-then-rehide is idempotent and order-free; incremental patching would have to reason about interleaved opens and closes. | -| Excluded elements match by containment, not identity | A layer above is reached through its own portal wrapper, and it is the wrapper that turns up as the sibling on the walk; an identity check would inert the layer with it. | +| Hiding descends from the body instead of walking up from the layer | A branch can hold page content beside a retained root, and `container` on the Portal parts lets a layer land on one; skipping the branch to spare the layer would leave the content beside it reachable. | | Containment sync guards on `element.isConnected` | At teardown the content may already be detached; hiding against a dead node would leak the undo. | | Completion is the element's own end event, first one wins | A transition ends once per property and descendants bubble theirs; the exit belongs to the element carrying `data-state`, styled to finish as one piece. | diff --git a/packages/dom/utils/overlay/src/hide-outside.ts b/packages/dom/utils/overlay/src/hide-outside.ts index 6e0c3d4..7e38e8c 100644 --- a/packages/dom/utils/overlay/src/hide-outside.ts +++ b/packages/dom/utils/overlay/src/hide-outside.ts @@ -1,57 +1,84 @@ // Never hide these: they carry no rendered content, or must stay announced. const HIDE_SKIP = /^(SCRIPT|STYLE|LINK|TEMPLATE)$/ +const NOOP = (): void => {} + /** - * The containment trick: walks from `target` up to the document root and marks - * every sibling along the way `aria-hidden` + `inert`, so assistive tech sees - * only the target's subtree and nothing outside it can be reached — by pointer, - * find-in-page, or programmatic focus. `exclude` names the elements rendered - * outside the target's subtree that must stay reachable: the layer's own - * backdrop (portalled alongside its viewport, and it must stay pressable) and - * the layers stacked above the target. A match is by containment, not - * identity — an excluded element is usually nested inside its own portal - * wrapper, and it is the wrapper that turns up as the sibling on the walk. + * The containment trick: descends from the body and marks everything outside + * the target `aria-hidden` + `inert`, so assistive tech sees only the target's + * subtree and nothing outside it can be reached — by pointer, find-in-page, or + * programmatic focus. The target plus `exclude` are the *retained roots*, left + * wholly reachable: `exclude` names the elements rendered outside the target's + * subtree that must stay so — the layer's own backdrop (portalled alongside its + * viewport, and it must stay pressable) and the layers stacked above the + * target. A branch holding a retained root is descended into rather than + * spared whole: where a layer sits is the consumer's choice — every overlay + * exposes `container` on its Portal part — so it can land on an app branch that + * holds page content beside it, and sparing the branch would leave that content + * reachable. A target at or above the body is a no-op: nothing sits outside it. * Returns a function that removes exactly what it added. Callers hide one * target at a time. */ export function hideOutside(target: HTMLElement, exclude?: readonly Element[]): () => void { - const hidden: Array<[Element, string | null]> = [] + // Without this the descent would hide the page itself rather than no-op. + // Covers `document.body` and `document.documentElement`. + if (target.contains(document.body)) return NOOP + + const roots: Element[] = [target] + if (exclude !== undefined) { + for (const element of exclude) roots.push(element) + } - // Hoisted out of the sibling loop: hot path, and the closure a `.some()` - // would allocate per sibling buys nothing here. `Node.contains` returns true - // for the node itself, so this also covers the direct-sibling backdrop case. - function isExcluded(sibling: Element): boolean { - if (exclude === undefined) return false - for (const element of exclude) { - if (sibling.contains(element)) return true + // Hoisted out of the walk: hot path, and the closure a `.some()` would + // allocate per node buys nothing here. + function isRoot(node: Element): boolean { + for (const root of roots) { + if (root === node) return true } return false } - let node: HTMLElement | null = target - while (node !== null && node !== document.body && node.parentElement !== null) { - for (const sibling of Array.from(node.parentElement.children)) { - // Skip the path itself, the excluded elements, 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. - const ariaHidden = sibling.getAttribute('aria-hidden') + // `Node.contains` is true of a node itself, so this only means "an ancestor + // of a root" because `isRoot` is always tested first. + function retainsRoot(node: Element): boolean { + for (const root of roots) { + if (node.contains(root)) return true + } + return false + } + + const hidden: Array<[Element, string | null]> = [] + + function hideOutsideOf(parent: Element): void { + for (const child of Array.from(parent.children)) { + // A retained root and its subtree stay wholly reachable. + if (isRoot(child)) continue + // Page content can sit beside a retained root, so descend rather than + // skip the whole branch. + if (retainsRoot(child)) { + 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 ( - sibling === node || - isExcluded(sibling) || - HIDE_SKIP.test(sibling.tagName) || + HIDE_SKIP.test(child.tagName) || (ariaHidden !== null && ariaHidden !== 'false') || - sibling.hasAttribute('inert') + child.hasAttribute('inert') ) { continue } - sibling.setAttribute('aria-hidden', 'true') - sibling.setAttribute('inert', '') - hidden.push([sibling, ariaHidden]) + child.setAttribute('aria-hidden', 'true') + child.setAttribute('inert', '') + hidden.push([child, ariaHidden]) } - node = node.parentElement } + hideOutsideOf(document.body) + return () => { for (const [element, previousAriaHidden] of hidden) { if (previousAriaHidden === null) element.removeAttribute('aria-hidden') diff --git a/packages/dom/utils/overlay/tests/containment.test.ts b/packages/dom/utils/overlay/tests/containment.test.ts index 3e10ddf..aeaf98f 100644 --- a/packages/dom/utils/overlay/tests/containment.test.ts +++ b/packages/dom/utils/overlay/tests/containment.test.ts @@ -96,6 +96,15 @@ describe('registerLayer containment', () => { expect(asserted.hasAttribute('inert')).toBe(false) }) + it('hides nothing for a layer at the body — there is no outside', () => { + const outside = document.createElement('main') + document.body.append(outside) + + register({ id: 'a', depth: 1, element: document.body, modal: true }) + expect(outside.hasAttribute('inert')).toBe(false) + expect(document.body.hasAttribute('inert')).toBe(false) + }) + it('hides nothing for a non-modal layer', () => { const outside = document.createElement('main') document.body.append(outside) @@ -191,6 +200,28 @@ describe('containment under a non-modal layer', () => { expect(hiddenFrom(outside)).toBe(true) }) + it('hides page content sitting beside a layer portalled into an app branch', () => { + // `container` on every overlay's Portal part is public API, so a layer can + // land on an app branch rather than the body. Skipping that whole branch to + // spare the layer would leave the page content beside it reachable. + const app = document.createElement('div') + const pageContent = document.createElement('article') + const menuPortal = document.createElement('div') + const menu = document.createElement('div') + menuPortal.append(menu) + app.append(pageContent, menuPortal) + document.body.append(app) + const dialog = mountLayer() + + register({ id: 'dialog', depth: 1, element: dialog.content, modal: true }) + register({ id: 'menu', depth: 2, element: menu, modal: false }) + + expect(hiddenFrom(pageContent)).toBe(true) + expect(app.hasAttribute('inert')).toBe(false) + expect(menuPortal.hasAttribute('inert')).toBe(false) + expect(menu.hasAttribute('inert')).toBe(false) + }) + it('follows the upper modal layer when a non-modal layer sits above both', () => { const outer = mountLayer() const inner = mountLayer() From 6c249f96dd6e3f821d4b71bae250f1d94e40298c Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Wed, 26 Aug 2026 15:16:56 +0200 Subject: [PATCH 2/5] feat(dom-element): share isRendered; initial focus skips unrendered candidates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getInitialFocus` filtered nothing: a field inside a collapsed section satisfied the selector, `focus()` on it silently no-opped, and focus fell back to the dialog window — with the fallback's warning unable to fire, because from its point of view the fallback had succeeded. A designated `initialFocus` that hadn't rendered was worse: it skipped the form-field step entirely, contradicting "when one is set and can take focus". The focus trap already asked this question, privately. Extract it to `@dunky.dev/dom-element` so the two callers can't drift, and filter every step of the initial-focus chain rather than just the last. The extracted predicate also gains an `isConnected` check and drops the trap's container bound on the display walk — a detached element and one under a hidden ancestor both can't take focus. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/dom-element-is-rendered.md | 48 +++++++++++++++++++ AGENTS.md | 7 +-- ARCHITECTURE.md | 12 +++-- .../dom/components/dialog/src/open-layer.ts | 9 +++- packages/dom/utils/element/README.md | 34 +++++++++++++ packages/dom/utils/element/SPEC.md | 46 ++++++++++++++++++ packages/dom/utils/element/package.json | 38 +++++++++++++++ packages/dom/utils/element/src/index.ts | 1 + packages/dom/utils/element/src/is-rendered.ts | 27 +++++++++++ .../utils/element/tests/is-rendered.test.ts | 37 ++++++++++++++ packages/dom/utils/focus-trap/SPEC.md | 17 ++++--- packages/dom/utils/focus-trap/package.json | 3 ++ .../utils/focus-trap/src/get-focusables.ts | 22 ++------- packages/dom/utils/overlay/README.md | 4 +- packages/dom/utils/overlay/SPEC.md | 29 +++++++---- packages/dom/utils/overlay/package.json | 1 + .../utils/overlay/src/get-initial-focus.ts | 25 +++++++++- .../utils/overlay/tests/containment.test.ts | 43 ++++++++++++++--- pnpm-lock.yaml | 11 ++++- tsconfig.json | 1 + tsdown.config.ts | 1 + 21 files changed, 358 insertions(+), 58 deletions(-) create mode 100644 .changeset/dom-element-is-rendered.md create mode 100644 packages/dom/utils/element/README.md create mode 100644 packages/dom/utils/element/SPEC.md create mode 100644 packages/dom/utils/element/package.json create mode 100644 packages/dom/utils/element/src/index.ts create mode 100644 packages/dom/utils/element/src/is-rendered.ts create mode 100644 packages/dom/utils/element/tests/is-rendered.test.ts diff --git a/.changeset/dom-element-is-rendered.md b/.changeset/dom-element-is-rendered.md new file mode 100644 index 0000000..10d9af0 --- /dev/null +++ b/.changeset/dom-element-is-rendered.md @@ -0,0 +1,48 @@ +--- +'@dunky.dev/dom-element': minor +'@dunky.dev/dom-focus-trap': patch +'@dunky.dev/dom-overlay': patch +'@dunky.dev/dom-dialog': patch +--- + +New package `@dunky.dev/dom-element`, and initial focus now skips a candidate +that didn't render. + +`isRendered(element)` answers whether an element actually rendered, and so can +take focus, be pressed, or be read out. Presence in the DOM is not enough: an +element inside a collapsed section still answers `querySelector`, but `focus()` +on it does nothing and reports nothing. + +```ts +import { isRendered } from '@dunky.dev/dom-element' + +for (const field of content.querySelectorAll('input, select, textarea')) { + if (isRendered(field)) { + field.focus() + break + } +} +``` + +Checked: the `hidden` attribute (`hidden="until-found"` included), +`display: none` on the element or any ancestor — `display` doesn't inherit, so +ancestors are walked — `visibility: hidden | collapse`, and being detached. +Not checked: `opacity: 0` and `content-visibility`, which do render, and +rendering is what decides focusability. + +The predicate is its own package because two utils have to agree on it. The +focus trap already filtered its Tab cycle this way; `getInitialFocus` did not, +so a field in a collapsed section won the draw, `focus()` silently no-opped, +and focus fell back to the dialog window — with the fallback's warning unable +to fire, because from its point of view the fallback had succeeded. + +`getInitialFocus` now takes the consumer's designated element as a second +argument and filters **every** step of the chain rather than just the last: + +```ts +getInitialFocus(content, designatedElement) // designated -> first field -> content +``` + +That fixes a second case in the same class: an unrendered designated element +used to go straight to the dialog window, skipping the form-field step, which +contradicted the documented "when one is set and can take focus". diff --git a/AGENTS.md b/AGENTS.md index 4e94b33..ae55113 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,9 +55,10 @@ architecture: imports only the state-machine runtime, the agnostic bindings vocabulary (`@dunky.dev/state-machine` + `@dunky.dev/state-machine-bindings`), and the machine utils under `core/utils`. A machine util imports only the runtime; - a DOM util imports nothing from this repo; a DOM component imports its core - counterpart and the DOM utils, never a framework; a substrate hook imports - only the DOM util it wraps. + a DOM util imports nothing from this repo except a smaller DOM util (a + predicate two utils must agree on, never a peer that imports it back); a DOM + component imports its core counterpart and the DOM utils, never a framework; + a substrate hook imports only the DOM util it wraps. - **DOM behavior is written once too.** Logic that is DOM-specific but not framework-specific — a document listener, an ordered focus/stack sequence — belongs in `dom/components/`, not copied across substrates. A DOM diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 1503ef3..0d5e090 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -63,8 +63,9 @@ folder. A new substrate reuses all of it and only writes the wrappers. DOM logic that belongs to **one** primitive but to **every** DOM substrate — the dialog's Escape listener, the ordered sequence around its open and exit edges — lives under `dom/components/` instead. A util is primitive-agnostic -and imports nothing from the repo; a component package is the opposite, and -may import the primitive's core package and any DOM util. Both are equally +and imports nothing from the repo but a smaller util; a component package is +the opposite, and may import the primitive's core package and any DOM util. +Both are equally framework-free. The split matters as substrates multiply: React and Solid differ in how they schedule an effect, not in what the effect does, so the what is written once and each binding contributes only its lifecycle. @@ -120,8 +121,11 @@ The rules, stated as imports: repo. - A core package imports only the state-machine runtime and the agnostic bindings vocabulary. -- A DOM util imports nothing from this repo; a substrate hook imports only the - DOM util it wraps. +- A DOM util imports nothing from this repo except another DOM util — and only + a smaller one, never a peer that would import it back. A shared predicate + (`isRendered`) is one package so its callers can't drift; the direction of + such an edge is a design decision, recorded in the importing package's + `SPEC.md`. A substrate hook imports only the DOM util it wraps. - A `dom/components` package imports its core counterpart and the DOM utils — never a framework, and never another primitive. - Primitives are independent of each other. If two need to share logic, that diff --git a/packages/dom/components/dialog/src/open-layer.ts b/packages/dom/components/dialog/src/open-layer.ts index be277b4..eba0860 100644 --- a/packages/dom/components/dialog/src/open-layer.ts +++ b/packages/dom/components/dialog/src/open-layer.ts @@ -37,9 +37,14 @@ export function openDialogLayer(content: HTMLElement, options: OpenDialogLayerOp // preventScroll everywhere: the scroll lock already froze the surface, so // moving focus must not scroll it — otherwise opening jumps the (top-of- // container) dialog into view and closing jumps back to the trigger. - const target = options.initialFocus ?? getInitialFocus(content) + // The whole chain — designated, then first form field, then the window — + // resolves in one call so each step is filtered for renderedness; a `??` + // here would spend the designated element on a candidate that can't take + // focus and skip the field step entirely. + const target = getInitialFocus(content, options.initialFocus) target.focus({ preventScroll: true }) - // A target that can't take focus (disabled, hidden) falls back to the panel. + // A target that can't take focus (disabled, no tabindex) falls back to the + // panel. if (document.activeElement !== target) { content.focus({ preventScroll: true }) // Focus still outside the layer breaks the APG modal pattern — a window diff --git a/packages/dom/utils/element/README.md b/packages/dom/utils/element/README.md new file mode 100644 index 0000000..7cd889a --- /dev/null +++ b/packages/dom/utils/element/README.md @@ -0,0 +1,34 @@ +# @dunky.dev/dom-element + +Framework-free predicates about a single element — the DOM questions more than +one primitive has to ask, answered once so the answers can't drift. + +`isRendered(element)` tells you whether an element actually rendered, and so +can take focus, be pressed, or be read out. Presence in the DOM is not enough: +an element inside a collapsed section still answers `querySelector`, but +`focus()` on it does nothing and reports nothing. + +## Install + +```sh +npm install @dunky.dev/dom-element +``` + +## Usage + +```ts +import { isRendered } from '@dunky.dev/dom-element' + +// Pick the first field that can really take focus, not just the first match. +for (const field of content.querySelectorAll('input, select, textarea')) { + if (isRendered(field)) { + field.focus() + break + } +} +``` + +Checked: the `hidden` attribute (including `hidden="until-found"`), +`display: none` on the element or any ancestor, `visibility: hidden | collapse`, +and being detached. Not checked: `opacity: 0` and `content-visibility` — those +render, and rendering is what decides focusability. diff --git a/packages/dom/utils/element/SPEC.md b/packages/dom/utils/element/SPEC.md new file mode 100644 index 0000000..7b3347e --- /dev/null +++ b/packages/dom/utils/element/SPEC.md @@ -0,0 +1,46 @@ +# SPEC / DOM / Element + +## Overview + +Framework-free predicates about a single element — the DOM questions more than +one primitive has to ask, answered once so the answers can't drift. Today that +is one question: did this element actually render? + +Two packages ask it for the same reason. `@dunky.dev/dom-focus-trap` filters +the Tab cycle, and `@dunky.dev/dom-overlay` filters the initial-focus +candidates; both are guarding against the same silent failure, so they must +agree on what counts. + +## Behavior + +- An element is **rendered** when it is connected, carries no `hidden` + attribute on itself or an ancestor, computes to neither + `visibility: hidden` nor `visibility: collapse`, and has no + `display: none` on itself or any ancestor. +- A detached element is never rendered. It can't take focus, and computed + style on one reports the property defaults rather than `none`, so nothing + else in the check would catch it. +- `opacity: 0` and `content-visibility` are out of scope: those render. + Whether something is _perceivable_ is a different question from whether it + rendered at all, and only the latter decides focusability. + +## API + +| Export | Description | +| --------------------- | -------------------------------------------------------------------------------- | +| `isRendered(element)` | Whether the element rendered, and so can take focus, be pressed, or be read out. | + +## Constraints + +- The answer is read from the live DOM on every call — a collapsed section + opens and closes between two of them. +- No caching and no layout reads: callers run this over every candidate in a + container, sometimes inside a `preventDefault`-ed keydown. + +## Internals + +| Position | Why | +| ---------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Rendered-ness via a computed-style walk, not `Element.checkVisibility()` | The API is recent (Chrome/Edge 105+, Firefox 106+, Safari 17.4+) and a caller may resolve candidates after a Tab keydown's `preventDefault()`, so on a browser without it the throw would leave Tab dead entirely; the walk is spec-defined behavior everywhere, and needs no test-environment shim. | +| Not `getClientRects().length` or `offsetParent` | Both are geometry, which test environments report as zeros — so nothing here would be covered. `offsetParent` is also `null` for a `position: fixed` element that is plainly visible, a false negative on exactly the overlay content this guards. | +| The `hidden` attribute is checked with `closest`, not folded into the display walk | `hidden="until-found"` hides through `content-visibility`, not `display`, so the computed `display` of an element inside one is its own value. The attribute is the only signal. | diff --git a/packages/dom/utils/element/package.json b/packages/dom/utils/element/package.json new file mode 100644 index 0000000..0b5ffd8 --- /dev/null +++ b/packages/dom/utils/element/package.json @@ -0,0 +1,38 @@ +{ + "name": "@dunky.dev/dom-element", + "version": "0.0.0", + "description": "Framework-free element predicates — the DOM questions every primitive asks.", + "license": "MIT", + "repository": { + "type": "git", + "url": "git+https://github.com/dunky-dev/ui.git", + "directory": "packages/dom/utils/element" + }, + "files": [ + "dist", + "src", + "SPEC.md" + ], + "type": "module", + "sideEffects": false, + "main": "./src/index.ts", + "types": "./src/index.ts", + "exports": { + ".": "./src/index.ts" + }, + "publishConfig": { + "main": "./dist/index.js", + "module": "./dist/index.js", + "types": "./dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js" + } + }, + "access": "public" + }, + "scripts": { + "build": "tsdown" + } +} diff --git a/packages/dom/utils/element/src/index.ts b/packages/dom/utils/element/src/index.ts new file mode 100644 index 0000000..ebce5e9 --- /dev/null +++ b/packages/dom/utils/element/src/index.ts @@ -0,0 +1 @@ +export { isRendered } from './is-rendered' diff --git a/packages/dom/utils/element/src/is-rendered.ts b/packages/dom/utils/element/src/is-rendered.ts new file mode 100644 index 0000000..f7f16bd --- /dev/null +++ b/packages/dom/utils/element/src/is-rendered.ts @@ -0,0 +1,27 @@ +/** + * Whether an element is actually rendered — and so able to take focus, be + * pressed, or be read out. Presence in the DOM is not enough: an element inside + * a collapsed section still answers `querySelector`, but `focus()` on it does + * nothing, and it does so silently. + * + * Deliberately out of scope: `opacity: 0` and `content-visibility`, which do + * render. Whether something is *perceivable* is a different question from + * whether it rendered at all, and only the latter decides focusability. + */ +export function isRendered(element: Element): boolean { + // A detached element can't take focus, and computed style on one reports the + // defaults rather than `none`, so the walk below would pass it. + if (!element.isConnected) return false + // The attribute check also covers hidden="until-found", which hides via + // content-visibility instead of display. + if (element.closest('[hidden]') !== null) return false + // `visibility` inherits, so the element's own computed value suffices. + const visibility = getComputedStyle(element).visibility + if (visibility === 'hidden' || visibility === 'collapse') return false + // `display` does not inherit — the computed `display` of a child of a + // `display: none` parent is its own value — so ancestors must be walked. + for (let node: Element | null = element; node !== null; node = node.parentElement) { + if (getComputedStyle(node).display === 'none') return false + } + return true +} diff --git a/packages/dom/utils/element/tests/is-rendered.test.ts b/packages/dom/utils/element/tests/is-rendered.test.ts new file mode 100644 index 0000000..379ba0b --- /dev/null +++ b/packages/dom/utils/element/tests/is-rendered.test.ts @@ -0,0 +1,37 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it } from 'vitest' +import { isRendered } from '@dunky.dev/dom-element' + +const mount = (html: string): HTMLElement => { + document.body.innerHTML = html + return document.getElementById('subject') as HTMLElement +} + +afterEach(() => { + document.body.innerHTML = '' +}) + +describe('isRendered', () => { + it('is true for an element that renders', () => { + expect(isRendered(mount(''))).toBe(true) + }) + + it('is false for a detached element', () => { + expect(isRendered(document.createElement('input'))).toBe(false) + }) + + it('is false when the element itself does not render', () => { + expect(isRendered(mount(''))).toBe(false) + expect(isRendered(mount(''))).toBe(false) + expect(isRendered(mount(''))).toBe(false) + }) + + it('is false inside a collapsed ancestor — display does not inherit', () => { + expect(isRendered(mount('
'))).toBe(false) + expect(isRendered(mount(''))).toBe(false) + }) + + it('is true under a transparent ancestor — opacity still renders', () => { + expect(isRendered(mount('
'))).toBe(true) + }) +}) diff --git a/packages/dom/utils/focus-trap/SPEC.md b/packages/dom/utils/focus-trap/SPEC.md index e365aa6..7354b15 100644 --- a/packages/dom/utils/focus-trap/SPEC.md +++ b/packages/dom/utils/focus-trap/SPEC.md @@ -37,10 +37,9 @@ identical containment. - `enabled` and `last` are re-evaluated on every press, so trapping follows runtime state — e.g. only the topmost layer of a stack traps. - A focusable is an element matching `FOCUSABLE_SELECTOR` whose `tabIndex` - is not negative and which is rendered: no `hidden` attribute, no - `display: none` on itself or an ancestor, no `visibility: hidden`. - Focusing a non-rendered element is a no-op, so keeping one in the cycle - would stall the trap on it. + is not negative and which is rendered — the shared predicate from + [`@dunky.dev/dom-element`](../element/SPEC.md). Focusing a non-rendered + element is a no-op, so keeping one in the cycle would stall the trap on it. - A same-name radio group is one tab stop — the checked radio, else the group's first — per the APG radio group pattern; groups are scoped by name and form owner, matching the browser's own grouping. @@ -64,8 +63,8 @@ identical containment. ## Internals -| Position | Why | -| ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| Focus is stepped manually on every press, not only at the edges | The `last` re-ordering makes the logical cycle diverge from DOM order, so native tabbing can't be trusted mid-cycle. | -| Document-level, capture-phase keydown listener | A container listener misses presses while focus is still outside; capture delivery survives a `stopPropagation` in the subtree. | -| Rendered-ness via a computed-style walk, not `Element.checkVisibility()` | The API is recent (Chrome/Edge 105+, Firefox 106+, Safari 17.4+) and the trap resolves focusables after the Tab keydown's `preventDefault()`, so on any browser without it the throw would leave Tab dead entirely; the walk is spec-defined behavior everywhere (and needs no test-environment shim). | +| Position | Why | +| --------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Focus is stepped manually on every press, not only at the edges | The `last` re-ordering makes the logical cycle diverge from DOM order, so native tabbing can't be trusted mid-cycle. | +| Document-level, capture-phase keydown listener | A container listener misses presses while focus is still outside; capture delivery survives a `stopPropagation` in the subtree. | +| Rendered-ness is asked of `@dunky.dev/dom-element`, not answered here | The overlay's initial-focus resolution guards against the same silent `focus()` no-op, and two packages answering separately would drift. The predicate's own trade-offs live in that package's SPEC. | diff --git a/packages/dom/utils/focus-trap/package.json b/packages/dom/utils/focus-trap/package.json index 8c2a7bc..a8a40e6 100644 --- a/packages/dom/utils/focus-trap/package.json +++ b/packages/dom/utils/focus-trap/package.json @@ -34,5 +34,8 @@ }, "scripts": { "build": "tsdown" + }, + "dependencies": { + "@dunky.dev/dom-element": "workspace:*" } } diff --git a/packages/dom/utils/focus-trap/src/get-focusables.ts b/packages/dom/utils/focus-trap/src/get-focusables.ts index ae6e901..00ca1d5 100644 --- a/packages/dom/utils/focus-trap/src/get-focusables.ts +++ b/packages/dom/utils/focus-trap/src/get-focusables.ts @@ -1,3 +1,5 @@ +import { isRendered } from '@dunky.dev/dom-element' + export const FOCUSABLE_SELECTOR: string = [ 'a[href]', 'area[href]', @@ -14,24 +16,6 @@ export const FOCUSABLE_SELECTOR: string = [ '[tabindex]', ].join(', ') -function isRendered(element: HTMLElement, container: HTMLElement): boolean { - // The attribute check also covers hidden="until-found", which hides via - // content-visibility instead of display. - if (element.closest('[hidden]') !== null) return false - // `visibility` inherits, so the element's own computed value suffices. - const visibility = getComputedStyle(element).visibility - if (visibility === 'hidden' || visibility === 'collapse') return false - // `display` does not inherit, so ancestors must be walked. - for ( - let node: HTMLElement | null = element; - node && node !== container; - node = node.parentElement - ) { - if (getComputedStyle(node).display === 'none') return false - } - return true -} - // A named radio participates in a group; groups are scoped by name AND form // owner, matching the browser's own grouping. function isGroupedRadio(element: HTMLElement): element is HTMLInputElement { @@ -45,7 +29,7 @@ export function getFocusables(container: HTMLElement): HTMLElement[] { const element = candidates[i]! // Focusing a non-rendered element is a no-op, so keeping one in the cycle // would stall the trap on it. - if (element.tabIndex >= 0 && isRendered(element, container)) { + if (element.tabIndex >= 0 && isRendered(element)) { eligible.push(element) } } diff --git a/packages/dom/utils/overlay/README.md b/packages/dom/utils/overlay/README.md index 848797a..b2f2403 100644 --- a/packages/dom/utils/overlay/README.md +++ b/packages/dom/utils/overlay/README.md @@ -44,7 +44,9 @@ const unregister = registerLayer({ modal: true, backdrop: () => backdropElement, // stays pressable while topmost }) -getInitialFocus(content).focus({ preventScroll: true }) +// Designated element, else the first form field, else the window — each step +// skipped unless it actually rendered. +getInitialFocus(content, designatedElement).focus({ preventScroll: true }) // Escape, outside-press, focus trapping: only the topmost layer answers. if (isTopmostLayer(id)) { diff --git a/packages/dom/utils/overlay/SPEC.md b/packages/dom/utils/overlay/SPEC.md index 27b6253..fba172b 100644 --- a/packages/dom/utils/overlay/SPEC.md +++ b/packages/dom/utils/overlay/SPEC.md @@ -58,7 +58,16 @@ against each other. The strict rule is only that focus moves into the overlay: an overlay that collects input starts at its first form field (input, select, textarea); any -other content keeps focus on the overlay window itself. +other content keeps focus on the overlay window itself. A caller may +designate an element ahead of both. + +Every candidate must be **rendered**, not merely present. A field inside a +collapsed section satisfies the selector, yet `focus()` on it does nothing +and reports nothing, so accepting it would spend the candidate and drop focus +to the overlay window — the fallback firing on a miss it can't see. Each step +of the chain is therefore filtered, not just the last one; the predicate is +[`@dunky.dev/dom-element`](../element/SPEC.md)'s `isRendered`, shared with the +focus trap so the two can't disagree on what counts. ### The exit window @@ -79,15 +88,15 @@ again — but keeps painting until its exit visual finishes: ## API -| Export | Description | -| ------------------------------------------------ | ---------------------------------------------------------------------------------------------- | -| `registerLayer(layer)` | Joins the shared stack and syncs containment; returns the disposer that restores it. | -| `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. | -| `getInitialFocus(content)` | The element to focus on open: first form field, else the overlay window itself. | -| `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. | +| Export | Description | +| ------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------- | +| `registerLayer(layer)` | Joins the shared stack and syncs containment; returns the disposer that restores it. | +| `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. | +| `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. | ## Constraints diff --git a/packages/dom/utils/overlay/package.json b/packages/dom/utils/overlay/package.json index 79c4f1b..2672b41 100644 --- a/packages/dom/utils/overlay/package.json +++ b/packages/dom/utils/overlay/package.json @@ -36,6 +36,7 @@ "build": "tsdown" }, "dependencies": { + "@dunky.dev/dom-element": "workspace:*", "@dunky.dev/overlay": "workspace:*" } } diff --git a/packages/dom/utils/overlay/src/get-initial-focus.ts b/packages/dom/utils/overlay/src/get-initial-focus.ts index 6d5b491..ff372a3 100644 --- a/packages/dom/utils/overlay/src/get-initial-focus.ts +++ b/packages/dom/utils/overlay/src/get-initial-focus.ts @@ -1,9 +1,30 @@ +import { isRendered } from '@dunky.dev/dom-element' + // The strict rule is only that focus moves into the overlay: an overlay that // collects input starts at its first form field; any other content keeps // focus on the overlay window itself. const FORM_FIELD_SELECTOR = 'input:not([disabled]):not([type="hidden"]), select:not([disabled]), textarea:not([disabled])' -export function getInitialFocus(content: HTMLElement): HTMLElement { - return content.querySelector(FORM_FIELD_SELECTOR) ?? content +/** + * Resolves where focus lands: the consumer's designated element, then the + * first form field, then the overlay window. Every candidate has to be + * *rendered*, not merely present — a field inside a collapsed section + * satisfies the selector, and `focus()` on it does nothing without saying so, + * which spends the candidate and drops focus to the window. Filtering each + * step keeps every fallback a real one. + */ +export function getInitialFocus( + content: HTMLElement, + designated?: HTMLElement | null, +): HTMLElement { + if (designated != null && isRendered(designated)) return designated + + const fields = content.querySelectorAll(FORM_FIELD_SELECTOR) + for (let i = 0; i < fields.length; i++) { + const field = fields[i]! + if (isRendered(field)) return field + } + + return content } diff --git a/packages/dom/utils/overlay/tests/containment.test.ts b/packages/dom/utils/overlay/tests/containment.test.ts index aeaf98f..bfba72a 100644 --- a/packages/dom/utils/overlay/tests/containment.test.ts +++ b/packages/dom/utils/overlay/tests/containment.test.ts @@ -253,21 +253,50 @@ describe('layer stack global anchoring', () => { }) describe('getInitialFocus', () => { - it('resolves the first form field that can take focus', () => { + // Mounted, not detached: a candidate has to be rendered to be picked, and a + // detached element renders nowhere. + const mountContent = (html: string): HTMLElement => { const content = document.createElement('div') - content.innerHTML = + content.innerHTML = html + document.body.append(content) + return content + } + + it('resolves the first form field that can take focus', () => { + const content = mountContent( '' + - '' + - '' + - '' + '' + + '' + + '', + ) expect(getInitialFocus(content).id).toBe('field') }) it('falls back to the content itself without form fields', () => { - const content = document.createElement('div') - content.innerHTML = '' + const content = mountContent('') expect(getInitialFocus(content)).toBe(content) }) + + it('skips a field that does not render for one that does', () => { + // A field in a collapsed section satisfies the selector but cannot take + // focus, and it fails silently — so taking it would spend the candidate + // and drop focus to the overlay window with nothing reporting the miss. + const content = mountContent( + '
', + ) + + expect(getInitialFocus(content).id).toBe('field') + }) + + it('hands over to the fields when the designated element does not render', () => { + // The contract conditions the designated element on being able to take + // focus, so an unrendered one must not consume the chain down to the + // window. + const content = mountContent('') + const designated = document.getElementById('designated') as HTMLElement + + expect(getInitialFocus(content, designated).id).toBe('field') + }) }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 64f8490..2022777 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -87,12 +87,21 @@ importers: specifier: ^0.3.3 version: 0.3.3 - packages/dom/utils/focus-trap: {} + packages/dom/utils/element: {} + + packages/dom/utils/focus-trap: + dependencies: + '@dunky.dev/dom-element': + specifier: workspace:* + version: link:../element packages/dom/utils/navigation: {} packages/dom/utils/overlay: dependencies: + '@dunky.dev/dom-element': + specifier: workspace:* + version: link:../element '@dunky.dev/overlay': specifier: workspace:* version: link:../../../core/utils/overlay diff --git a/tsconfig.json b/tsconfig.json index 9c5f7f4..4e39ec3 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -23,6 +23,7 @@ "@dunky.dev/dom-dialog": ["./packages/dom/components/dialog/src"], "@dunky.dev/dom-overlay": ["./packages/dom/utils/overlay/src"], "@dunky.dev/dom-focus-trap": ["./packages/dom/utils/focus-trap/src"], + "@dunky.dev/dom-element": ["./packages/dom/utils/element/src"], "@dunky.dev/browser-navigation": ["./packages/dom/utils/navigation/src"], "@dunky.dev/dom-scroll-lock": ["./packages/dom/utils/scroll-lock/src"], "@dunky.dev/react-use-focus-trap": ["./packages/react/hooks/use-focus-trap/src"], diff --git a/tsdown.config.ts b/tsdown.config.ts index c96c0c5..2704eb7 100644 --- a/tsdown.config.ts +++ b/tsdown.config.ts @@ -14,6 +14,7 @@ export default defineConfig({ 'packages/core/utils/controllable', 'packages/core/utils/overlay', 'packages/dom/components/dialog', + 'packages/dom/utils/element', 'packages/dom/utils/focus-trap', 'packages/dom/utils/overlay', 'packages/dom/utils/navigation', From bfbe86307b07bfc8d55207c70cfdc328693e5814 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Wed, 26 Aug 2026 15:19:53 +0200 Subject: [PATCH 3/5] docs(changeset): split the isRendered changeset into its three stories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One entry covered a new package, a refactor with two behavior changes, and a bug fix — three different things for a consumer reading the changelog. Same bump set, one story each. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/dom-element-is-rendered.md | 27 ++++----------- .../focus-trap-shared-rendered-check.md | 20 +++++++++++ .changeset/overlay-initial-focus-rendered.md | 34 +++++++++++++++++++ 3 files changed, 60 insertions(+), 21 deletions(-) create mode 100644 .changeset/focus-trap-shared-rendered-check.md create mode 100644 .changeset/overlay-initial-focus-rendered.md diff --git a/.changeset/dom-element-is-rendered.md b/.changeset/dom-element-is-rendered.md index 10d9af0..4c749ee 100644 --- a/.changeset/dom-element-is-rendered.md +++ b/.changeset/dom-element-is-rendered.md @@ -1,12 +1,9 @@ --- '@dunky.dev/dom-element': minor -'@dunky.dev/dom-focus-trap': patch -'@dunky.dev/dom-overlay': patch -'@dunky.dev/dom-dialog': patch --- -New package `@dunky.dev/dom-element`, and initial focus now skips a candidate -that didn't render. +New package: `@dunky.dev/dom-element`, framework-free predicates about a single +element. `isRendered(element)` answers whether an element actually rendered, and so can take focus, be pressed, or be read out. Presence in the DOM is not enough: an @@ -30,19 +27,7 @@ ancestors are walked — `visibility: hidden | collapse`, and being detached. Not checked: `opacity: 0` and `content-visibility`, which do render, and rendering is what decides focusability. -The predicate is its own package because two utils have to agree on it. The -focus trap already filtered its Tab cycle this way; `getInitialFocus` did not, -so a field in a collapsed section won the draw, `focus()` silently no-opped, -and focus fell back to the dialog window — with the fallback's warning unable -to fire, because from its point of view the fallback had succeeded. - -`getInitialFocus` now takes the consumer's designated element as a second -argument and filters **every** step of the chain rather than just the last: - -```ts -getInitialFocus(content, designatedElement) // designated -> first field -> content -``` - -That fixes a second case in the same class: an unrendered designated element -used to go straight to the dialog window, skipping the form-field step, which -contradicted the documented "when one is set and can take focus". +It's a package of its own because two utils have to agree on the answer: +`@dunky.dev/dom-focus-trap` filters its Tab cycle with it and +`@dunky.dev/dom-overlay` filters its initial-focus candidates, both guarding +against the same silent `focus()` no-op. diff --git a/.changeset/focus-trap-shared-rendered-check.md b/.changeset/focus-trap-shared-rendered-check.md new file mode 100644 index 0000000..6739e21 --- /dev/null +++ b/.changeset/focus-trap-shared-rendered-check.md @@ -0,0 +1,20 @@ +--- +'@dunky.dev/dom-focus-trap': patch +--- + +The Tab cycle's rendered check now comes from `@dunky.dev/dom-element` instead +of a private copy. + +`@dunky.dev/dom-overlay` needs the same predicate to filter its initial-focus +candidates, and two packages answering the question separately would drift. +The check itself is unchanged in intent — a non-rendered element is a no-op to +focus, so keeping one in the cycle would stall the trap on it — but sharing it +tightens two cases: + +- A **detached** element is now excluded. It can't take focus, and computed + style on one reports the property defaults rather than `none`, so the display + walk alone let it through. +- A `display: none` ancestor **above the container** now excludes the + focusables under it. The private copy stopped its walk at the container. + Nothing inside a hidden container can take focus either way, so this lands on + the trap's documented behavior for an empty cycle: Tab is a no-op. diff --git a/.changeset/overlay-initial-focus-rendered.md b/.changeset/overlay-initial-focus-rendered.md new file mode 100644 index 0000000..e39f292 --- /dev/null +++ b/.changeset/overlay-initial-focus-rendered.md @@ -0,0 +1,34 @@ +--- +'@dunky.dev/dom-overlay': patch +'@dunky.dev/dom-dialog': patch +--- + +Initial focus now skips a candidate that didn't render. + +`getInitialFocus` filtered `[disabled]` and `[type="hidden"]` but never asked +whether the element actually rendered. A field inside a collapsed section +satisfied the selector and won the draw; `focus()` on it did nothing — and said +nothing — so focus fell back to the dialog window, with the fallback's warning +unable to fire, because from its point of view the fallback had succeeded. The +overlay opened on its window instead of the field: degraded, not broken, and +silent. + +A designated `initialFocus` that hadn't rendered was worse. It went straight to +the window and skipped the form-field step entirely, contradicting the +documented "when one is set **and can take focus**". So `getInitialFocus` now +takes the designated element as a second argument and resolves the whole chain +in one call, filtering every step rather than just the last: + +```ts +// designated -> first form field -> the overlay window itself +getInitialFocus(content, designatedElement).focus({ preventScroll: true }) +``` + +Callers that were writing `initialFocus ?? getInitialFocus(content)` should +pass the designated element in instead — the `??` is what spent it on a +candidate that couldn't take focus. `@dunky.dev/dom-dialog` does this for every +DOM substrate already, so a dialog's `initialFocus` inherits the fix without a +change on the consumer's side. + +The predicate is `isRendered` from `@dunky.dev/dom-element`, shared with the +focus trap so the two can't disagree on what counts as rendered. From a06ed9b7d33b278eac27ea4bf0d5e0ee50eb05b2 Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Wed, 26 Aug 2026 16:08:57 +0200 Subject: [PATCH 4/5] =?UTF-8?q?docs(stories):=20containment=20story=20?= =?UTF-8?q?=E2=80=94=20page=20content=20beside=20a=20portalled=20layer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A modal dialog plus a non-modal panel portalled into an app branch that also holds page content: the branch is descended into rather than spared whole, so the article beside the panel is hidden individually while the panel stays reachable. The story's [inert] rule dims what containment hid, making the aria-hidden + inert state visible on the canvas. Mirrored in react and solid; recorded as skipped on native, where containment is the host Modal's own. Co-Authored-By: Claude Opus 5 (1M context) --- .../native/dialog/stories/dialog.stories.tsx | 6 +- .../react/dialog/stories/dialog.stories.tsx | 85 +++++++++++++++++++ .../solid/dialog/stories/dialog.stories.tsx | 85 +++++++++++++++++++ 3 files changed, 174 insertions(+), 2 deletions(-) diff --git a/packages/native/dialog/stories/dialog.stories.tsx b/packages/native/dialog/stories/dialog.stories.tsx index 1b3381d..b68210d 100644 --- a/packages/native/dialog/stories/dialog.stories.tsx +++ b/packages/native/dialog/stories/dialog.stories.tsx @@ -5,8 +5,10 @@ import { Dialog } from '@dunky.dev/native-dialog' // The story set mirrors packages/react/dialog — same names, same scenarios — // minus the ones whose premise doesn't exist on this host: `scoped` (no -// container portals; a Modal owns the whole screen) and `loginForm` (its -// point is the web focus trap; here the host manages focus). +// container portals; a Modal owns the whole screen), `loginForm` (its point is +// the web focus trap; here the host manages focus), and `containment` +// (aria-hidden + inert containment is a DOM mechanism; the host Modal hides +// the page whole, so there is no branch to descend into). const meta: Meta = { title: 'Primitives/Dialog', component: Dialog, diff --git a/packages/react/dialog/stories/dialog.stories.tsx b/packages/react/dialog/stories/dialog.stories.tsx index 9b27a00..aa022ed 100644 --- a/packages/react/dialog/stories/dialog.stories.tsx +++ b/packages/react/dialog/stories/dialog.stories.tsx @@ -522,3 +522,88 @@ export const nestedCloseOnBack: StoryType = { ), } + +// Containment makes everything outside the topmost modal layer invisible to +// assistive tech and unreachable by pointer, Tab, and find-in-page +// (`aria-hidden` + `inert`) — but it keeps painting, so the story's `[inert]` +// rule dims what containment hid to make the state visible. The panel is the +// interesting half: it is non-modal (a select menu's habitat) and portalled +// into the app branch, BESIDE page content. A branch holding a retained layer +// is descended into rather than spared whole, so the article next to the +// panel dims individually while the panel itself stays bright and reachable. +const appBranch: CSSProperties = { + // The panel's absolute viewport pins to the branch. + position: 'relative', + marginTop: 16, + padding: 16, + border: '1px dashed #999', + borderRadius: 8, +} +const branchViewport: CSSProperties = { + position: 'absolute', + inset: 0, + display: 'flex', + padding: 16, +} +const branchPanel: CSSProperties = { + margin: 'auto', + maxWidth: 320, + padding: 16, + background: 'white', + border: '1px solid #ccc', + borderRadius: 8, + boxShadow: '0 8px 32px rgba(0, 0, 0, 0.24)', +} + +const ContainedPage = () => { + const [branch, setBranch] = useState(null) + return ( + <> + +
+ Page content at the canvas root — a body-level cousin of the dialog's portal.{' '} + +
+
+
+ The app branch: the panel portals in here, right beside this article.{' '} + +
+
+ + Open dialog + + + + + + Containment + + Everything dimmed is aria-hidden and inert: Tab never reaches it, presses fall flat, + screen readers see only this window. Open the panel — it lands inside the app + branch, and the article beside it stays contained. + + {branch && ( + + Open panel in the app branch + + + + A non-modal layer above the dialog, held out of the containment while its + neighbor article stays in it. Escape closes this layer first. + + + + + )} + + + + + + ) +} + +export const containment: StoryType = { + render: () => , +} diff --git a/packages/solid/dialog/stories/dialog.stories.tsx b/packages/solid/dialog/stories/dialog.stories.tsx index 1053549..a1c71ba 100644 --- a/packages/solid/dialog/stories/dialog.stories.tsx +++ b/packages/solid/dialog/stories/dialog.stories.tsx @@ -525,3 +525,88 @@ export const nestedCloseOnBack: StoryType = { ), } + +// Containment makes everything outside the topmost modal layer invisible to +// assistive tech and unreachable by pointer, Tab, and find-in-page +// (`aria-hidden` + `inert`) — but it keeps painting, so the story's `[inert]` +// rule dims what containment hid to make the state visible. The panel is the +// interesting half: it is non-modal (a select menu's habitat) and portalled +// into the app branch, BESIDE page content. A branch holding a retained layer +// is descended into rather than spared whole, so the article next to the +// panel dims individually while the panel itself stays bright and reachable. +const appBranch: JSX.CSSProperties = { + // The panel's absolute viewport pins to the branch. + position: 'relative', + 'margin-top': '16px', + padding: '16px', + border: '1px dashed #999', + 'border-radius': '8px', +} +const branchViewport: JSX.CSSProperties = { + position: 'absolute', + inset: 0, + display: 'flex', + padding: '16px', +} +const branchPanel: JSX.CSSProperties = { + margin: 'auto', + 'max-width': '320px', + padding: '16px', + background: 'white', + border: '1px solid #ccc', + 'border-radius': '8px', + 'box-shadow': '0 8px 32px rgba(0, 0, 0, 0.24)', +} + +// The branch element fills its ref during render, before the dialog's effects +// run — the panel's portal reads a real element the moment it opens. +const ContainedPage = () => { + const [branch, setBranch] = createSignal(null) + return ( + <> + +
+ Page content at the canvas root — a body-level cousin of the dialog's portal.{' '} + +
+
+
+ The app branch: the panel portals in here, right beside this article.{' '} + +
+
+ + Open dialog + + + + + + Containment + + Everything dimmed is aria-hidden and inert: Tab never reaches it, presses fall flat, + screen readers see only this window. Open the panel — it lands inside the app + branch, and the article beside it stays contained. + + + Open panel in the app branch + + + + A non-modal layer above the dialog, held out of the containment while its + neighbor article stays in it. Escape closes this layer first. + + + + + + + + + + ) +} + +export const containment: StoryType = { + render: () => , +} From bbb04da6397f5e9a1641cbea9e2eb0c082c2965c Mon Sep 17 00:00:00 2001 From: Ivan Banov Date: Wed, 26 Aug 2026 17:32:24 +0200 Subject: [PATCH 5/5] fix(dom-element,dom-focus-trap,dom-overlay): bar fieldset-disabled and inert focus candidates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The focusability selectors gate on an element's own attributes, but disabling and inertness also arrive from ancestors: a control inside a fieldset[disabled] subtree, or anything inside [inert], satisfies the selector while a browser refuses to focus it — silently. In the trap that is a hard dead end: the Tab is already preventDefault()-ed when focus is stepped by hand, so every press recomputes the same refused target and focus never moves again. In the initial-focus chain it is the quieter miss: the barred field wins the draw and focus falls to the window past a viable later field. Add isFocusable to @dunky.dev/dom-element — :disabled resolves fieldset ancestry with the native first-legend exception, where the IDL property sees only the element's own attribute — and filter both callers with it, beside the isRendered facet they already share. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/dom-element-is-rendered.md | 11 ++++-- .../focus-candidates-barred-elements.md | 27 +++++++++++++++ packages/dom/utils/element/README.md | 25 +++++++++----- packages/dom/utils/element/SPEC.md | 26 ++++++++++---- packages/dom/utils/element/src/index.ts | 1 + .../dom/utils/element/src/is-focusable.ts | 17 ++++++++++ .../utils/element/tests/is-rendered.test.ts | 24 ++++++++++++- packages/dom/utils/focus-trap/SPEC.md | 13 ++++--- .../utils/focus-trap/src/get-focusables.ts | 10 +++--- .../utils/focus-trap/tests/trap-focus.test.ts | 34 +++++++++++++++++++ packages/dom/utils/overlay/SPEC.md | 17 ++++++---- .../utils/overlay/src/get-initial-focus.ts | 18 +++++----- .../utils/overlay/tests/containment.test.ts | 13 +++++++ 13 files changed, 195 insertions(+), 41 deletions(-) create mode 100644 .changeset/focus-candidates-barred-elements.md create mode 100644 packages/dom/utils/element/src/is-focusable.ts diff --git a/.changeset/dom-element-is-rendered.md b/.changeset/dom-element-is-rendered.md index 4c749ee..3407059 100644 --- a/.changeset/dom-element-is-rendered.md +++ b/.changeset/dom-element-is-rendered.md @@ -27,7 +27,14 @@ ancestors are walked — `visibility: hidden | collapse`, and being detached. Not checked: `opacity: 0` and `content-visibility`, which do render, and rendering is what decides focusability. -It's a package of its own because two utils have to agree on the answer: -`@dunky.dev/dom-focus-trap` filters its Tab cycle with it and +`isFocusable(element)` is the sibling facet: whether anything bars the element +from taking focus. Disabling and inertness also arrive from ancestors — a +control inside a `fieldset[disabled]` subtree (with the native exception for +its first `legend`) or anything inside `[inert]` refuses `focus()` — which a +selector's own-attribute checks (`:not([disabled])`) can't see. The facets are +deliberately narrow and compose; the tab order stays the caller's question. + +It's a package of its own because two utils have to agree on the answers: +`@dunky.dev/dom-focus-trap` filters its Tab cycle with them and `@dunky.dev/dom-overlay` filters its initial-focus candidates, both guarding against the same silent `focus()` no-op. diff --git a/.changeset/focus-candidates-barred-elements.md b/.changeset/focus-candidates-barred-elements.md new file mode 100644 index 0000000..3df9c6a --- /dev/null +++ b/.changeset/focus-candidates-barred-elements.md @@ -0,0 +1,27 @@ +--- +'@dunky.dev/dom-focus-trap': patch +'@dunky.dev/dom-overlay': patch +'@dunky.dev/dom-dialog': patch +--- + +Focus candidates barred by an ancestor are excluded: controls disabled through +`fieldset[disabled]` and anything inside `[inert]`. + +`FOCUSABLE_SELECTOR` and the form-field selector gate on an element's own +attributes (`input:not([disabled])`), but both bars also arrive from +ancestors, so a barred control satisfied the selector while a browser refuses +to focus it — silently. + +In the focus trap that was a hard dead end: the Tab keydown is already +`preventDefault()`-ed when focus is stepped by hand, so every press recomputed +the same refused target and focus never moved again. The cycle now only holds +what a browser would actually focus, keeping the native exception that +controls in a disabled fieldset's first `legend` stay enabled. + +In the initial-focus chain it was the quieter failure mode: the barred field +won the draw, `focus()` no-opped, and focus fell to the overlay window even +when a viable field came later. Every candidate — designated element and form +fields alike — is now also filtered for these bars. + +Both use the new `isFocusable` from `@dunky.dev/dom-element`, beside the +`isRendered` filter they already shared. diff --git a/packages/dom/utils/element/README.md b/packages/dom/utils/element/README.md index 7cd889a..2be341c 100644 --- a/packages/dom/utils/element/README.md +++ b/packages/dom/utils/element/README.md @@ -3,10 +3,14 @@ Framework-free predicates about a single element — the DOM questions more than one primitive has to ask, answered once so the answers can't drift. -`isRendered(element)` tells you whether an element actually rendered, and so -can take focus, be pressed, or be read out. Presence in the DOM is not enough: -an element inside a collapsed section still answers `querySelector`, but -`focus()` on it does nothing and reports nothing. +Two facets of "would a browser actually focus this element": + +- `isRendered(element)` — did it render? Presence in the DOM is not enough: an + element inside a collapsed section still answers `querySelector`, but + `focus()` on it does nothing and reports nothing. +- `isFocusable(element)` — is nothing barring it? Disabling and inertness also + arrive from ancestors (`fieldset[disabled]`, `[inert]`), which a selector's + own-attribute checks can't see. ## Install @@ -17,18 +21,23 @@ npm install @dunky.dev/dom-element ## Usage ```ts -import { isRendered } from '@dunky.dev/dom-element' +import { isFocusable, isRendered } from '@dunky.dev/dom-element' -// Pick the first field that can really take focus, not just the first match. +// Pick the first field a browser would really focus, not the first match. for (const field of content.querySelectorAll('input, select, textarea')) { - if (isRendered(field)) { + if (isFocusable(field) && isRendered(field)) { field.focus() break } } ``` -Checked: the `hidden` attribute (including `hidden="until-found"`), +`isRendered` checks: the `hidden` attribute (including `hidden="until-found"`), `display: none` on the element or any ancestor, `visibility: hidden | collapse`, and being detached. Not checked: `opacity: 0` and `content-visibility` — those render, and rendering is what decides focusability. + +`isFocusable` checks: `:disabled` (own attribute or an ancestor +`fieldset[disabled]`, keeping the native exception for controls in its first +`legend`) and `[inert]` on the element or any ancestor. Rendering is +`isRendered`'s question — the facets compose. diff --git a/packages/dom/utils/element/SPEC.md b/packages/dom/utils/element/SPEC.md index 7b3347e..8a0eda4 100644 --- a/packages/dom/utils/element/SPEC.md +++ b/packages/dom/utils/element/SPEC.md @@ -4,12 +4,14 @@ Framework-free predicates about a single element — the DOM questions more than one primitive has to ask, answered once so the answers can't drift. Today that -is one question: did this element actually render? +is two questions about whether a browser would actually focus an element: did +it render, and is it barred? -Two packages ask it for the same reason. `@dunky.dev/dom-focus-trap` filters +Two packages ask them for the same reason. `@dunky.dev/dom-focus-trap` filters the Tab cycle, and `@dunky.dev/dom-overlay` filters the initial-focus -candidates; both are guarding against the same silent failure, so they must -agree on what counts. +candidates; both are guarding against the same silent failure — a focus +candidate that looks right to a selector but refuses `focus()` without saying +so — so they must agree on what counts. ## Behavior @@ -23,12 +25,21 @@ agree on what counts. - `opacity: 0` and `content-visibility` are out of scope: those render. Whether something is _perceivable_ is a different question from whether it rendered at all, and only the latter decides focusability. +- An element is **focusable** when nothing bars it from taking focus: + neither disabled — its own attribute or an ancestor `fieldset[disabled]`, + with the native exception that controls in the fieldset's first `legend` + stay enabled — nor inside an `[inert]` element or subtree. A selector's + own-attribute checks (`:not([disabled])`) can't see either ancestry. +- The facets are deliberately narrow and compose: `isFocusable` doesn't ask + about rendering, `isRendered` doesn't ask about bars, and the tab order + (`tabIndex`) stays the caller's question. ## API -| Export | Description | -| --------------------- | -------------------------------------------------------------------------------- | -| `isRendered(element)` | Whether the element rendered, and so can take focus, be pressed, or be read out. | +| Export | Description | +| ---------------------- | ------------------------------------------------------------------------------------------ | +| `isRendered(element)` | Whether the element rendered, and so can take focus, be pressed, or be read out. | +| `isFocusable(element)` | Whether nothing bars the element from focus: not `:disabled`, not in an `[inert]` subtree. | ## Constraints @@ -44,3 +55,4 @@ agree on what counts. | Rendered-ness via a computed-style walk, not `Element.checkVisibility()` | The API is recent (Chrome/Edge 105+, Firefox 106+, Safari 17.4+) and a caller may resolve candidates after a Tab keydown's `preventDefault()`, so on a browser without it the throw would leave Tab dead entirely; the walk is spec-defined behavior everywhere, and needs no test-environment shim. | | Not `getClientRects().length` or `offsetParent` | Both are geometry, which test environments report as zeros — so nothing here would be covered. `offsetParent` is also `null` for a `position: fixed` element that is plainly visible, a false negative on exactly the overlay content this guards. | | The `hidden` attribute is checked with `closest`, not folded into the display walk | `hidden="until-found"` hides through `content-visibility`, not `display`, so the computed `display` of an element inside one is its own value. The attribute is the only signal. | +| Disabling is asked via `element.matches(':disabled')`, not the IDL property | The property reflects only the element's own attribute and misses `fieldset[disabled]` ancestry; the pseudo-class resolves it — first-`legend` exception included — for free. | diff --git a/packages/dom/utils/element/src/index.ts b/packages/dom/utils/element/src/index.ts index ebce5e9..0cdeff1 100644 --- a/packages/dom/utils/element/src/index.ts +++ b/packages/dom/utils/element/src/index.ts @@ -1 +1,2 @@ +export { isFocusable } from './is-focusable' export { isRendered } from './is-rendered' diff --git a/packages/dom/utils/element/src/is-focusable.ts b/packages/dom/utils/element/src/is-focusable.ts new file mode 100644 index 0000000..9567816 --- /dev/null +++ b/packages/dom/utils/element/src/is-focusable.ts @@ -0,0 +1,17 @@ +/** + * Whether an element can take focus at all — not barred by disabling or + * inertness. A focusability selector only sees an element's own attributes, + * but both bars also arrive from ancestors: a control inside a + * `fieldset[disabled]` subtree, or anything inside an `[inert]` one, refuses + * `focus()` — and refuses it silently. + * + * A deliberately narrow facet: renderedness is `isRendered`'s question, and + * the tab order is the caller's. The three compose. + */ +export function isFocusable(element: Element): boolean { + // `:disabled` resolves fieldset ancestry — including the native exception + // that controls in a disabled fieldset's first `legend` stay enabled — + // where the `disabled` IDL property reflects only the element's own + // attribute. + return !element.matches(':disabled') && element.closest('[inert]') === null +} diff --git a/packages/dom/utils/element/tests/is-rendered.test.ts b/packages/dom/utils/element/tests/is-rendered.test.ts index 379ba0b..558545c 100644 --- a/packages/dom/utils/element/tests/is-rendered.test.ts +++ b/packages/dom/utils/element/tests/is-rendered.test.ts @@ -1,6 +1,6 @@ // @vitest-environment jsdom import { afterEach, describe, expect, it } from 'vitest' -import { isRendered } from '@dunky.dev/dom-element' +import { isFocusable, isRendered } from '@dunky.dev/dom-element' const mount = (html: string): HTMLElement => { document.body.innerHTML = html @@ -35,3 +35,25 @@ describe('isRendered', () => { expect(isRendered(mount('
'))).toBe(true) }) }) + +describe('isFocusable', () => { + it('is true for a plain control', () => { + expect(isFocusable(mount(''))).toBe(true) + }) + + it('is false for a disabled control — own or through an ancestor fieldset', () => { + expect(isFocusable(mount(''))).toBe(false) + expect(isFocusable(mount('
'))).toBe(false) + }) + + it("is true inside a disabled fieldset's first legend — the native exception", () => { + expect( + isFocusable(mount('
')), + ).toBe(true) + }) + + it('is false inside an inert element or subtree', () => { + expect(isFocusable(mount(''))).toBe(false) + expect(isFocusable(mount('
'))).toBe(false) + }) +}) diff --git a/packages/dom/utils/focus-trap/SPEC.md b/packages/dom/utils/focus-trap/SPEC.md index 7354b15..a28ed00 100644 --- a/packages/dom/utils/focus-trap/SPEC.md +++ b/packages/dom/utils/focus-trap/SPEC.md @@ -36,10 +36,15 @@ identical containment. - With no focusables inside, Tab is a no-op; focus stays where it is. - `enabled` and `last` are re-evaluated on every press, so trapping follows runtime state — e.g. only the topmost layer of a stack traps. -- A focusable is an element matching `FOCUSABLE_SELECTOR` whose `tabIndex` - is not negative and which is rendered — the shared predicate from - [`@dunky.dev/dom-element`](../element/SPEC.md). Focusing a non-rendered - element is a no-op, so keeping one in the cycle would stall the trap on it. +- The cycle only holds what a browser would actually focus: an element + matching `FOCUSABLE_SELECTOR` whose `tabIndex` is not negative, which is + rendered, and which nothing bars — not disabled through an ancestor + `fieldset[disabled]` (controls in its first `legend` stay enabled, as they + do natively) and not in an `[inert]` element or subtree. The rendered and + barred questions are the shared predicates from + [`@dunky.dev/dom-element`](../element/SPEC.md). Focusing such an element + is a refused no-op and the Tab is already `preventDefault()`-ed, so + keeping one in the cycle would dead-end the trap on it. - A same-name radio group is one tab stop — the checked radio, else the group's first — per the APG radio group pattern; groups are scoped by name and form owner, matching the browser's own grouping. diff --git a/packages/dom/utils/focus-trap/src/get-focusables.ts b/packages/dom/utils/focus-trap/src/get-focusables.ts index 00ca1d5..c5fabd7 100644 --- a/packages/dom/utils/focus-trap/src/get-focusables.ts +++ b/packages/dom/utils/focus-trap/src/get-focusables.ts @@ -1,4 +1,4 @@ -import { isRendered } from '@dunky.dev/dom-element' +import { isFocusable, isRendered } from '@dunky.dev/dom-element' export const FOCUSABLE_SELECTOR: string = [ 'a[href]', @@ -27,9 +27,11 @@ export function getFocusables(container: HTMLElement): HTMLElement[] { const eligible: HTMLElement[] = [] for (let i = 0; i < candidates.length; i++) { const element = candidates[i]! - // Focusing a non-rendered element is a no-op, so keeping one in the cycle - // would stall the trap on it. - if (element.tabIndex >= 0 && isRendered(element)) { + // Three facets, all required: can take focus at all, in the tab order, + // and rendered. A barred or non-rendered element refuses focus silently, + // and the Tab is already preventDefault()-ed — keeping one in the cycle + // would dead-end the trap on it. + if (isFocusable(element) && element.tabIndex >= 0 && isRendered(element)) { eligible.push(element) } } diff --git a/packages/dom/utils/focus-trap/tests/trap-focus.test.ts b/packages/dom/utils/focus-trap/tests/trap-focus.test.ts index 675bb97..ec64e4b 100644 --- a/packages/dom/utils/focus-trap/tests/trap-focus.test.ts +++ b/packages/dom/utils/focus-trap/tests/trap-focus.test.ts @@ -123,6 +123,40 @@ describe('trapFocus', () => { expect(document.activeElement?.id).toBe('last') }) + it('excludes controls disabled by an ancestor fieldset, except in its first legend', () => { + // The selector only sees an element's own attributes; a control inside a + // disabled fieldset matches it, but a browser refuses to focus it — and + // the trap has already preventDefault()-ed, so the cycle would dead-end. + const container = mount( + '' + + '
' + + '' + + '' + + '
' + + '', + ) + document.getElementById('first')?.focus() + + // Controls in a disabled fieldset's first legend stay enabled natively. + pressTab(container) + expect(document.activeElement?.id).toBe('legend-input') + pressTab(container) + expect(document.activeElement?.id).toBe('last') + }) + + it('excludes inert elements and inert subtrees from the cycle', () => { + const container = mount( + '' + + '' + + '
' + + '', + ) + document.getElementById('first')?.focus() + + pressTab(container) + expect(document.activeElement?.id).toBe('last') + }) + it('includes iframes and a details summary in the cycle', () => { const container = mount( '' + diff --git a/packages/dom/utils/overlay/SPEC.md b/packages/dom/utils/overlay/SPEC.md index fba172b..139b251 100644 --- a/packages/dom/utils/overlay/SPEC.md +++ b/packages/dom/utils/overlay/SPEC.md @@ -61,13 +61,16 @@ collects input starts at its first form field (input, select, textarea); any other content keeps focus on the overlay window itself. A caller may designate an element ahead of both. -Every candidate must be **rendered**, not merely present. A field inside a -collapsed section satisfies the selector, yet `focus()` on it does nothing -and reports nothing, so accepting it would spend the candidate and drop focus -to the overlay window — the fallback firing on a miss it can't see. Each step -of the chain is therefore filtered, not just the last one; the predicate is -[`@dunky.dev/dom-element`](../element/SPEC.md)'s `isRendered`, shared with the -focus trap so the two can't disagree on what counts. +Every candidate must be one a browser would actually focus — rendered, and +not barred by an ancestor `fieldset[disabled]` or `[inert]`, which the +selector's own-attribute checks can't see. A field inside a collapsed +section or a disabled fieldset satisfies the selector, yet `focus()` on it +does nothing and reports nothing, so accepting it would spend the candidate +and drop focus to the overlay window — the fallback firing on a miss it +can't see. Each step of the chain is therefore filtered, not just the last +one; the predicates are [`@dunky.dev/dom-element`](../element/SPEC.md)'s +`isRendered` and `isFocusable`, shared with the focus trap so the two can't +disagree on what counts. ### The exit window diff --git a/packages/dom/utils/overlay/src/get-initial-focus.ts b/packages/dom/utils/overlay/src/get-initial-focus.ts index ff372a3..88ee2f2 100644 --- a/packages/dom/utils/overlay/src/get-initial-focus.ts +++ b/packages/dom/utils/overlay/src/get-initial-focus.ts @@ -1,4 +1,4 @@ -import { isRendered } from '@dunky.dev/dom-element' +import { isFocusable, isRendered } from '@dunky.dev/dom-element' // The strict rule is only that focus moves into the overlay: an overlay that // collects input starts at its first form field; any other content keeps @@ -8,22 +8,24 @@ const FORM_FIELD_SELECTOR = /** * Resolves where focus lands: the consumer's designated element, then the - * first form field, then the overlay window. Every candidate has to be - * *rendered*, not merely present — a field inside a collapsed section - * satisfies the selector, and `focus()` on it does nothing without saying so, - * which spends the candidate and drops focus to the window. Filtering each - * step keeps every fallback a real one. + * first form field, then the overlay window. Every candidate has to be one a + * browser would actually focus — rendered, and not barred by an ancestor + * `fieldset[disabled]` or `[inert]`, which the selector's own-attribute + * checks can't see. A candidate that fails these still satisfies the + * selector, and `focus()` on it does nothing without saying so, which spends + * the candidate and drops focus to the window. Filtering each step keeps + * every fallback a real one. */ export function getInitialFocus( content: HTMLElement, designated?: HTMLElement | null, ): HTMLElement { - if (designated != null && isRendered(designated)) return designated + if (designated != null && isFocusable(designated) && isRendered(designated)) return designated const fields = content.querySelectorAll(FORM_FIELD_SELECTOR) for (let i = 0; i < fields.length; i++) { const field = fields[i]! - if (isRendered(field)) return field + if (isFocusable(field) && isRendered(field)) return field } return content diff --git a/packages/dom/utils/overlay/tests/containment.test.ts b/packages/dom/utils/overlay/tests/containment.test.ts index bfba72a..ef46ed2 100644 --- a/packages/dom/utils/overlay/tests/containment.test.ts +++ b/packages/dom/utils/overlay/tests/containment.test.ts @@ -290,6 +290,19 @@ describe('getInitialFocus', () => { expect(getInitialFocus(content).id).toBe('field') }) + it('skips a field that cannot take focus — fieldset-disabled or inert', () => { + // The selector gates on own attributes only, so a field disabled through + // an ancestor fieldset (or barred by inert) satisfies it, yet a browser + // refuses to focus it — the same silent miss as an unrendered field. + const content = mountContent( + '
' + + '
' + + '', + ) + + expect(getInitialFocus(content).id).toBe('field') + }) + it('hands over to the fields when the designated element does not render', () => { // The contract conditions the designated element on being able to take // focus, so an unrendered one must not consume the chain down to the