[Dropdown] Introduce higher-level menu component #37667

michaldudak · 2023-06-21T21:02:16Z

This PR introduces hooks and components that aim to make building menus easier. The general shape was discussed in #32088.

One notable change from what was discussed in the linked issue is the name of the outermost component. We agreed to use MenuProvider, but this name is already used elsewhere in Base UI. I decided to call the component (and the associated hook) DropdownMenu. Similarly to MenuProvider, this won't conflict with any existing identifiers from Material UI.

The structure currently looks as follows:

<DropdownMenu>
  <MenuButton>Menu</MenuButton>
  <Menu>
    <MenuItem>Item 1</MenuItem>
    <MenuItem>Item 2</MenuItem>
    <MenuItem>Item 3</MenuItem>
  </Menu>
</DropdownMenu>

I'm looking forward to seeing opinions about this naming pattern. I'm considering renaming the Menu to MenuList to make it less ambiguous.

As for the implementation, controlling the open/close state is done via a controllable reducer owned by the DropdownMenu. Menu and MenuItem dispatch actions, and the whole logic sits in dropdownMenuReducer. The dispatch function could be exposed externally in the future if needed. Such architecture allows for easy communication between components and enables painless introduction of other menu items, such as radio button or checkbox going forward.

⚠️ Breaking change

This PR changes how menus are used. Instead of custom wiring of a menu and a button to open/close it, place the Menu and the MenuButton within a Dropdown component. The open and onOpenChange props now are accepted by the Dropdown, not the menu.

Playground: https://deploy-preview-37667--material-ui.netlify.app/experiments/base/menu/

Closes #32088

mui-bot · 2023-06-21T21:05:40Z

Netlify deploy preview

@material-ui/unstyled: parsed: +2.92% , gzip: +2.45%
@mui/joy: parsed: +1.39% , gzip: +1.52%

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against dafec79

siriwatknp · 2023-06-23T02:25:28Z

👏 Love it!

siriwatknp

For the naming, here is my thought

Dropdown sounds better and shorter than DropdownMenu without losing the meaning.
I don't agree to change Menu to MenuList because that will introduce a big change to the codebase and to developers (I think the Menu works great).

docs/pages/experiments/base/menu.tsx

michaldudak · 2023-06-23T09:54:28Z

Dropdown sounds better and shorter than DropdownMenu without losing the meaning.

Interesting idea. I'm not sure I', 100% sold, but I'll consider it. When we add a context menu in the future, I wonder what the naming scheme should be. <Context> itself is a no-go, so we'd need <ContextMenu>. But alternatively, we could say that a context menu is also a dropdown menu, just with a different trigger, so it could look like this:

<Dropdown>
  <ContextTrigger element={ref} />
  <Menu>...</Menu>
</Dropdown>

While it's something for the future, it could be worth having this at the back of our heads we create an intuitive API.

mnajdova · 2023-06-26T10:51:02Z

On the naming, I would go with Dropdown as well, however, there is slight difference in what we consider Dropdown, vs what other libraries are considering it, for e.g. fluent ui - https://developer.microsoft.com/en-us/fluentui#/controls/web/dropdown, https://www.downshift-js.com/use-combobox#materialui - it seems to be more associated with the autocomplete. If we consider this, maybe DropdownMenu is a better name.

michaldudak · 2023-06-26T19:19:45Z

But in our case, we still have a Menu component inside, so it's more clear, I guess. So essentially, the Dropdown component would implement a pretty generic concept of dropdown-anything, and we will use it to build the Menu, but in theory it could be used in a more popover-like way.

mnajdova · 2023-06-27T06:19:53Z

But in our case, we still have a Menu component inside, so it's more clear, I guess. So essentially, the Dropdown component would implement a pretty generic concept of dropdown-anything, and we will use it to build the Menu, but in theory it could be used in a more popover-like way.

Fair enough, let's go with Dropdown only then for the start. I am wondering whether we should make this unstable at start, similarly to the NumberInput, and stabilize it once we have enough feedback on it. That way, we can experiment and change the name later if it creates confusion. What do you think?

michaldudak · 2023-06-27T07:01:08Z

A problem with this approach is that we'd have to make the Menu unstable as well as it had to be adjusted to work with the Dropdown

michaldudak · 2023-06-27T17:21:08Z

@siriwatknp I'll introduce these changes to Joy as well. Are you OK with it?

siriwatknp · 2023-06-28T09:26:34Z

@siriwatknp I'll introduce these changes to Joy as well. Are you OK with it?

Sounds good to me!

siriwatknp · 2023-06-28T09:28:21Z

@siriwatknp I'll introduce these changes to Joy as well. Are you OK with it?

Sounds good to me!

Thinking about it again. Would it be better to separate the PR? easier to review and track in the changelog.

michaldudak · 2023-06-28T10:43:32Z

Merging just the changes from Base UI would leave Joy UI broken as the Base Menu's API was changed.
I can create a separate PR based on this branch to it's easier to review

DiegoAndai

This is a great utility component, I think it completely delivers on the promise of making easier the building of dropdown menus 🎉

Regarding naming, Dropdown and Menu sound good to me, at the beginning, I was also thinking MenuList would have been better but the role is menu and on the ARIA design patterns they name it Menu as well, so I think it's consistent with the ecosystem.

It's a complex problem and thus complex implementation so reviewing was hard. I got the general picture of how it works and it makes sense to me, as well as how it was implemented, so I'm approving.

I just left a little comment/question.

I think we should wait for @siriwatknp to review and approve the Joy changes

DiegoAndai · 2023-07-14T21:12:45Z

docs/data/base/components/menu/UnstyledMenuBasic/css/index.js

 import { useTheme } from '@mui/system';
