[#1747] Fix aria-hidden blocked when isolating subtrees due to initial focus delay by stefcameron · Pull Request #1748 · focus-trap/focus-trap

stefcameron · 2026-01-24T23:51:31Z

Fixes #1747 (relates to #1575)

Also re-orders when addListeners(), subtree isolation, and updateObservedNodes() are called to make it consistent in all cases, always in the same order, for both activation and un-pausing.

The changeset entry in this PR explains everything (copied here for convenience)...

Breaking: Fixed a long-standing bug where onPostActivate() would be called before the initial focus node was focused and the trap was fully activated.
- By default (and for many years now), a trap delays setting focus to the initial focus node to the next frame (setTimeout(0)) but wasn't delaying calling onPostActivate() until after that delay.
- With the new isolateSubtrees='aria-hidden' option, the currently-focused node's container (a non-subtree being "disabled") would get hidden before the delay was up, resulting in Chrome preventing the effect of aria-hidden on that subtree with a warning in the console due to the container being hidden still containing focus (e.g. the "activate trap" button).
- With the fix, subtree isolation and the call to onPostActivate() await the initial focus delay (if there is one, which is default behavior; remove it with delayInitialFocus=false) before being applied/called.
- This may cause tests to fail, requiring the addition of slight delays before testing a given state (e.g. await waitFor(() => expect(initialFocusNode).toBeFocused()).
- It may also disrupt current assumptions about the state of the initial focus node in code that runs in your onPostActivate() handler (prior to this change, that node would not be focused yet; after this change, it will be focused).

PR Checklist

Please leave this checklist in your PR.

Issue being fixed is referenced.
Source changes maintain stated browser compatibility.
Web APIs introduced have deep browser coverage, including Safari (often very late to adopt new APIs).
Includes updated docs demo bundle if source/docs code was changed (run npm run demo-bundle in your branch and include the /docs/demo-bundle.js file that gets generated in your PR).
Unit test coverage added/updated.
E2E (i.e. demos) test coverage added/updated.
- ⚠️ Non-covered demos (look for // TEST MANUALLY comments here) that can't be fully tested in Cypress have been manually verified.
Typings added/updated.
Changes do not break SSR:
- Careful to test typeof document/window !== 'undefined' before using it in code that gets executed on load.
README updated (API changes, instructions, etc.).
Changes to dependencies explained.
Changeset added (run npm run changeset locally to add one, and follow the prompts).
- EXCEPTION: A Changeset is not required if the change does not affect any of the source files that produce the package bundle. For example, demo changes, tooling changes, test updates, or a new dev-only dependency to run tests more efficiently should not have a Changeset since it will not affect package consumers.

Summary by CodeRabbit

Breaking Changes
- onPostActivate() now runs after any initial-focus delay; handlers may need timing adjustments.
New Features
- Activation/unpause sequencing now ensures initial focus is applied before subtree isolation.
- Optional control to delay initial focus (defaults to enabled).
Bug Fixes
- Reduced timing conflicts between focus placement and subtree isolation to prevent focus/aria-hidden issues.
Tests
- Tests updated to await asynchronous focus behavior.
Documentation
- Demo text and examples updated to reflect new focus timing.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…l focus delay Fixes #1747 (relates to #1575) The changeset entry explains everything... Also re-orders when `addListeners()`, subtree isolation, and `updateObservedNodes()` are called to make it consistent in all cases, always in the same order, for both activation and un-pausing.

changeset-bot · 2026-01-24T23:51:35Z

🦋 Changeset detected

Latest commit: 8be7b5a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
focus-trap	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

coderabbitai · 2026-01-24T23:54:49Z

Walkthrough

Activation and unpause flows were restructured to await initial focus placement before applying subtree isolation (aria-hidden/inert). addListeners now returns a Promise when delayInitialFocus is enabled; subtree isolation and onPostActivate are invoked after the initial focus has been moved.

Changes

Cohort / File(s)	Summary
Core async activation flow `index.js`	`addListeners` now returns a Promise that resolves after initial focus is applied (or immediately if `delayInitialFocus` is false). Activation/unpause flows await this promise before enabling subtree isolation and calling post-activation hooks.
Demo bundle async sequencing `docs/demo-bundle.js`	Mirrors core sequencing: `finishActivation`/`finishUnpause` are async, await `addListeners`, then enable subtree isolation and call post-activation hooks. Initial-focus timing adjusted to avoid aria-hidden focus warnings.
Demo post-activation hooks `docs/js/delay.js`, `docs/js/no-delay.js`	Added `onPostActivate()` handlers that record whether the close button was focused after activation. `no-delay.js` also prevents default on Enter activation to avoid immediate deactivation.
Tests updated for timing `test/suites/isolateSubtrees.test.js`	Replaced direct boolean assertions with `waitFor` to accommodate asynchronous DOM/focus updates and avoid brittle timing assumptions.
Demo E2E assertions `cypress/e2e/focus-trap-demo.cy.js`	Added assertions that verify `data-hide-is-focused-on-post-activate` is set on containers for both delay and no-delay demos after activation.
Docs & changelog `.changeset/shy-plums-burn.md`, `docs/index.html`	Added breaking-change changelog entry describing delayed `onPostActivate()` semantics and `delayInitialFocus` behavior; minor wording tweak in "No delay" demo description.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant User as User (click)
participant Button as Activate Button
participant Trap as FocusTrap
participant DOM as DOM / Subtrees
participant Hook as onPostActivate
Note over Button,Trap: Activation starts
User->>Button: click/keydown
Button->>Trap: activate()
Trap->>Trap: updateTabbableNodes()
Trap->>Trap: addListeners() returns Promise (delayed if delayInitialFocus)
Trap->>DOM: focus(initialFocusNode)
DOM-->>Trap: focus applied
Trap->>DOM: apply subtree isolation (aria-hidden/inert)
Trap->>Hook: onPostActivate()
Hook-->>User: post-activation checks complete

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and specifically captures the main fix: ensuring aria-hidden subtree isolation doesn't block focus due to initial focus delay timing.
Description check	✅ Passed	The description comprehensively explains the breaking change, the root cause, the fix, and impacts on tests and onPostActivate behavior. The PR checklist is completed with all relevant items addressed.
Linked Issues check	✅ Passed	The PR fully addresses issue `#1747` by ensuring focus moves into the trap before aria-hidden is applied, eliminating the timing conflict that caused Chrome to block aria-hidden on the element containing the activate button.
Out of Scope Changes check	✅ Passed	All changes directly support the core objective: making initial focus timing and subtree isolation sequencing consistent and correct. The updated tests, docs, demos, and internal flow adjustments are all in-scope and necessary for the fix.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@docs/js/no-delay.js`:
- Around line 11-18: The comment in the onPostActivate() method contains a
copy-paste artifact referencing "delay.js"; update the comment to accurately
describe this file (no-delay.js) — locate the onPostActivate function and
replace the incorrect sentence ("`no-delay.js` demo has the same check") with a
corrected description (e.g., mention this demo itself performs the check or
remove the reference) so the comment reflects the current file context.

🧹 Nitpick comments (1)

index.js (1)
1064-1089: Async finishActivation solves the Chrome warning—but fire-and-forget could mask errors.

The async finishActivation correctly awaits addListeners() before applying subtree isolation, which fixes the issue. However, on line 1089, finishActivation() is called without await, meaning any errors thrown inside will become unhandled Promise rejections rather than propagating to the caller.

This is probably intentional for backwards compatibility (the activate() method returns this synchronously), but it's worth noting that errors in onPostActivate or other async steps won't be catchable by the caller.

If error visibility is desired, consider adding a .catch() handler:
finishActivation().catch((err) => {
  console.error('focus-trap: error during activation:', err);
});

docs/js/no-delay.js

coderabbitai bot reviewed Jan 24, 2026

View reviewed changes

docs/js/no-delay.js Show resolved Hide resolved

Fix typo in no-delay.js demo comment

8be7b5a

stefcameron merged commit b6ea4b5 into master Jan 28, 2026
3 checks passed

stefcameron deleted the fix-activation-delay branch January 28, 2026 16:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

[#1747] Fix aria-hidden blocked when isolating subtrees due to initial focus delay#1748

[#1747] Fix aria-hidden blocked when isolating subtrees due to initial focus delay#1748
stefcameron merged 2 commits intomasterfrom
fix-activation-delay

stefcameron commented Jan 24, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

changeset-bot bot commented Jan 24, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Jan 24, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

Conversation

stefcameron commented Jan 24, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

changeset-bot bot commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

coderabbitai bot commented Jan 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

stefcameron commented Jan 24, 2026 •

edited by coderabbitai bot

Loading

changeset-bot bot commented Jan 24, 2026 •

edited

Loading

coderabbitai bot commented Jan 24, 2026 •

edited

Loading