Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Menu][joy] Remove open and onClose props #38354

Closed
wants to merge 8 commits into from

Conversation

michaldudak
Copy link
Member

@michaldudak michaldudak commented Aug 7, 2023

The open and onClose props are no longer exposed by the Menu component. It is now the responsibility of the Dropdown to control the open/close state. These props were inadvertently omitted when implementing the Dropdown.

This is a part of the attempt to solve a problem described in #38324.

@michaldudak michaldudak requested a review from siriwatknp August 7, 2023 08:23
@michaldudak michaldudak added component: menu This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy labels Aug 7, 2023
@mui-bot
Copy link

mui-bot commented Aug 7, 2023

Netlify deploy preview

https://deploy-preview-38354--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against fd1d932

@michaldudak michaldudak force-pushed the joy-controlled-menu branch from 1a3c817 to 2f8b9c5 Compare August 7, 2023 12:55
@siriwatknp
Copy link
Member

@michaldudak Is this mean that Menu cannot be used without the Dropdown?

@michaldudak
Copy link
Member Author

Yes. If there are use cases where it would be needed, we can think about how to implement it.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 14, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 16, 2023
@michaldudak
Copy link
Member Author

@siriwatknp, if the explicit use of the Dropdown is not desired, you can use a pattern similar to what's in #38934 - a wrapper component that uses useDropdown exported as Menu.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 9, 2023
@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Feb 15, 2024

I think this may resolve #39903. I noticed it while reviewing #41090. However, there's an offset issue when the menu is opened: link. I believe it was discussed in #38324 not to rely solely on MenuButton when using Dropdown.

@michaldudak
Copy link
Member Author

@siriwatknp, do you think this PR still makes sense, or should we discard it?

@michaldudak michaldudak closed this May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: menu This is the name of the generic UI component, not the React module! package: joy-ui Specific to @mui/joy PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants