Repository navigation
fix: restore focus when overlays begin exiting - #10955
bhumikabiyani wants to merge 3 commits into
Conversation
snowystinger
left a comment
There was a problem hiding this comment.
Test instructions are not to be which unit tests to run, they are how a human can verify the fix manually.
I don't see any mention of previous attempts at this work, were you unable to find any?
| restoreFocus?: boolean, | ||
| contain?: boolean | ||
| contain?: boolean, | ||
| isExiting?: boolean |
There was a problem hiding this comment.
isExiting makes no sense in FocusScope. Is it actually disabling the focus scope?
There was a problem hiding this comment.
Agreed. I re-evaluated the layering and agreed that isExiting does not belong in @react-aria/focus or FocusScopeProps.
I have completely reverted all changes to FocusScope.tsx and FocusScope.test.js back to main. Early focus restoration is now cleanly encapsulated inside Overlay (@react-aria/overlays), which already receives isExiting from Popover and Modal. When isExiting transitions to true while mounted, Overlay synchronously restores focus to nodeToRestore if activeElement is still within the exiting overlay. When FocusScope unmounts after the exit animation ends, focus has already returned outside the scope, so FocusScope's unmount cleanup safely no-ops without any double restoration. FocusScopeProps remains completely untouched.
|
Thank you for the guidance! I have updated the PR description with:
In addition, I addressed your review comment on |
Closes #10943
Summary
Problem
When an overlay with an exit animation (such as a RAC Menu popover or Modal dialog) is closed using the Escape key, focus previously remained on the active element inside the closing overlay (e.g. a MenuItem) for the duration of the CSS exit animation (typically 200–300 ms).
If a user pressed Enter or another key during this interval, intending to reopen the menu or activate its trigger, the closing MenuItem's
onActioncallback could be triggered unexpectedly.Root Cause
When an overlay begins its exit animation:
Overlaydisables focus containment viacontain={(props.shouldContainFocus || contain) && !isExiting}.FocusScopeonly restores focus during its unmount cleanup.document.activeElementcontinues pointing to an element inside the closing overlay until it unmounts.Solution & Architecture
isExitingorisDisabledprop is added to the publicFocusScopePropsAPI. The animation lifecycle remains encapsulated withinOverlay(@react-aria/overlays), which already receivesisExitingfromPopoverandModal. To avoid duplicate scope tree traversal logic, canonical FocusScope helpers (isElementInChildScopeandrestoreFocusToElement) are exported as/** @private */.queueMicrotaskand cancellation: InOverlay.tsx, focus restoration is scheduled viaqueueMicrotaskwhenisExitingtransitions totrue. This ensures synchronous browser event dispatch and user interaction events on the closing turn (such as clicking an outside input, or complex touch drag resizers) settle cleanly before inspectingactiveElement. AnisCancelledflag in the effect cleanup prevents the queued microtask from stealing focus if the overlay is reopened or unmounted before the microtask executes.focusScopeTree.traverse()withnode.nodeToRestore === nodeToRestore && isElementInChildScope(activeElement, node.scopeRef).useSelect.tsblur handling: When early restoration returns focus to the select trigger button while the listbox overlay is still mounted in its exit animation,useSelect'smenuProps.onBlurchecksnodeContains(ref.current, e.relatedTarget)to recognize that focus is still within the component, preventing premature blur callbacks.SelectTesterandMenuTestertest-utility timing: In@react-aria/test-utils,SelectTester.close(),SelectTester.toggleOptionSelection(),MenuTester.close(), andMenuTester.toggleOptionSelection()now assert DOM unmounting insidewaitFor(() => ...). Because focus is now restored on frame 0 during exit transitions rather than on unmount, test assertions relying on focus movement as a proxy for DOM unmounting now explicitly await DOM removal.isExitingonly when closing:PopoverandModalpassisExiting={!state.isOpen && isExiting}so early restoration only runs when the overlay is logically closing.Prior Attempts & Context
db6c9347479by @snowystinger): Addedpointer-events: noneto.spectrum-Popover.is-exitingto prevent mouse clicks on closing popovers from selecting items. This addressed pointer interactions but did not handle keyboard focus remaining on the active menu item during exit transitions.77fbf106d41(PR Table column resizing support for RAC #4785 by @devongovett): AddedisExitingtoOverlayPropsand disabled focus containment during exit (contain && !isExiting), allowing focus to leave an exiting overlay. However, focus restoration was not triggered early, so focus remained inside the exiting overlay until unmount.inert: Applying the HTMLinertattribute to an exiting overlay was evaluated; however, per WHATWG HTML specification §7.4.3, making a focused element inert forces the browser to reset focus todocument.bodyrather than restoring to the opening trigger, stranding keyboard focus.useMenuTrigger/useSelectableCollectionautofocus lifecycle and is intentionally kept separate. This PR verifies that completion of the previous menu's exit animation does not steal focus after the menu has been reopened.Test Instructions (Manual Verification)
Scenario 1: RAC Menu with CSS Exit Animation
Original reproduction: StackBlitz – Menu focus remains in the closing Menu
onActioncallback.Scenario 2: Dialog / Modal with Exit Animation
Scenario 3: Outside Interaction During Exit Animation
<input>or button.Automated Tests Added
packages/react-aria-components/test/Menu.test.tsxrestores focus to trigger immediately on Escape during exit animation without invoking onActionunmounts normally after exit animation completes and preserves trigger focusrestores focus to parent submenu item when nested submenu is dismissed with Escape during exit animationpreserves focus on an external input when clicked to dismiss menu during exit animationdoes not restore focus back to trigger after Menu is reopened during exit animation and animation completespackages/react-aria-components/test/Dialog.test.jsrestores focus to trigger immediately on Escape during exit animationVerification Results
yarn format:check: Passed (6,545 files checked, 0 errors).yarn lint: Passed with exit code 0 (2,847 files checked, 0 errors, 0 warnings).yarn check-types: Passed with exit code 0.Unit tests (514 passed across 10 suites):
packages/react-aria/test/focus/FocusScope.test.jspackages/react-aria-components/test/Menu.test.tsxpackages/react-aria-components/test/Dialog.test.jspackages/react-aria-components/test/Popover.test.jspackages/@adobe/react-spectrum/test/menu/MenuTrigger.test.jspackages/react-aria/test/overlays/useOverlay.test.jspackages/@adobe/react-spectrum/test/overlays/Overlay.test.jspackages/@adobe/react-spectrum/test/picker/Picker.test.jspackages/@adobe/react-spectrum/test/picker/TempUtilTest.test.jspackages/@adobe/react-spectrum/test/table/TableSizing.test.tsxRelease Impact
[✓] Fixes a bug or improves existing code without changing how apps use it.
PR Checklist
[✓] I followed the contributing guide: https://github.com/adobe/react-spectrum/blob/main/CONTRIBUTING.md
[✓] I tested this change locally and included the results in the Test Instructions section.
[✓] I understand and can explain every change in this PR. If I used AI assistance, I followed the AI contribution guidance: https://github.com/adobe/react-spectrum/blob/main/CONTRIBUTING.md#ai-assisted-contributions
Project
Adobe React Spectrum / Open Source Contribution