Menu: move focus to the menu when the pointer leaves it entirely - #10448
Open
MILLERMARRU wants to merge 1 commit into
Open
Menu: move focus to the menu when the pointer leaves it entirely#10448MILLERMARRU wants to merge 1 commit into
MILLERMARRU wants to merge 1 commit into
Conversation
useMenuItem's onHoverStart moves the selection manager's focused key to the hovered item, but nothing ever clears it on hover-out. Once you've hovered an item, its key stays the focused key even after the pointer leaves the menu, so Enter can still activate an item you're no longer pointing at. Add an onHoverEnd on the menu container in useMenu that clears focusedKey when the pointer leaves the whole menu. useSelectableCollection already moves DOM focus to the collection container whenever focusedKey becomes null while the collection is still focused (existing path, used today when the focused item is removed from the collection), so this reuses that instead of adding new focus-restoration logic. Guarded against submenus: a submenu's popover isn't a DOM descendant of the parent menu, so moving the pointer from a trigger item toward its open submenu also leaves the parent menu's bounding box. Skip clearing focus while an item within the menu has aria-haspopup + aria-expanded=true, so the trigger stays focused while its submenu is open, matching current behavior. No new public props. Discussed in adobe#10143.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the loop on #10143.
What this does
Right now
useMenuItem's hover handling only moves focus in one direction:onHoverStartcallsselectionManager.setFocusedKey(key), but nothing clears it on hover-out, so the last-hovered item keeps the selection manager's focused key even after the pointer leaves the menu entirely. That's what lets Enter still activate an item you're no longer pointing at.I added an
onHoverEndon the menu container itself (useMenu), not on individual items. When the pointer leaves the whole menu, it clearsfocusedKeyback tonull. I didn't have to touch focus-restoration logic for that:useSelectableCollectionalready moves real DOM focus to the collection container wheneverfocusedKeybecomesnullwhile the collection is still focused (existing code, used today for the "last item removed" case), so clearing the key is enough to get focus onto the<Menu>itself, matching the native/Base UI behavior discussed above.Guarded one case: if the pointer is leaving the menu because it's moving toward an already-open submenu, don't clear. The submenu popover renders outside the parent menu's DOM (it's a separate popover, not a descendant), so the pointer leaving the parent's bounding box is indistinguishable at the DOM level from actually leaving the whole widget. I check for
[aria-haspopup][aria-expanded="true"]inside the menu's own subtree before clearing, so a trigger stays visually focused while its submenu is open, same as today.No new public props. Kept it to the two files that own this behavior:
packages/react-aria/src/menu/useMenu.tspackages/react-aria-components/test/Menu.test.tsx(tests)What I checked against the ramifications you mentioned
useSafelyMouseToSubmenukeeps working unmodified, I didn't touch it. Added a test (should keep the submenu trigger focused when the pointer leaves the parent menu towards its open submenu) that opens a submenu via hover then firespointerleaveon the parent and asserts the trigger keepsdata-focused.useSelectableCollectionis already gated on!shouldUseVirtualFocus, so in virtual focus mode clearingfocusedKeyonly drops thearia-activedescendanthighlight, it never touches real DOM focus, which stays on the input. I didn't add a dedicated Autocomplete integration test for this one, flagging it explicitly since it's exactly the case you called out and I'd rather you or someone with more Autocomplete test coverage context sanity-check it than have me assert something I haven't fully exercised.<Menu>level has its ownuseTreeState/selection manager, the same mechanism recurses naturally, leaving a nested submenu clears that level's focus onto its own container without touching parent levels.What I did not try to fix
The discussion also mentions the first item flashing a focus state during the menu's closing transition. That's a separate bug from the hover-follows-mouse one, didn't touch it here to keep this change scoped to what we discussed.
Testing
yarn jest packages/react-aria-components/test/Menu.test.tsx— 111/111 passing, including the two new tests.yarn jest packages/react-aria-components/test/Autocomplete packages/react-aria/test/menu— 106 passing/3 pre-existing skips, no regressions, this is what exercises the virtual focus path.yarn check-types(repo-wide) andoxlinton the changed files — both clean.Open to a different mechanism if you'd rather not lean on the existing "focusedKey null -> focus container" path, but reusing it kept the diff small and didn't require introducing new state.