-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: expose closeOnSelect on React Aria Menu #8315
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
Conversation
|
Thanks for the PR! It looks like in React Spectrum, we exposed it on the MenuTrigger https://react-spectrum.adobe.com/react-spectrum/Menu.html#menu I think we'll want to discuss aggregate places to put this behaviour so it isn't required on every MenuItem and you aren't as likely to end up with differences within the same menu accidentally. Maybe on Menu and MenuSection instead would be better. Will bring up with the team. |
|
In all the cases in our codebase the prop wouldn't really make sense in an aggregate place. We could work around that with that probably but would be nice if it were just exposed on the option |
|
We had a chance to talk about this today. We want to support this in |
LFDanLu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just one small comment, but otherwise looks good to me
| /** | ||
| * | ||
| * Whether the menu should close when the menu item is selected. | ||
| * @default true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO I feel like this jsdoc default and the same ones in Menu is a bit misleading. Keeping this option as undefined vs explicitly providing true will result in different behaviors, especially if we are in the multi select case
Closes #8208
I noticed the hooks supports this but the component doesn't expose the type. It currently works cause of how the props are passed through.
✅ Pull Request Checklist:
📝 Test Instructions:
No tests needed
🧢 Your Project: