Skip to content

[menu] Add highlightItem('first' | 'last' | 'none') imperative action#4815

Open
chsmc-ant wants to merge 6 commits into
mui:masterfrom
chsmc-ant:menu-actionsref-setactiveindex
Open

[menu] Add highlightItem('first' | 'last' | 'none') imperative action#4815
chsmc-ant wants to merge 6 commits into
mui:masterfrom
chsmc-ant:menu-actionsref-setactiveindex

Conversation

@chsmc-ant
Copy link
Copy Markdown

@chsmc-ant chsmc-ant commented May 12, 2026

When a menu is opened programmatically (via controlled open, or MenuHandle.open(), or onOpenChange(true) from a custom interaction that isn't one of Menu.Trigger's built-in open keys), useListNavigation leaves activeIndex as null. This is correct for pointer-driven opens, but for custom keyboard shortcuts it means every item renders with tabindex="-1" and FloatingFocusManager moves 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.Root actionsRef — alongside unmount / close:

    const actionsRef = React.useRef<Menu.Root.Actions>(null);
    
    function onCustomKeyDown(event: React.KeyboardEvent) {
      if (event.key === 'ArrowRight') {
        setOpen(true);
        actionsRef.current?.highlightItem('first');
      }
    }
    
    <Menu.Root open={open} onOpenChange={setOpen} actionsRef={actionsRef}>
  • MenuHandle.open(triggerId, highlightItem?) — same values as a second parameter:

    handle.open('my-trigger', 'first');

The requested target is stored as pendingHighlightItem on MenuStore and resolved inside useListNavigation's initial-sync layout effect once the list is populated, using getMinListIndex / getMaxListIndex so hidden items are skipped — the same logic used for the built-in focusItemOnOpen path. useListNavigation then moves DOM focus to the resolved item (for Menu; virtual-focus consumers like Combobox would only update activeIndex).

A Menu.Root.HighlightItem type 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


Earlier revisions exposed a raw setActiveIndex(index) setter and then a focusItem() action; reworked per review to use highlightItem() with directional targets and to cover MenuHandle.open as well.

@code-infra-dashboard
Copy link
Copy Markdown

code-infra-dashboard Bot commented May 12, 2026

Bundle size

Bundle Parsed size Gzip size
@base-ui/react 🔺+569B(+0.12%) 🔺+149B(+0.10%)

Details of bundle changes

Performance

Total duration: 1,221.72 ms +129.57 ms(+11.9%) | Renders: 50 (+0) | Paint: 1,867.22 ms +203.44 ms(+12.2%)

Test Duration Renders
Tabs mount (200 instances) 251.75 ms 🔺+49.19 ms(+24.3%) 4 (+0)
Checkbox mount (500 instances) 82.72 ms 🔺+20.89 ms(+33.8%) 1 (+0)
Tooltip mount (300 contained roots) 60.65 ms 🔺+15.25 ms(+33.6%) 1 (+0)

9 tests within noise — details


Check out the code infra dashboard for more information about this PR.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 12, 2026

Deploy Preview for base-ui ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit e540436
🔍 Latest deploy log https://app.netlify.com/projects/base-ui/deploys/6a0c79695624ca0008feac6c
😎 Deploy Preview https://deploy-preview-4815--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@oliviertassinari oliviertassinari added the component: menu Changes related to the menu component. label May 12, 2026
@oliviertassinari oliviertassinari changed the title [Menu] Expose setActiveIndex on Menu.Root actionsRef [menu] Expose setActiveIndex on Menu.Root actionsRef May 12, 2026
@oliviertassinari oliviertassinari changed the title [menu] Expose setActiveIndex on Menu.Root actionsRef [menu] Expose setActiveIndex() on Menu.Root actionsRef May 12, 2026
@oliviertassinari
Copy link
Copy Markdown
Member

oliviertassinari commented May 12, 2026

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.mov

It'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 open prop.

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.

@chsmc-ant chsmc-ant force-pushed the menu-actionsref-setactiveindex branch from 60e54e8 to e1cad93 Compare May 12, 2026 23:09
@chsmc-ant chsmc-ant changed the title [menu] Expose setActiveIndex() on Menu.Root actionsRef [Menu] Always highlight the first item on open May 12, 2026
@chsmc-ant chsmc-ant force-pushed the menu-actionsref-setactiveindex branch from e1cad93 to 60e54e8 Compare May 12, 2026 23:11
@chsmc-ant chsmc-ant changed the title [Menu] Always highlight the first item on open [Menu] Expose setActiveIndex on Menu.Root actionsRef May 12, 2026
@chsmc-ant
Copy link
Copy Markdown
Author

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.

