Repository navigation
Restyle web menus to match the menu design #12310
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
Open
mozzius
wants to merge
11
commits into
samuel/app-3734-more-inline-menus
from
samuel/app-3734-web-menu-design
+209
−78
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
ae38986
restyle web menus to match the menu design
mozzius 83ec24b
give web menu items android-style shapes
mozzius 7e16b09
round highlighted and selected web menu items evenly
mozzius eba4e12
round the bottom of web menu items above a label
mozzius a6b2b0d
match android's menu insets on web, shrink item text, keep hover corn…
mozzius 927542d
space web menu items 2px apart so fills don't touch
mozzius b667b82
shrink web menu icons to 18px and pull them 4px closer to the edge
mozzius a6ca020
shorten web menu items to 40px
mozzius dbeb5b4
give keyboard-focused web menu items a focus ring
mozzius 857bdbb
lighten web menus' selected colours a shade in dark themes
mozzius d006e10
keep right chevrons trailing in web menu items
mozzius File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
non-blocking: a disclosure chevron ends up in front of the label
Moving every
ItemIconto the leading side also moves directional affordances that only make sense trailing. On web, the composer's language menu (src/view/com/composer/select-language/PostLanguageSelect.tsx:126-129, which has no.webvariant) renders "More languages..." with<Menu.ItemIcon icon={ChevronRightIcon} />after the text. With this change the right-pointing chevron is drawn before the label (› More languages...), so it no longer reads as "opens more". The oldItemIconhonoured the written order.Possible fixes: keep trailing-only glyphs like
ChevronRightafter the text inarrangeItemContent, keepposition="right"meaningful on web, or change that call site.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.
Fixed in a3e1ade:
arrangeItemContentkeeps a right chevron trailing, like Android's submenu arrow, so "More languages..." reads as opening more options again.