[menu] Add highlightItem('first' | 'last' | 'none') imperative action#4815
[menu] Add highlightItem('first' | 'last' | 'none') imperative action#4815chsmc-ant wants to merge 6 commits into
Conversation
Bundle size
PerformanceTotal duration: 1,221.72 ms +129.57 ms(+11.9%) | Renders: 50 (+0) | Paint: 1,867.22 ms +203.44 ms(+12.2%)
9 tests within noise — details Check out the code infra dashboard for more information about this PR. |
✅ Deploy Preview for base-ui ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
To help the team a bit (and your team on Claude), here is a reproduction that seems to reproduce the problem: https://stackblitz.com/edit/va1fcbzq?file=src%2FApp.tsx (repro based on this demo): Screen.Recording.2026-05-13.at.00.37.47.movIt's not clear that this should be solved with a new API; this feels like a bug: I would expect the same behavior regardless of how the menu is open: with a trigger, with the For example, in https://mui.com/material-ui/react-menu/, the focus is placed on the first menu item when the menu opens (with the keyboard) but the behavior can be configured with a prop. Let's wait for team inputs. |
60e54e8 to
e1cad93
Compare
e1cad93 to
60e54e8
Compare
|
Thanks @oliviertassinari and good call! I opened #4816 as well so we can compare this approach to that one. Will defer to the team on which they'd prefer. |
commit: |
|
Thanks for the PR @chsmc-ant Makes sense as controlled-opens are assumed to always be pointer-driven, but a keyboard-driven controlled open is also valid in which case the highlight should be set on the first item. An imperative action also makes more sense than a prop like The only issue is naming: I'm also not sure if exposing the index is a good idea, because 1) disabled items should be skipped on initial open, which should pass through the internal logic to skip them, and 2) it's likely hard to keep track of which index is the last item if the controlled open is an Possible alternative: cc: @colmtuite Side note: in case you need this behavior before release, this technically already works, if hacky: firstItem.focus();
// or firstItem.dispatchEvent(new MouseEvent('mousemove', { bubbles: true })) |
|
We have another imperative API that has the same issue: |
Addresses review feedback: - Rename to focusItem and use directional targets instead of a raw index, so disabled/hidden items are skipped via getMinListIndex/getMaxListIndex. - Add an optional focusItem parameter to MenuHandle.open() with the same values.
|
Thanks for the feedback @atomiks @michaldudak — pushed 974094c which:
PR description updated to match. Happy to bikeshed |
3f70508 to
d8abaf8
Compare
| pendingFocusItem, | ||
| onPendingFocusItemChange() { | ||
| store.set('pendingFocusItem', null); | ||
| }, |
There was a problem hiding this comment.
Folded this into useListNavigation as it removes the duplicate effect, and AriaCombobox will likely want this behavior as well: #4745
d8abaf8 to
77d7be1
Compare
|
I think |
|
thanks @atomiks happy to go ahead and implement that rename if you think it's the right approach |
|
Renamed to |
|
I wonder how we will fix this demo https://base-ui.com/react/components/menu#controlled-mode-with-multiple-triggers later on. I guess we will add something like: <button
type="button"
className={styles.Button}
onClick={(event) => {
// Detect if the interaction was explicitly driven by a keyboard. Unless we need isVirtualClick().
const isKeyboard = event.detail === 0;
setOpen(true);
actionsRef.current!.highlightItem(isKeyboard ? 'first' : null);Not fully self managed by the popover (testing for :focus-visible), but this stays kind of simple. |
When a menu is opened programmatically (via controlled
open, orMenuHandle.open(), oronOpenChange(true)from a custom interaction that isn't one ofMenu.Trigger's built-in open keys),useListNavigationleavesactiveIndexasnull. This is correct for pointer-driven opens, but for custom keyboard shortcuts it means every item renders withtabindex="-1"andFloatingFocusManagermoves focus to the popup container rather than the first item — the user then has to press ↓ once more before arrow navigation works.This PR adds an imperative
highlightItem(target: 'first' | 'last' | 'none')action so callers can ask the menu to highlight an item once it opens:Menu.RootactionsRef— alongsideunmount/close:MenuHandle.open(triggerId, highlightItem?)— same values as a second parameter:The requested target is stored as
pendingHighlightItemonMenuStoreand resolved insideuseListNavigation's initial-sync layout effect once the list is populated, usinggetMinListIndex/getMaxListIndexso hidden items are skipped — the same logic used for the built-infocusItemOnOpenpath.useListNavigationthen moves DOM focus to the resolved item (for Menu; virtual-focus consumers like Combobox would only updateactiveIndex).A
Menu.Root.HighlightItemtype alias is exported for the'first' | 'last' | 'none'union.Preview: https://deploy-preview-4815--base-ui.netlify.app/react/components/menu#controlled-mode-with-multiple-triggers