Menu design upgrade #20898
Merged
Menu design upgrade #20898
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
valentinpalkovic
force-pushed
the
valentin/menu-design-upgrade
branch
3 times, most recently
from
8c157e1 to
58399ba
Compare
valentinpalkovic
force-pushed
the
valentin/menu-design-upgrade
branch
3 times, most recently
from
172d13f to
2099b56
Compare
valentinpalkovic
force-pushed
the
valentin/menu-design-upgrade
branch
from
2099b56 to
eb751fd
Compare
yannbf
reviewed
Feb 6, 2023
yannbf
approved these changes
Feb 6, 2023
There was a problem hiding this comment.
This looks good! There are some changes requested in Chromatic though, please check them out.
@MichaelArestad did we want to account for RTL? Because it believe it does not
@yannbf That's a good question. Does Storybook itself account for RTL (sidebar switching sides, etc)? |
1. Icons are always on the left. If there is one icon in a menu item, every other menu item will have an indent to match the space of the icon as seen in the Storybook menu. 2. If there are no icons, the item label is not indented. 3. There is a node on the right of each menu item that can be populated with keyboard shortcuts, illustrations, etc. 4. Active items (denoted with a checkmark) will be both bold and blue. 5. There will be an optional `description` text node that will be below the item label. 6. Keyboard shortcuts will list all commands in one container. 7. The decorative menu tooltip arrows are no longer shown.
valentinpalkovic
force-pushed
the
valentin/menu-design-upgrade
branch
2 times, most recently
from
e6b9c0f to
9ebb8fe
Compare
For the most part, yes: You can do |
valentinpalkovic
force-pushed
the
valentin/menu-design-upgrade
branch
from
9ebb8fe to
7820ac7
Compare
valentinpalkovic
force-pushed
the
valentin/menu-design-upgrade
branch
from
7820ac7 to
882330a
Compare
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: #20897
What I did
descriptiontext node that will be below the item label.How to test
Checklist
MIGRATION.MD
Maintainers
make sure to add the
ci:mergedorci:dailyGH label to it.["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]