Drafts: Composable DropdownMenu v2 #1735
Conversation
🦋 Changeset detectedLatest commit: d0da4db The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
siddharthkp
changed the title
size-limit report 📦
|
|
|
||
| <br /> | ||
|
|
||
| <State default={1}> |
There was a problem hiding this comment.
This example appears at the top of docs, but with more recent updates we've been moving them under examples heading and keeping import statements at the top. Can we do the same here for consistency?
There was a problem hiding this comment.
I want to vote for keeping it 🤔
I've done this for ActionList and ActionMenu as well to quickly see what this component is. Meanwhile, the examples slowly build up starting from an easy example to more complex ones.
There was a problem hiding this comment.
Looks quite nice actually, almost like a preview / hero.
👍 .. we'll need to comm this to others so we can consider doing same elsewhere.
|
|
||
| ### With External Anchor | ||
|
|
||
| To create an anchor outside of the menu, you need to switch to controlled mode for the menu and pass it as `anchorRef` to `DropdownMenu`: |
There was a problem hiding this comment.
Could we elaborate on this? And should it be uncontrolled mode instead? Or maybe neither as it appears to run in controlled mode by default? and we're not really mutating ref state either?
There was a problem hiding this comment.
And should it be uncontrolled mode instead?
When you pull the open/close out, that's controlled mode. By default, you don't have to control that. Changed the text a little to make that clear.
| render(<Example />) | ||
| ``` | ||
|
|
||
| ### With a custom anchor |
There was a problem hiding this comment.
Could anchor in this title be confusing for readers as it's used in a different context further up?
| ### With a custom anchor | |
| ### With a custom trigger |
There was a problem hiding this comment.
We call this anchor because the menu still anchors on DropdownMenu.Anchor.
|
|
||
| <Note variant="warning"> | ||
|
|
||
| Use `DropdownMenu` to select an option from a small list. If you’re looking for filters or multiple selection, use [SelectPanel](/SelectPanel) instead. |
There was a problem hiding this comment.
This feels quite important, shall we hoist it near the top of page so it has better visibility? We've done this for other components before.
There was a problem hiding this comment.
This came up before with ActionMenu. What we decided was that interface guidelines is the first place where you learn which component to use, and this is the second note near props just in case you missed that.
| } | ||
|
|
||
| Menu.displayName = 'ActionMenu' | ||
| export const DropdownMenu = Object.assign(Menu, {Button: MenuButton, Anchor, Overlay, Divider}) |
There was a problem hiding this comment.
This entire file is awesome 😍 ✨
Co-authored-by: Rez <rezrah@github.com>
Part of https://github.com/github/primer/issues/337
Docs | Storybook
Next steps: https://github.com/github/primer/issues/557