[utils] Explicitly register roving tab items with parent by mj12albert · Pull Request #48122 · mui/material-ui

mj12albert · 2026-03-28T23:52:49Z

Migration doc: https://deploy-preview-48122--material-ui.netlify.app/material-ui/migration/upgrade-to-v9/#menu-and-menulist

Previews:

Notable changes:

1. Roving items explicitly opt in

Previously, the parent (e.g. MenuList) computes and manages child indexes, injecting ref and tabindex into each children. Now an additional useRovingTabIndexItem hook is available for roving items to explicitly register themselves with the parent. This way the parent doesn't have to scan children to determine out whether something is a roving item or not. (Also a child hook wouldn't have worked inside React.Children.map)

This makes muiSkipListHighlight unnecessary, and easier to support conditional rendering/Fragments inside Menus

The idea and mechanics here are borrowed from Base UI - registry map of roving item refs, DOM order tracking

2. Boundary between useRovingTabIndex and individual components

Previously the hook required components to "express" their specific focus rules through the hook call in the form of focusableIndex, shouldFocus. These often happened inside React.Children.map e.g. in MenuList, where props are created and injected into the children. I think this is mainly due to the constraint of scanning children, but with explicit registration this is no longer an issue.

Now the hook is more generic, component specific things like Tabs selection state and MenuList's "autofocus" rules stay within the component and no longer have to be "translated" through the hook.

Fixes #33268
Fixes #34218

⚠️ The bundle size report is inaccurate

