✨ <Menu /> support static option for Items #128
Conversation
| @@ -1,5 +1,5 @@ | |||
| {{! template-lint-disable no-down-event-binding }} | |||
| {{#if @isOpen}} | |||
| {{#if (or @isOpen @static)}} | |||
There was a problem hiding this comment.
do we need @static? I really don't like over-expanding public API, and it seems just setting @isOpen={{true}} would be sufficient?
There was a problem hiding this comment.
It seems the other implementations support a static option as well. So I assume, that's why the PR has been made.
There was a problem hiding this comment.
Yes I included the addition of the static option as it is supported in the React & Vue implementations and is already available in the ListBoxOptions component
In terms of a use case we will be using multiple Menus in a left nav and we need don't require the dropdown to close when focus is lost, only if the user has clicked to close the Menu, or another Menu has been opened. Utilising the static option to override the behavior in our app seemed like the best way to achieve this.
There was a problem hiding this comment.
Regarding use case:
Somewhere in react doc, I saw information that might be useful for handling with own transition and animation.
There was a problem hiding this comment.
You're right it's here thank you!
There was a problem hiding this comment.
What's the advantage over using isOpen though? I'm confused. Are the semantics really that much better even though the arg does the exact same thing?
Is there a difference in the other implementation?
🤔
There was a problem hiding this comment.
hmm I think that <Menu.Items/> does not expect to receive isOpen prop. It is in template because is passed down from <Menu/> component, but intention was to not override this prop.
Here
By default, your Menu.Items instance will be shown/hidden automatically based on the internal open state tracked within the Menu component itself.
Probably it is possible to do it without static arg, but then the question is how far we want to be from original API in react and vue
There was a problem hiding this comment.
ah! so @isOpen is actually private api.
I think perhaps I need to document the public/private ness of all the args 😅
|
Hello!!! thank you for your contribution ✨ ❤️ |
aoifehannigan
force-pushed
the
dropdown-support-static-option
branch
from
82a5243 to
6bc98a4
Compare
|
I guess the api to menu is already pretty small. If we can keep that in check, and diverge from the other implementations if we need to, I'm ok with this change. I mostly want to protect our users if react or vue libraries go off the rails. |
|
Thanks for doing this!!! <3 |
|
Released in 0.12 |
|
Thank you @NullVoxPopuli and @far-fetched! |
This PR add support for the
staticoption in the MenuItems component which is used to determine whether the element should ignore the internally managed open/closed state.Prop:
staticType:
BooleanDefault:
false