-import { ListActionTypes } from '@mui/base/useList';

 export default function UnstyledMenuSimple() {


Shouldn't we replace the "Unstyled" prefixes for "Base"? Or are we keeping the "Unstyled"? I think it might be confusing. Maybe just use "MenuSimple"

Yeah, good point. I'll create a separate PR for this.

michaldudak · 2023-07-18T07:06:04Z

@siriwatknp I'd appreciate if you could double-check the changes in Joy.

siriwatknp · 2023-07-18T10:11:35Z

@siriwatknp I'd appreciate if you could double-check the changes in Joy.

Got it.

siriwatknp

👍 Looks great!

michaldudak added component: menu This is the name of the generic UI component, not the React module! package: base-ui Specific to @mui/base labels Jun 21, 2023

michaldudak added 2 commits June 22, 2023 18:44

Add DropdownMenu and MenuButton components

d030938

Improve the return type of combineHooksSlotProps

d64e35b

michaldudak force-pushed the menubutton branch from c8be9e4 to d64e35b Compare June 22, 2023 16:44

siriwatknp reviewed Jun 23, 2023

View reviewed changes

docs/pages/experiments/base/menu.tsx Outdated Show resolved Hide resolved

michaldudak added this to the Base UI: Stable release milestone Jun 23, 2023

michaldudak added 4 commits June 23, 2023 20:33

Add ARIA attributes to menu button

9e2068c

Update the demos

e6125c7

Fix TS errors

20925a1

Update the API docs

ab70b38

michaldudak added 4 commits June 27, 2023 13:41

Fix Base tests

d71aa92

Open menu by arrow keys

9a03c6b

Rename the DropdownMenu to Dropdown

564fc1c

Update the docs about focus-visible support

316e293

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 28, 2023

Merge remote-tracking branch 'upstream/master' into menubutton

743c45c

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jun 28, 2023

Update plain CSS and Tailwind demos

8a322f3

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 14, 2023

DiegoAndai approved these changes Jul 14, 2023

View reviewed changes

michaldudak force-pushed the menubutton branch 2 times, most recently from 42fe487 to cbdc759 Compare July 17, 2023 09:11

Restore the popup slot

b82ea23

michaldudak force-pushed the menubutton branch from cbdc759 to b82ea23 Compare July 17, 2023 09:22

Pass the id to useMenu in MenuList

c8e9579

Merge remote-tracking branch 'upstream/master' into menubutton

77e9f0e

update joy menu demos

742121a

siriwatknp approved these changes Jul 18, 2023

View reviewed changes

michaldudak added 5 commits July 18, 2023 22:43

Fix the experiment styles

2ca4e20

Merge remote-tracking branch 'upstream/master' into menubutton

28c31b5

Prevent a flash of opened menu on mount

552ef87

Simplify registering the trigger element

168dccf

Prevent focus stealing on mount

1f3fb97

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 20, 2023

Merge remote-tracking branch 'upstream/master' into menubutton

6be0358

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 24, 2023

michaldudak added 5 commits July 24, 2023 11:31

Add 'use client' directive

17cd436

Fix Base UI home page Menu usage

3225b3d

Fix Base UI demos

8e0e0ff

Fix unit test

5322232

Polish the docs

dafec79

michaldudak added the breaking change label Jul 24, 2023

michaldudak merged commit fbc4c66 into mui:master Jul 25, 2023
22 checks passed

michaldudak deleted the menubutton branch July 25, 2023 13:08

sai6855 mentioned this pull request Aug 1, 2023

[docs][Menu][base] Remove Unstyled prefix from demos function names #38270

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dropdown] Introduce higher-level menu component #37667

[Dropdown] Introduce higher-level menu component #37667

michaldudak commented Jun 21, 2023 •

edited

mui-bot commented Jun 21, 2023 •

edited

siriwatknp commented Jun 23, 2023

siriwatknp left a comment

michaldudak commented Jun 23, 2023 •

edited

mnajdova commented Jun 26, 2023

michaldudak commented Jun 26, 2023

mnajdova commented Jun 27, 2023

michaldudak commented Jun 27, 2023

michaldudak commented Jun 27, 2023

siriwatknp commented Jun 28, 2023

siriwatknp commented Jun 28, 2023

michaldudak commented Jun 28, 2023

DiegoAndai left a comment

DiegoAndai Jul 14, 2023

michaldudak Jul 18, 2023

michaldudak commented Jul 18, 2023

siriwatknp commented Jul 18, 2023

siriwatknp left a comment

[Dropdown] Introduce higher-level menu component #37667

[Dropdown] Introduce higher-level menu component #37667

Conversation

michaldudak commented Jun 21, 2023 • edited

⚠️ Breaking change

mui-bot commented Jun 21, 2023 • edited

Netlify deploy preview

Bundle size report

siriwatknp commented Jun 23, 2023

siriwatknp left a comment

Choose a reason for hiding this comment

michaldudak commented Jun 23, 2023 • edited

mnajdova commented Jun 26, 2023

michaldudak commented Jun 26, 2023

mnajdova commented Jun 27, 2023

michaldudak commented Jun 27, 2023

michaldudak commented Jun 27, 2023

siriwatknp commented Jun 28, 2023

siriwatknp commented Jun 28, 2023

michaldudak commented Jun 28, 2023

DiegoAndai left a comment

Choose a reason for hiding this comment

DiegoAndai Jul 14, 2023

Choose a reason for hiding this comment

michaldudak Jul 18, 2023

Choose a reason for hiding this comment

michaldudak commented Jul 18, 2023

siriwatknp commented Jul 18, 2023

siriwatknp left a comment

Choose a reason for hiding this comment

michaldudak commented Jun 21, 2023 •

edited

mui-bot commented Jun 21, 2023 •

edited

michaldudak commented Jun 23, 2023 •

edited