-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add reusable block icon. #23552
Add reusable block icon. #23552
Conversation
Size Change: +209 B (0%) Total Size: 1.17 MB
ℹ️ View Unchanged
|
Adding "in progress" because the icon really needs to be on the right to look balanced, and that needs more work. |
f79cbd6
to
61d4b05
Compare
I moved the icon to the right, in the spot of the shortcut, as outlined in #18667. Screenshots: This also affects other menu items: I think it adds a nice bit of consistency and balance. I'd like for more work to be done to optimize the code, there's a lot of duplication and it can align to the grid. But it's somewhat deep surgery so probably best done separately. |
Does this mean a menu item could have an icon or a shortcut, but not both? What happens when I select Code Editor in the more menu; Does the shortcut get replaced by a check mark? |
Yes and no. You can still add an icon, as shown in the tools menu, but it's a different code syntax for it, not perhaps as such something you should only do if you know what you're doing. Which goes okay with the idea that we should be using less icons overall in menus.
Serendipitiously, this is already the case in the core project, because the shortcut for the active editor view is intentionally hidden. But yes, now that's also technically the case. |
3d0be45
to
feb0354
Compare
Refreshed this one, and revisited. It now:
Props to @MichaelArestad for some mockups that inspired these changes. I have updated the screenshots in the PR description to match these changes. |
532f913
to
4826cbe
Compare
@jasmussen This looks and feels solid. I did a bit of clicking around and for the most part, everything worked very well. I see one spot in the site editor where the icon alignment is a little weird. I suspect it is because the component configuration might be custom or something specific to the editor CSS: |
Ugh we need a new icon for that :D great call, let me take a look at that one. Thank you Michael! |
576baaa
to
a6c908e
Compare
a228c26
to
6d6a4b6
Compare
Thanks @jasmussen ! We might end up removing the reusable block one now that it exists as a tab and is not present elsewhere, but the improvement in the positioning and the other icons is great. |
This adds a reusable block icon to the menu.
It also:
Props to @MichaelArestad for some mockups that inspired these changes.
Before:
After:
In no places (that I could find) do we currently show BOTH a shortcut, and an icon. But since it's theoretically possible this will be necessary (props @shaunandrews), it is accommodated. The following is a fake example (reusable blocks don't have a shortcut):