@oliviertassinari oliviertassinari changed the title [Menu] Expose setActiveIndex on Menu.Root actionsRef [menu] Expose setActiveIndex on Menu.Root actionsRef May 12, 2026
@atomiks atomiks added the type: new feature Expand the scope of the product to solve a new problem. label May 14, 2026
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 14, 2026

commit: d8abaf8

@atomiks
Copy link
Copy Markdown
Contributor

atomiks commented May 14, 2026

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 autoHighlight (which Autocomplete has) since this is a one-shot/event-driven action that probably shouldn't need state.

The only issue is naming: setActiveIndex is quite broad and "active" is more so an internal name as we use highlighted or focus publicly.

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 ArrowUp for example.

Possible alternative: .focusItem('first' | 'last' | 'none') or .focusItem('start' | 'end' | 'none')

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 }))

@michaldudak
Copy link
Copy Markdown
Member

We have another imperative API that has the same issue: MenuHandle.open. Whatever solution we land on for the actionsRef, we should also apply it there. I like @atomiks idea with .focusItem('first' | 'last' | 'none'). We could add an optional parameter to the handle's open method: open(triggerId: string, focusItem?: 'first' | 'last' | 'none').

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.
@chsmc-ant chsmc-ant changed the title [menu] Expose setActiveIndex on Menu.Root actionsRef [Menu] Add focusItem('first' | 'last' | 'none') imperative action May 15, 2026
@chsmc-ant
Copy link
Copy Markdown
Author

Thanks for the feedback @atomiks @michaldudak — pushed 974094c which:

  • Replaces setActiveIndex(index) with focusItem('first' | 'last' | 'none') on actionsRef, resolved via getMinListIndex/getMaxListIndex so hidden items are skipped.
  • Adds the same option as a second parameter to MenuHandle.open(triggerId, focusItem?).
  • Exports Menu.Root.FocusItem for the union type.

PR description updated to match. Happy to bikeshed 'first' | 'last' vs 'start' | 'end' if there's a preference.

@atomiks atomiks force-pushed the menu-actionsref-setactiveindex branch from 3f70508 to d8abaf8 Compare May 18, 2026 05:17
pendingFocusItem,
onPendingFocusItemChange() {
store.set('pendingFocusItem', null);
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Folded this into useListNavigation as it removes the duplicate effect, and AriaCombobox will likely want this behavior as well: #4745

@atomiks atomiks force-pushed the menu-actionsref-setactiveindex branch from d8abaf8 to 77d7be1 Compare May 18, 2026 05:28
@atomiks
Copy link
Copy Markdown
Contributor

atomiks commented May 18, 2026

I think .highlightItem() is likely a better term for this, because the public API is about the highlighted state, even though focus is in sync with it. And AriaCombobox doesn't use DOM focus, so highlight makes more sense if this is to be added there — @michaldudak

@chsmc-ant
Copy link
Copy Markdown
Author

thanks @atomiks happy to go ahead and implement that rename if you think it's the right approach

@chsmc-ant chsmc-ant requested a review from aarongarciah as a code owner May 19, 2026 14:44
@chsmc-ant chsmc-ant changed the title [Menu] Add focusItem('first' | 'last' | 'none') imperative action [Menu] Add highlightItem('first' | 'last' | 'none') imperative action May 19, 2026
@chsmc-ant
Copy link
Copy Markdown
Author

Renamed to highlightItem in f4aa2a8 — covers actionsRef.highlightItem(), MenuHandle.open(triggerId, highlightItem?), the exported Menu.Root.HighlightItem type, and the internal pendingHighlightItem plumbing through MenuStore / useListNavigation. Left the pre-existing internal focusItem/resolveFocusItem helpers in useListNavigation alone since those are about DOM focus and shared with the focusItemOnOpen path.

@oliviertassinari oliviertassinari changed the title [Menu] Add highlightItem('first' | 'last' | 'none') imperative action [menu] Add highlightItem('first' | 'last' | 'none') imperative action May 19, 2026
@oliviertassinari
Copy link
Copy Markdown
Member

oliviertassinari commented May 20, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: menu Changes related to the menu component. type: new feature Expand the scope of the product to solve a new problem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants