Skip to content

Conversation

@methyl
Copy link

@methyl methyl commented Oct 28, 2022

Being able to control Menu visibility from the outside, using a prop and callbacks, opens up a lot of possibilities.

For example, one can use the Menu without a Button trigger as a context menu, while keeping all the great accessibility features @headlessui/menu provides.

It fixes #649, #1937 and similar PR for Popover will fix #828

I know you normally don't accept PRs from the outside, but since I'll be keeping the fork up to date for our internal usage, I thought it's worth considering.

Vue implementation is missing as I'm not fluent in Vue, but I think the implementation should be just as straightforward as for React.

So that it can be used as a controlled component, for example
when it's used as a context menu without any trigger button
@vercel
Copy link

vercel bot commented Oct 28, 2022

@methyl is attempting to deploy a commit to the Tailwind Labs Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 28, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
headlessui-react ✅ Ready (Inspect) Visit Preview Oct 28, 2022 at 0:03AM (UTC)

@adamwathan
Copy link
Member

Hey thanks for the PR! This is something we do want to support and have run into it on our own projects, but I'm not totally convinced yet that this is the right API so I’m going to close this as-is.

Unfortunately can’t make it a priority to think through this particular feature or decide on any APIs right now so I’m not able to provide any specific feedback or suggestions on what to do differently to increase the chances of it being merged.

Thanks regardless, I appreciate the contribution 👍🏻 Hopefully this is something we can dedicate some time to in the future — in the mean time if you really, really need this you always have the option to fork until we've implemented an API for this that we're happy with.

@adamwathan adamwathan closed this Nov 1, 2022
@methyl
Copy link
Author

methyl commented Nov 4, 2022

@adamwathan awesome, that works. Fork is good enough for now, but keep us posted!

@smaccoun
Copy link

Thannks for this @methyl. This is a really useful feature and I hope it gets merged in. I know @adamwathan you have probably a lot of priority things, but I wonder if you could reconsider just merging this as the API to me seems at least as straightforward as I'd imagine it. Seems at least good enough to iterate on. Just my 2 cents

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants