[experiment] Menu RFC playground - #48823
Draft
LukasTy wants to merge 58 commits into
Draft
Conversation
Brings in mj12albert's MenuPreview proof of concept from mui#48663, rebased context onto current master: @base-ui/react aligned to ^1.6.0 (catalog + direct dependency of @mui/material), lockfile regenerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Non-public experiments page accompanying the Menu successor RFC: kitchen-sink demo with three-level submenus, groups, checkbox/radio items, link items, and an onOpenChange reason log; knobs for root behavior, submenu hover/delays, positioning, and the RFC open questions (elevation, default animation candidate, dense, dividers, RTL); classic-vs-successor parity section; classic-style controlled anchor usage; typeahead + scrolling demo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
github-actions
Bot
added
the
PR: out-of-date
LukasTy
added
type: new feature
github-actions
Bot
removed
the
PR: out-of-date
Lockfile regenerated after dependency bumps (pnpm 11.13.1, Vite 8). MenuItem.js composes cleanly: upstream aria-checked for menuitemcheckbox/menuitemradio (mui#48651) + the PoC style extraction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LukasTy
force-pushed
the
menu-rfc-playground
branch
from
July 22, 2026 14:13
b8d5972 to
905a982
Compare
Per the agreed standard, docs tooling is a fixed constraint: drop the projectSettings/getComponentImports carve-out, the apiDocs.d.ts entry point, the generated API reference pages and translations, and restore the menus.md frontmatter. The experiment is exercised via the non-public /experiments pages instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Aligns the experiment with the agreed naming lifecycle: - public subpaths and directories use Unstable_Menu2* (one component per subpath); the root index no longer re-exports parts or Base UI-style short aliases - internal identifiers and file names use clean Menu2* naming, following the Unstable_TrapFocus/Grid2 precedent enforced by the naming-convention lint rules - theme keys and classes use MuiMenu2* (the name-matches-component lint rule derives keys from component names; matches Grid2, and keys survive the later Unstable_ -> stable rename) - data attributes use data-mui-menu2-* Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Explores replacing the bespoke theming/slots/classes hygiene tests with the standard conformance harness. Both parts pass the full suite (ref forwarding, className merge, props spread, root class, component prop, slot swaps with class assertions, theme defaultProps/styleOverrides/ variants) with two Base UI-specific adaptations: - portalled roots sit behind Base UI focus-guard spans, so the render wrapper hands the harness a container satisfying its firstChild contract (candidate for a getRootElement option in the shared harness) - componentProp is skipped on the popup: the host element swaps via slots.popup instead Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Applies the agreed prop-surface standard (extend Base UI types, Omit what does not fit, declare only Material additions or changed defaults): - Menu2 and Menu2SubmenuRoot inherit the full Base UI Root/SubmenuRoot surface (gains openOnHover/delay/closeDelay/actionsRef and future props); children is curated to a plain ReactNode - the popup's flattened positioner/portal surface is inherited via Pick; only side/align (changed defaults) and the Material elevation convenience prop stay locally declared - the playground exercises the inherited surface: root openOnHover knob and the top-level elevation prop Empirical finding recorded for the RFC: the proptypes generator does not expand members declared in node_modules, so runtime PropTypes on the renderless roots degrade to `children` (generator-stable; types carry the full contract). Teaching the generator to expand external heritage is the shared-infra follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The shared classic paper styles carry maxHeight: calc(100% - 96px), which in the classic Menu resolves against the full-viewport Modal. Inside the content-sized Base UI popup the percentage resolves against the popup itself (browser-dependent), clamping the Paper ~96px shorter than its content and clipping the end of the menu (trailing items and the separator), with a visible reflow on open. Override it for the successor with the positioner-provided collision-aware space: min(calc(100vh - 96px), var(--available-height)) plus overflowY: auto so long menus scroll inside the surface. Classic Menu keeps its original styles untouched. Regression test asserts the paper's computed max-height is no longer the classic percentage form and that the surface scrolls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Opening a submenu mounts Base UI inline focus-guard and portal-anchor nodes as siblings of the submenu trigger, which breaks the legacy `[item] + .MuiDivider-root` adjacency rule carried by the shared item styles: the following separator lost its 8px margins and the menu visibly contracted below the trigger. Own the spacing on Menu2Separator's root instead (same values, so the closed state is unchanged where the adjacency rule also applies). Side effect: a separator following a Group now gets the same margins, making separator spacing uniform. Regression test opens a submenu (defaultOpen) and asserts the separator keeps its margins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Restore docs/data/material/pagesApi.js to upstream: it still registered the 16 removed /material-ui/api/menu-preview* routes (kebab-case, so the earlier cleanup sweep missed it), breaking the docs build on Netlify. - Narrow the JSDoc-typed classic paper styles before spreading (CSSInterpolation union fails TS2698 under tsconfig.build declaration emit, which broke the package build, publish dry run, and bundle monitor; the dev tsconfig tolerated it). - Drop the dead api/menu-preview* links from the generated component JSDoc headers. - Wire the playground hover-open knob to the trigger: Base UI puts openOnHover/delay on Menu.Trigger, not Menu.Root (caught by the docs typecheck once the barrel types were exercised). Verified: @mui/material build with dependency graph (declaration emit) exit 0, docs + package tsgo clean, proptypes regeneration churn-free, Menu2 suite 117 passed / 15 skipped, eslint/prettier clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The docs tooling cleanup removed the generated API JSONs and translations but left the sibling page components in docs/pages/material-ui/api/, whose imports of the deleted JSONs broke the Netlify docs build (webpack module-not-found). Verified with the full Netlify-parity build locally: pnpm docs:build (llms + next build + export + build-sw + link-check) exits 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Unstable_ subpaths use default exports, so consumers naturally drop the prefix at the import binding. The experiment pages and the type spec now import Menu2* names directly (paths keep the Unstable_ subpaths) and the JSX reads like the future stable API; the local alias blocks are gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A detached (virtual-anchor) menu has no trigger to return focus to on
close, and right-clicking text blurs the previously focused element to
body, so Base UI's focus manager fell back to its internal previously-
focused-element record -- typically the trigger of whichever menu was
used earlier on the page. Closing the context menu with Escape then
focused an unrelated demo's trigger.
Follow the APG context-menu pattern instead: the invoked surface gets
tabIndex={-1} and is passed as the popup's finalFocus.
The regression test seeds the stale record with another menu, opens
the detached menu via contextmenu, and asserts Escape restores focus
to the invoked surface (fails without finalFocus).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Versions the living RFC draft next to the component so revisions land atomically with the discoveries that motivate them and reviewers can comment line-by-line. Formatted per repo markdown conventions (vale clean); the file is removed when the RFC is posted publicly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
the
PR: out-of-date
Animation (decided): the popup surface now carries a CSS transition matching the classic Grow -- scale(0.75, 0.5625), the theme's entering/leaving durations, and the transform running at two thirds of the opacity duration -- so migrating apps keep the animation the classic Menu had. It opts out under prefers-reduced-motion and is overridable through slotProps.popup, styleOverrides or the theme. It has to sit on the popup element: Base UI waits for animations on the popup itself before unmounting (useAnimationsFinished calls element.getAnimations() with no subtree), so a transition on a descendant would animate in and be clipped on exit. Two consequences handled here: geometry read right after opening is mid-animation, so the benchmark now waits for the transition to settle, and Base UI suppresses the transition for the frame it applies the starting style, so the assertion waits too. Backdrop (decided): slots.backdrop and slotProps.backdrop now exist on the popup, mirroring the classic Menu, with a default element that is transparent and click-through like the classic invisible backdrop -- dismissal stays with Base UI's outside-press listener. It renders only when the consumer opts in: rendering it unconditionally would hand non-modal menus a full-screen layer they never had, and modal menus already get Base UI's own inert backdrop. The playground gains a backdrop knob and its animation knob now demonstrates overriding the new default away. Verified: 378 passed / 124 skipped, package typecheck and proptypes clean, and the emitted CSS asserted directly since the test runner emulates reduced motion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Decisions:
- elevation: keep the top-level convenience prop (default 8, forwards
to the Paper slot)
- actionsRef: inherit Base UI's as-is -- no Material-flavoured rename
and no hand-rolled action ref, since renaming creates permanent
wrapper drift and hand-rolling duplicates machinery Base UI owns.
The classic action ref's updatePosition has no counterpart because
the positioner tracks the anchor automatically
- submenus keep opening on hover by default
- context menu: reframed as an open question about whether a dedicated
component should own the focus-restore quirk internally, rather than
shipping a recipe whose finalFocus requirement is invisible until a
second menu exists on the page
Correction: the previous draft asked whether non-modal should be the
default. There was never a question there -- Base UI's modal defaults
to true, the classic Menu is always modal, and the benchmark measured
scroll locking as parity. The successor merely adds the ability to opt
out with modal={false}.
Repair: the previous commit's edits matched the numbered requirements
list instead of the open questions, overwriting requirements 1, 2, 4
and 7 (WAI-ARIA behavior, pointer UX, pixel parity, adjacent
capabilities). Rebuilt from the last good revision and re-applied every
open-question edit scoped to that section.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cuts the draft from ~6,400 to ~4,800 words without dropping decisions, facts or links. Most of the reduction is redundancy rather than content: the "here is how the experiment did it" tails inside the standards bullets, the four-paragraph selectedMenu discussion, the per-blocker prose in Option A, and the paragraph-length justifications on questions that are already decided. Bare URLs became named links. Wording is plainer throughout: "the rule" instead of "meta-principle", "keep, they are accessibility fixes" instead of "ratify them", "our API drifts from Base UI's" instead of "wrapper drift", shorter sentences and fewer em-dash chains. Three appendix rows were stale and are corrected: the backdrop is surfaced (not "not surfaced"), hideBackdrop maps to the backdrop slot, and the default animation ships rather than being a proposal. An audit pass over the rewrite caught two mistakes of mine, both fixed: the differences count said four while five were listed, and the mui#48651 link had been re-pointed at a claim that PR does not make. It also flagged seven places where trimming had removed something a reader needs -- that dropping selectedMenu narrows the pointer-focus difference without closing it, the mid-animation measurement caveat, the scoping caveat on presentational props, the measured bundle number behind mui#20591, what finalFocus points at, the parity criterion for testing, and that onOpenChange exposes the native event -- all restored in short form. Verified: pnpm docs:build exits 0 with the page generated, vale clean, and the structure checked (20 headings, 10 requirements, 11 questions, 6 balanced details blocks). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Select coupling is out of scope: the next major may rewrite it anyway, so the RFC should not carry that decision. Open questions 9 and 11 said "what do we still verify" and "to confirm during implementation" without saying what. Both are now specific: - Accessibility: enhanceHighContrast matches classic items through menuItemClasses, so menu2ItemClasses items (state class `highlighted`, no `focusVisible`) match nothing today, while the trigger is covered as a styled Button. The highlight is a background tint with the native outline removed, and axe only runs in the non-interactive visual regression suite, which skips the menus page. - SSR: the directives are present and placed like the classic Menu, but nothing exercises an RSC boundary, since the docs site is Pages Router with output: 'export'. Only the trigger renders on the server, and defaultOpen/keepMounted do not change that. The open part is ref typing, which is inherited wide from Base UI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
siriwatknp
reviewed
Jul 30, 2026
|
|
||
| - Stay as close to today's `Menu` as the foundation allows. | ||
| - Wiring parts (Portal, Positioner, Popup, Paper, List) collapse into one flat container, configured through `slots`/`slotProps`. | ||
| - Parts users customize per instance stay separate components: items, submenu triggers, checkbox and radio items. |
Member
There was a problem hiding this comment.
On checkbox/radio composition. Current shape needs 2 parts per item:
<Menu2RadioItem value="pdf">
<Menu2RadioItemIndicator keepMounted />
PDF document
</Menu2RadioItem>Two easy mistakes here. Missing indicator, checked state has no visual but the item still works. Missing keepMounted, label shifts when unchecked.
Have you considered building the indicator into the item? Default usage becomes:
<Menu2RadioItem value="pdf">PDF document</Menu2RadioItem>Customization goes through slotProps={{ indicator: ... }}.
Also, could the built-in indicator reuse Material UI Radio/Checkbox internally? Theming MuiRadio/MuiCheckbox would then affect the menu items too, one source of truth.
The rule above says parts users customize per instance stay separate components. I think the indicator is not one of those, most users want the default look.
Member
Author
There was a problem hiding this comment.
keepMounted: great observation. All 14 indicator call sites in the experiment pass it, and an unchecked indicator is absent from the DOM entirely, which is not obvious. Folding it in, with slots.indicator for replacement.
Reusing Radio/Checkbox internally seems a bit problematic. Checkbox renders SwitchBase, a ButtonBase wrapping a real <input type="checkbox">, so that is a focusable control inside a menuitemcheckbox: the pattern open question 1 rules out (mui/base-ui#2622), and axe's nested-interactive.
One more reason to fold it in: the indicator has no selected class, so enhanceHighContrast had to drive the checkmark from the item side.
siriwatknp
reviewed
Jul 30, 2026
| Agreed rules, instead of one global flat-vs-compound choice: | ||
|
|
||
| - Stay as close to today's `Menu` as the foundation allows. | ||
| - Wiring parts (Portal, Positioner, Popup, Paper, List) collapse into one flat container, configured through `slots`/`slotProps`. |
Member
There was a problem hiding this comment.
Proposal on the container split. Menu2 renders nothing, Menu2Popup carries all positioning and appearance. The structure Menu2 -> Menu2Popup is fixed anyway, one popup per root, always direct child. So the split only adds one wrapper level to every usage.
What do you think about collapsing them into one, with the trigger as a prop:
<Menu2
open={open}
onOpenChange={handleOpenChange}
anchor={anchorEl}
sideOffset={4}
trigger={<Button>Options</Button>} // or <IconButton>
slotProps={{ trigger: { ... } }}
>
<Menu2Item>Cut</Menu2Item>
</Menu2>This reads much closer to the existing Menu. The classic controlled anchorEl pattern stays one component, same as today. The trigger element gets the aria wiring and ArrowDown-to-open injected, same as Menu2Trigger does now. Omit trigger and pass anchor for the controlled pattern.
Submenu parts can stay compound, they are the real per-instance composition.
Member
Author
There was a problem hiding this comment.
Agreed, and it matches the rule at the top of that section: indistinguishable from any other Material UI component. The split exists only because Base UI has two roots.
One change: apply it at both levels. The submenu has the same anatomy, SubmenuRoot does not render an element either and our submenu popup already goes through the same shared module, so leaving submenus compound is indirection for no reason.
<Menu2 trigger={<Button>Options</Button>}>
<Menu2Item>Cut</Menu2Item>
<Menu2Submenu trigger={<Menu2Item>View</Menu2Item>}>
<Menu2Item>Zoom in</Menu2Item>
</Menu2Submenu>
</Menu2>Same rule at both levels: prop-only root, trigger takes the element, children are the popup. Drops 5 public parts (Menu2Trigger, Menu2Popup, Menu2SubmenuRoot, Menu2SubmenuTrigger, Menu2SubmenuPopup) for 2.
Feasible as described:
trigger={<Button>}maps onto Base UI'srender, which already merges props into an element. Same mechanismMenu2Triggeruses internally.- Prop typing survives and stays symmetric: root merges Root+Positioner+Popup, submenu merges SubmenuRoot+Positioner+Popup. Neither provider has
className/style/renderto collide.
I'd also note that a submenu trigger is a menu item, not a button.
Explicit <Menu2Item> is symmetric but a bit verbose; the alternative is accepting a node and wrapping it, but I would lean towards being explicit.
Did I understand your idea correctly?
I would be curious to hear if @aarongarciah agrees with this approach. 😊
enhanceHighContrast matched menu items through menuItemClasses, so the successor's items matched nothing: they are MuiMenu2Item and friends, and their state class is `highlighted` (Base UI's data-highlighted covers keyboard and pointer alike) where the classic item has `focusVisible`. The trigger was already covered as a styled Button, so forced colors worked on the way in and stopped at the popup. Adds the five item-like parts and both indicators. `forced-color-adjust` inherits, so the item rules also stop the browser forcing colors on the indicator; the indicators now inherit the item's color the way ListItemIcon does, and the checkbox's knocked-out checkmark is repainted in the item's background, which it otherwise merges into. The playground gets an `enhanceHighContrast theme` toggle, since experiment pages do not go through DemoInstanceThemeProvider the way docs demos do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An adversarial review pass found these; all four are confirmed against the source rather than theoretical. The integration test was a false positive. It grepped the whole stylesheet for `forced-colors` and for the Menu2 class names, all of which are emitted without the enhancer, so it passed with the entire Menu2 block deleted. It now looks inside the forced-colors media rules for ones that name a Menu2 class, and deleting the block fails it. A highlighted disabled item lost its disabled cue. `&.disabled` came first and the highlight rule matched at equal specificity, so a disabled item rendered as HighlightText on Highlight at full opacity, which is worse than no enhancer at all. This is unreachable on the classic item, because MenuList skips disabled items, but Base UI keeps them focusable by design, so one ArrowDown reaches it. The disabled rules now come last and the combination gets Canvas plus a border, so the cue survives and focus stays visible. The checked mark vanished on a selected checkbox item. The knocked-out checkmark was only repainted for the highlighted background, so on a selected item it stayed Canvas while the box painted SelectedItemText, and those are the same colour in both stock Windows palettes. The item now drives the checkmark for its selected state too. A selected submenu trigger kept its translucent background while open, because the base style pairs `selected` with `open` and outranked the lone `open` override. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Per review: the two-part shape had two easy mistakes. Omit the
indicator and the checked state has no visual while the item still
works; omit `keepMounted` and the label shifts when unchecked. Both are
real -- every one of our own 14 call sites passed `keepMounted`, and a
test written without it silently rendered no indicator at all, because
Base UI defaults it to false.
Items now render their own indicator with the space reserved. Usage
drops to:
<Menu2RadioItem value="pdf">PDF document</Menu2RadioItem>
Customization goes through `slots.indicator` / `slotProps.indicator`,
and `slots={{ indicator: null }}` renders none, which is what the
indicator's own conformance suite uses to mount one directly.
Not done: reusing Radio/Checkbox internally. Checkbox renders
SwitchBase, a ButtonBase wrapping a real input, which would put a
focusable control inside a menuitemcheckbox -- the pattern open
question 1 rules out, and axe's nested-interactive.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Applies the review proposal, extended so the two levels match. `Menu2`
now carries the trigger and the popup surface, and `Menu2Submenu` is the
same shape one level down, so a menu tree uses one component per menu
instead of a root plus a trigger plus a popup:
<Menu2 trigger={<Button>Options</Button>}>
<Menu2Item>Cut</Menu2Item>
<Menu2Submenu trigger="View">
<Menu2Item>Zoom in</Menu2Item>
</Menu2Submenu>
</Menu2>
The root `trigger` takes an element and Base UI's `render` merges the
behavior into it, so the caller keeps whatever component they chose;
anything else falls back to the default trigger. Omit it and drive the
menu with `open` and `anchor`, which keeps the classic controlled shape.
The submenu `trigger` takes content rather than an element, which is the
one place the two levels cannot match: a submenu trigger is already a
menu item, so passing `<Menu2Item>` nests an item inside an item and the
submenu never opens. The prototype is what surfaced that; the review
reply I drafted had it wrong. `slots.trigger` swaps the component.
The 12 conformance suites move to the collapsed form and stay green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The trigger and popup parts are rendered by Menu2 and Menu2Submenu now, so they stop being public deep imports. They move into the Unstable_Menu2 folder next to the other internal modules; only their class hooks stay exported, from Unstable_Menu2 and Unstable_Menu2Submenu, so styleOverrides and sx keep working. Their four conformance suites go with them: what they covered is reached through the collapsed components, which have their own suites. Migrates everything still on the compound shape: Menu2.test.tsx (35 roots, 6 submenus), the benchmark, the type spec, the playground, and the recipes. The benchmark passing unchanged is the useful signal here, since it measures behavior against the classic Menu rather than structure. One regression worth naming: a tooltip could wrap the trigger element directly in the compound shape. With the trigger as a prop the wrapper has to move into the trigger's root slot, which costs a forwardRef component. Both the test suite and the recipes page hit it, so it is not a one-off. Recorded in both places. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The API section still showed a compound strawman and called the shape undecided. It now shows what was built, and names the three things that only surfaced by building it: the root trigger takes an element while the submenu trigger takes content, and wrapping a trigger in a Tooltip now costs a forwardRef component. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two cleanups from review. The index files were index.js plus index.d.ts, which is the pattern for the JS-authored components. Every .tsx-authored one here (ClickAwayListener, Grid, PigmentGrid, DefaultPropsProvider) uses a single index.ts, and Menu2 is all .tsx. The split had already caused the drift it invites: the type exports lived only in the .d.ts half. Test identifiers carried the Unstable_ prefix that belongs on the import path, not the local binding: `import Unstable_Menu2Item from '@mui/material/Unstable_Menu2Item'`. Only paths, subpaths and exports carry it, per the naming rule in the RFC. The unused `Unstable_Menu2` named re-export goes too, since the default export covers it. Also fixes 8 pre-existing markdown table-alignment lint errors in the RFC. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
I only ever ran the Menu2 directories and never `pnpm proptypes` or the browser scope, so both of these reached CI. Proptypes: the hand-written entries for Menu2, Menu2Submenu and the two items were out of date. Regenerated. Typing `slots.indicator` as `ElementType | null` also made the generator emit a 180-line union of every HTML tag, so the null option is gone; the indicator suites suppress the built-in with a component that renders nothing, which needs no extra API surface and has precedent. Placement benchmark: `waitForSettled` awaited `popup.getAnimations()`, but a CSS transition is absent from that list until it actually starts, so the call could return an empty list and let a mid-transition rect through. The scaled-down surface then read as an 8px offset rather than an obvious failure, which is why it looked like a placement bug. It now also waits for the transform and opacity to settle. Worth noting for anyone reading the benchmark: this only bites because the successor animates by default. Collision handling was my first suspicion and it was wrong -- disabling it did not change the number. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review found three real defects.
The portal and positioner slots replaced the Base parts with whatever was
passed. Both are context providers -- the positioner needs the portal's
context, the popup needs the positioner's -- so `slots={{ portal: 'div' }}`
threw "Base UI: <Menu.Portal> is missing" at runtime. The type fixture
advertised both but only typechecked them, which is why nothing caught it.
The Base parts are now always rendered and a slot only changes what they
render, through `render`, which is what the popup slot already did and what
the RFC's own customization rule says. Covered by a test that renders both.
Collapsing the containers dropped the public ref: classic Menu forwards one
and Menu2 was a plain function. Menu2 and Menu2Submenu now forward to the
popup surface, matching where the classic ref lands.
`slots.backdrop` was advertised in the RFC but missing from Menu2PopupSlots,
since the public slots type redeclared the list without it.
RFC corrections: the stale "Left" items, the +77 B figure (the current
report is +685 B gzip, +3.54 KB parsed), forced colors now being covered,
`disableScrollLock` described as removed rather than mapped to `modal`, and
the animation no longer claimed to match Grow -- classic derives its
duration from menu height via transitionDuration="auto", a CSS transition
cannot.
The pointer-focus justification was wrong: the APG says focus moves to an
item on open and carves out no pointer exception. The behavior stays, now
documented as a deliberate deviation.
Adds open questions 12-14 for the calls that are not mine to make: the theme
API still being compound while the JSX is flat, the MenuItem/Select
lifecycle, and how much of Base UI's API is part of the Material contract.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The JSX collapsed but the theme did not, so the parts were only half internal: styleOverrides and variants still had to go through MuiMenu2Popup, MuiMenu2Trigger, MuiMenu2SubmenuPopup, MuiMenu2SubmenuTrigger and MuiMenu2SubmenuRoot, which are public contracts. Those five keys are gone. MuiMenu2 and MuiMenu2Submenu now carry defaultProps, variants and styleOverrides for their slots -- root, trigger, backdrop, paper, list for the menu; root, trigger, paper, list for the submenu -- by renaming the styled parts onto those names and slots. The internal components no longer call useDefaultProps for keys that no longer exist; the collapsed component applies them once. The forced-colors enhancer moves with it: the submenu trigger block now sits on MuiMenu2Submenu's `trigger` slot. Class hooks stay per element. CSS has to select distinct nodes and their states, and the submenu trigger carries the full item state set, so folding those into one token bag would lose selectors for no gain. This is the same split Autocomplete has: one theme key, per-slot classes. Covered by a test that themes all four slots and reads the computed styles off the rendered elements, which is what would have caught the half-collapse in the first place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The RFC now follows ASD-STE100: short sentences, the active voice, simple tenses, one word for one meaning, and no idioms or metaphors. Verified against the original, and every structural invariant holds: 20 headings, 4 code fences, 6 details blocks, 41 link targets, 223 inline code spans, 69 table rows, 8 alignment rows, and 57 numbers. A review pass over each section found 9 places where the rewrite changed the meaning. All 9 are corrected. Examples: a hedge became a commitment on the next major, an open list of close reasons became a closed list, "opt-in" became "optional", and a recommendation became a requirement. Five long enumerations are now vertical lists, which is how STE handles a list. 2 of 557 sentences are still over 25 words: one is a verbatim quotation from base-ui#2143, the other is the frontmatter description. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The submenu popup now uses sideOffset -4 and alignOffset -8. The submenu overlaps its parent by 4px, and it starts 8px higher than its trigger. The 8px cancels the top padding of the list, so the first item of the submenu lines up with the trigger row. Base UI applies its own offsets only for a context-menu parent, which are -5 and 2. For a submenu parent it sets the side and the align, and no offset. The overlap in the Base UI demos comes from their styles. These values come from the Material spacing instead: 8px is the list padding, and 4px is half of one spacing unit. The playground and the recipes pinned sideOffset 8 on submenus, which hid the default. They now use the default. The root menus keep their offset. A test measures the geometry and pins both offsets. It opens the menu with defaultOpen, because a click on the trigger Button starts a ripple that outlives the test and fails the act() check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The RFC was correct but hard to scan. Long paragraphs held facts that a table or a list shows better. Changes: - The three failed attempts, the two style regressions, and the three slot helpers are now tables. - The six decided items move out of "Open questions" into a "Decisions" table. Each row keeps its caveat, for example that the animation is not identical to Grow. "Open questions" now holds only the 8 items that need an answer, and the cross-references point to the new places. - The `selectedMenu` explanation and several dense paragraphs are now lists with a bold lead-in. The share of prose falls from 32% to 18%. Tables rise from 10 tables of 1108 words to 10 tables of 1703 words. The word count falls from 6486 to 6164, which is small: the document is dense, and a larger cut would remove facts that the review asked us to keep. A review pass over each section found 5 changes of meaning, and all 5 are corrected. One example: "One of our oldest requests" became "An old request", which loses the rank. A structural check also found a lost `action` code span, and I restored that decision. Every link, code span, code block, and details block is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- PropTypes: the infra now extracts inherited props
(mui-public#1709). The RFC no longer states the limitation as
permanent. We get the capability when we sync with master.
- Ripple: decided. It comes by default, the item root becomes a styled
ButtonBase, and `disableRipple` turns it off, the same as the other
Material UI components. The item moves from the open questions to the
decisions table. A ButtonBase inside an item stays impossible.
- Removed the MenuItem and Select lifecycle question. That answer waits
for the next major plan.
- Removed the bundle size question. The path is decided.
Also corrects the Tooltip statement. It said a Tooltip around any trigger
needs a forwardRef component. That is true for a submenu trigger only,
because the submenu trigger takes content. The root trigger takes an
element, so trigger={<Tooltip><Button /></Tooltip>} works directly. A
test in the browser project confirms this.
Open questions drop from 8 to 5, and the cross-references point to the
new numbers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The items had no ripple, because their root was a plain element. The root of each item is now a styled ButtonBase, which is what the classic MenuItem already does. The items get a ripple like the other Material UI components, and `disableRipple` turns it off. This covers the five item parts: Menu2Item, Menu2LinkItem, Menu2CheckboxItem, Menu2RadioItem, and Menu2SubmenuTrigger. ButtonBase renders a <button> by default, so each root now passes its own element through `component`. The items keep the div, and the link item keeps the anchor. The `nativeButton` inference is unchanged. A test asserts the ripple and the opt-out. ButtonBase mounts the ripple lazily, on the first interaction, so the test presses the item before it reads the ripple element. Also guards the submenu overlap test with isJsdom(). It measures geometry, and jsdom has no layout. This is the node-scope failure I could not identify earlier. It was not a flake. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`trigger` is typed as a node, but only some nodes worked. The check used `React.isValidElement`, which is true for a fragment. A fragment then took the render path, and it cannot take props or a ref, so no trigger element rendered at all and React logged an error. The root now treats a fragment as content, the same as text or several nodes. A single element still becomes the trigger itself. Corrects the prop documentation and the RFC. Both said the root "takes an element", which hid this gap. They now say the prop takes a node, and they name what each node kind does. A test covers text, a fragment, and several nodes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Per review: a plain string trigger raised too many questions, and it behaved differently from an element. `MuiMenu2.styleOverrides.trigger` reached the string form but not the element form, so the two were not interchangeable. `trigger` now takes an element at both levels, and Base UI's `render` merges the trigger behavior into it. The default trigger, the `slots.trigger` escape, and the dead `trigger` theme slots are gone. `Menu2Trigger` and `Menu2SubmenuTrigger` are deleted, since nothing renders them. The submenu trigger keeps its open-state styling through the list that contains it, because the caller now supplies the element. This is the compromise we chose; the cascade order between that rule and the item's own selected rule is still unasserted. The `href` guard is gone with the default trigger. It defended a Button that we built. Base UI never had it, and the classic Menu has no trigger part at all. Two behaviors this exposed, both now explicit: - A submenu trigger must not close the menu. The caller usually passes a `Menu2Item`, which closes on click, so the submenu forces `closeOnClick` off. - We no longer infer `nativeButton`. The caller declares it when the element is not a native button. A wrapper used as a trigger must forward the trigger's props and ref to its child, the way Tooltip does. The tooltip test helper now does, and the recipes wrap the trigger element directly, so the forwardRef helper is deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Option A styles the submenu trigger's open state from the list that contains it, while the item styles its own selected state. Both selectors are (0,2,0), so the winner depends on style insertion order. A test now pins it: a selected trigger keeps its primary-colour blend when its submenu opens, and the blend gets stronger rather than falling back to the neutral focus colour. The order is correct today, and the test fails if it changes. The RFC now describes the element-only trigger: - The sample passes an element at both levels. - The three old results become four: `trigger` takes an element at both levels, a wrapper must forward props and ref, a submenu trigger must not close the menu, and we no longer infer `nativeButton`. - The Tooltip caveat is gone. It is now a worked example instead, because wrapping the trigger works at either level. - The theme row drops the `trigger` slots, which no longer exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
added
the
PR: out-of-date
…nu-support-4992a3 # Conflicts: # pnpm-lock.yaml
The submenu triggers rendered as bare inline text instead of menu items. The docs pages still passed a `React.Fragment` as the trigger, which the earlier content-based API accepted. A fragment is an element, so the type accepts it, but Base UI cannot merge the trigger behavior or a ref into it. The trigger then loses its item styling and its behavior. The 9 fragment triggers in the playground and the recipes are now `Menu2Item` elements. `Menu2` and `Menu2Submenu` now log an error in development when `trigger` is a fragment, because the type cannot catch this. A test covers the warning. A second test pins the layout: a submenu trigger must be a flex row that fills the list width. The fragment version rendered inline, so this test fails if the problem returns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
removed
the
PR: out-of-date
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
3 participants
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.
Companion experiment for the upcoming "Menu: Base UI-based successor" RFC. Draft, not intended to merge as-is.
This branch:
@base-ui/reactaligned to^1.6.0(docs catalog + direct dependency of@mui/material).@mui/material/Unstable_Menu2*(one component per subpath, no Base UI-style short aliases), internalMenu2*identifiers, theme keys and classesMuiMenu2*(the name-matches-component lint rule derives keys from component names, matching the Grid2 precedent; keys survive the laterUnstable_-> stable rename).projectSettingscarve-out, generated API pages, translations); the experiment is exercised via the non-public/experimentspages instead.describeConformanceonMenu2ItemandMenu2Popupas the replacement for the bespoke theming/slots/classes hygiene tests (62 passing conformance tests; two documented Base UI-specific adaptations)./experiments/menu2-playground(knobs and use case demos),/experiments/menu2-recipes(Tooltip, PreviewCard, ContextMenu integrations) and/experiments/menu2-rfc(the rendered RFC draft).Playground knobs
modal,loopFocus,highlightItemOnHoveropenOnHover,delay,closeDelay,closeParentOnEscside,align,sideOffset,alignOffset,keepMountedelevation, default animation candidate (CSS approximation ofGrowviadata-starting-style/data-ending-style), dense items, item dividers, RTL directionSections
onOpenChangereason logopen/onOpenChange, no Trigger partslotProps.paperThe recipes page originates from #48663 and is updated to the new imports.
RFC: the living draft is tracked in this PR at
docs/pages/experiments/menu2-rfc.mdand rendered at/experiments/menu2-rfcon the deploy preview -- review comments welcome on the source file. It moves to a public RFC issue once settled.Verified locally: full Menu2 unit suite green (115 passed / 13 skipped across jsdom + browser projects),
tsgotypecheck clean, fresh no-cache ESLint clean, and the playground exercised against the dev server (keyboard submenu navigation, Escape ordering,onOpenChangereasons,MuiMenu2*classes in the DOM).