I measured the @mui/utils/useRovingTabIndex after esbuilding it and and it should be 2653 gzip (+1506) (an increase, but there's another hook in there now). The mui-bot bundle size report shows -11.31% gzip for the utils package somehow because the export surface changed probably.
By comparison, Base UI's composites are about 7069 gzip.

I have followed (at least) the PR section of the contributing guide.

mui-bot · 2026-03-28T23:53:41Z

Netlify deploy preview

Bundle size report

Bundle	Parsed size	Gzip size
@mui/material	🔺+3.98KB^(+0.79%)	🔺+1.4KB^(+0.97%)
@mui/lab	0B^(0.00%)	0B^(0.00%)
@mui/system	0B^(0.00%)	0B^(0.00%)
@mui/utils	▼-1.62KB^(-10.01%)	▼-621B^(-9.91%)

Details of bundle changes

Generated by 🚫 dangerJS against be593f7

siriwatknp · 2026-03-30T05:33:55Z

Tested on mui/mui-x#21858, this PR fixes the bug.

siriwatknp

Review comments on the registration-based roving tab index refactor.

packages/mui-material/src/Tab/Tab.js

siriwatknp · 2026-03-30T10:13:59Z

packages/mui-material/src/Menu/Menu.js


  return (
    <MenuRoot
+      // `autoFocus` here means Menu should move focus itself, usually into MenuList or its active item.


The inverse logic (disableAutoFocus={autoFocus}) is correct but reads counter-intuitively. A named binding like:

// Menu manages its own focus; prevent Popover/Modal from racing with that. const disablePopoverAutoFocus = autoFocus;

would make the intent immediately obvious.

Oh I found that Claude code really hates disableAutoFocus={autoFocus} 😆

I made the code comment more detailed instead, making another var here is confusing in other ways

packages/mui-material/src/MenuItem/MenuItem.js

packages/mui-material/src/Tabs/Tabs.js

siriwatknp · 2026-03-30T10:20:29Z

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts

+
+  // `mapTick` is only an invalidation signal. The source of truth stays in the stable item map.
+  const orderedItems = React.useMemo(() => {
+    void mapTick;


Nit: the void mapTick pattern to force useMemo invalidation is clever but non-obvious. A one-liner like // mapTick is only read to invalidate the memo; the source of truth is itemMapRef would help.

Updated the comment (also added a link to the original detailed explanation)

siriwatknp · 2026-03-30T10:21:43Z

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts

+        queueMicrotask(() => {
+          if (itemRef.current === null) {
+            unregisterItem(item.id);
+          }
+        });


The queueMicrotask deferral is subtle — it avoids nested state updates during commit, but relies on the guard itemRef.current === null to handle the remove-then-re-add race. A brief inline comment explaining why a microtask (vs setTimeout or layout effect cleanup) would help future readers.

Also worth confirming queueMicrotask is available in the project's browser support matrix (it's not polyfilled in older browsers).

There shouldn't be a browser support issue; TBH I don't fully understand the Claude code comment, but queueMicrotask is a always the go-to when waiting for refs to settle

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts

packages/mui-material/src/Tab/Tab.js

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts

silviuaavram · 2026-03-31T08:02:49Z

Maybe we need more jsdoc on the hooks, functions, because it's not obvious what's going on.

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts

siriwatknp · 2026-04-01T10:45:57Z

packages/mui-material/src/StepButton/StepButton.js

+  });
+
+  return (
+    <StepButtonRoot {...rovingItemProps} {...other}>


Suggested change

<StepButtonRoot {...rovingItemProps} {...other}>

<StepButtonRoot disabled={disabled} {...rovingItemProps} {...other}>

stepButtonProps includes disabled (line 101). It's spread onto
RovingStepButton (line 114). Then inside RovingStepButton, disabled is
destructured out (line 61) and only passed to the hook — not back to
StepButtonRoot.

I think a test should be added to cover this.

Fixed in beddbdb

docs/data/material/components/selects/selects.md

mnajdova · 2026-04-01T10:45:02Z

packages/mui-material/src/Menu/Menu.js

      <ListSlot
        actions={menuListActionsRef}
-        autoFocus={autoFocus && (activeItemIndex === -1 || disableAutoFocusItem)}
+        autoFocus={autoFocus && open}


Can you explain why this was changed? It's hard to follow.

I renamed the variables a bit to improve readability, here it becomes autoFocus={shouldManageInitialFocus}

The conditions for shouldManageInitialFocus changed because the logic of activeItemIndex === -1 || disableAutoFocusItem (whether to focus an item or the container) is centralized in MenuList, since it is the roving tab index parent.

Now we're practically just passing through autoFocus={autoFocus}, && open is to prevent a keepMounted: true menu from accidentally stealing focus via autoFocus

packages/mui-material/src/Menu/Menu.test.js

packages/mui-material/src/MenuItem/MenuItem.test.js

packages/mui-material/src/MenuList/MenuList.test.js

siriwatknp

👍

Janpot · 2026-04-01T15:31:45Z

I measured the @mui/utils/useRovingTabIndex after esbuilding it and and it should be 2653 gzip (+1506) (an increase, but there's another hook in there now). The mui-bot bundle size report shows -11.31% gzip for the utils package somehow because the export surface changed probably.

Ok, looking at the report

@mui/utils/useRovingTabIndex went up with 3.44KB, seems to be expected
@mui/utils went down 1.62KB, seems to be expected as per https://github.com/mui/material-ui/pull/48122/changes#diff-8ce58b04bee89ac4cd393cac0918a8b2938e66415a79831e5ed4d3da3ce282f1L54 you removed the re-exports from './useRovingTabIndex'.

mnajdova

Good work!

silviuaavram · 2026-04-02T09:11:03Z

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts

+  // Focuses the item that already owns `tabIndex=0`. Consumers use this when they have
+  // already decided which item should be active and just need DOM focus to enter that item,
+  // such as `MenuList` when a menu opens.
+  const focusActiveItem = React.useCallback(() => {


this is not used anywhere yet

Only MenuList needed this and it became focusInitialItem there

silviuaavram · 2026-04-02T09:13:05Z

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts

+    });
+  }, [item, registerItem]);
+
+  useEnhancedEffect(() => {


why don't we do the cleanup in the effect above?

Added a comment, the upper effect for registration will re-run on any item (meta)data change, the lower effect for de-registration reruns only on id change, separating it prevents unregister/re-register on any metadata change

mj12albert added the type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. label Mar 28, 2026

mj12albert changed the title ~~Feat/roving set registry~~ [utils] roving item registration Mar 28, 2026

mj12albert requested review from silviuaavram and siriwatknp March 29, 2026 15:24

siriwatknp reviewed Mar 30, 2026

View reviewed changes

packages/mui-material/src/MenuItem/MenuItem.js Outdated Show resolved Hide resolved

siriwatknp reviewed Mar 30, 2026

View reviewed changes

packages/mui-material/src/Tabs/Tabs.js Outdated Show resolved Hide resolved

siriwatknp reviewed Mar 30, 2026

View reviewed changes

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts Show resolved Hide resolved

silviuaavram reviewed Mar 31, 2026

View reviewed changes

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts Outdated Show resolved Hide resolved

silviuaavram reviewed Mar 31, 2026

View reviewed changes

packages/mui-material/src/Tab/Tab.js Outdated Show resolved Hide resolved

silviuaavram reviewed Mar 31, 2026

View reviewed changes

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts Outdated Show resolved Hide resolved

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts Outdated Show resolved Hide resolved

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts Outdated Show resolved Hide resolved

silviuaavram reviewed Mar 31, 2026

View reviewed changes

packages/mui-utils/src/useRovingTabIndex/useRovingTabIndex.ts Outdated Show resolved Hide resolved

mj12albert force-pushed the feat/roving-set-registry branch from 58fe71f to 0399439 Compare March 31, 2026 08:27

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Mar 31, 2026

mj12albert force-pushed the feat/roving-set-registry branch from 11daa9f to 24fcc41 Compare March 31, 2026 14:02

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Mar 31, 2026

mj12albert force-pushed the feat/roving-set-registry branch 4 times, most recently from 7f20e54 to 3071ccc Compare April 1, 2026 07:12

docs cleanup

de95d6c

mj12albert marked this pull request as ready for review April 1, 2026 09:46

mj12albert added 2 commits April 1, 2026 18:10

fix review comments

15ee4dd

align null checks

4ee41f4

mj12albert requested review from mnajdova, silviuaavram and siriwatknp April 1, 2026 10:30

siriwatknp reviewed Apr 1, 2026

View reviewed changes

mnajdova reviewed Apr 1, 2026

View reviewed changes

improve readability

53d6ab0

mj12albert force-pushed the feat/roving-set-registry branch from c83b7ec to 53d6ab0 Compare April 1, 2026 12:49

fix disabled step button

beddbdb

siriwatknp approved these changes Apr 1, 2026

View reviewed changes

mj12albert added 2 commits April 1, 2026 21:33

update doc

e6596db

comment

90bd96e

mj12albert requested a review from mnajdova April 1, 2026 15:01

Merge branch 'master' into feat/roving-set-registry

d811b93

mj12albert added 3 commits April 2, 2026 00:01

fix visual highlight and add tests

d49aaff

fix open interaction handling

92fe098

add select disablePortal test

ec0ee8c

mnajdova approved these changes Apr 2, 2026

View reviewed changes

mj12albert mentioned this pull request Apr 2, 2026

MenuItems as Link break keyboard input #33268

Closed

2 tasks

silviuaavram approved these changes Apr 2, 2026

View reviewed changes

mj12albert added 4 commits April 2, 2026 17:45

cleanup focusActiveItem

bbcab89

cleanup

4623923

comment

634e655

Merge branch 'master' into feat/roving-set-registry

be593f7

mj12albert enabled auto-merge (squash) April 2, 2026 10:33

mj12albert merged commit a2ac691 into mui:master Apr 2, 2026
23 checks passed

	<StepButtonRoot {...rovingItemProps} {...other}>
	<StepButtonRoot disabled={disabled} {...rovingItemProps} {...other}>

Uh oh!

Conversation

mj12albert commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Roving items explicitly opt in

2. Boundary between useRovingTabIndex and individual components

Uh oh!

mui-bot commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Netlify deploy preview

Bundle size report

Uh oh!

siriwatknp commented Mar 30, 2026

Uh oh!

siriwatknp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

silviuaavram commented Mar 31, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

siriwatknp left a comment

Choose a reason for hiding this comment

Uh oh!

Janpot commented Apr 1, 2026

Uh oh!

mnajdova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mj12albert commented Mar 28, 2026 •

edited

Loading

mui-bot commented Mar 28, 2026 •

edited

Loading