Image block: Normalize the block toolbar #29205
Merged
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
youknowriad
added
[Type] Enhancement
|
Size Change: +209 B (0%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
|
Riad my friend, this is so good! 👏 Before:
After:
The weight this removes from the toolbar is incredible, it's such a better experience:
And it also much better helps group sections in relevant categories. The dropdown is offset a bit to the left:
But I suspect there isn't any good clean fix for this, and that it's best addressed separately by the effort that also improves it for text buttons. Designwise, this is solid to me! Thank you! |
|
I think we could consider moving the "A" to the main group before the "replace". What do you think? The icon is not the best it could be. It might be better with a small box around the letter. |
Speaking of, that A is a dashicon. Here's an updated version, in case you end up touching that part: |
|
Shouldn't we just update the icon source separately? |
|
Yes we should: #29212. I realized in doing that, that the icon is really balanced to have a colored bar under it, and works less successfully when used just as a "transform to cover / add text" icon as is done here. Should we make a larger version of the A icon, just for this? Happy to. |
|
Yes, I think we could use a separate "overlay-text" icon and include the box I mentioned |
|
Went a little fast there and missed that. How about this?
|
|
That works for me :) |
|
Cool: #29215 |
|
I moved the "A" icon in the toolbar but I left the icon itself for your separate PR. |
|
What should we do about these tools? Should they be all in the same toolbar? (no borders at all) |
I think putting zoom, aspect ratio and rotate into one panel is a great start. I'd probably keep apply/cancel separate for now. The whole editing toolbar needs separate love. Here's an older mockup that's beyond the scope of this PR (and probably in need of tweaks):
|
youknowriad
requested review from
ajitbohra and
chrisvanpatten
as code owners
youknowriad
force-pushed
the
update/normalize-image-block-toolbar
branch
from
f69011d to
c2ecfbb
Compare
|
Sweet to see this! It helps understand the toolbar a lot more. Checking the "only text buttons" setting, I think it might need some adjustments to the spacing, but we can check that separately.
|
mtias
approved these changes
Feb 22, 2021
mtias
reviewed
Feb 22, 2021
| value, | ||
| onChange, | ||
| controls = DEFAULT_CONTROLS, | ||
| isToolbar, |
There was a problem hiding this comment.
Do we plan to remove this once we transition?
There was a problem hiding this comment.
I'm not sure we can, we have a valid use-case for it at the moment: The inspector control of the latest posts block. Also, it might have some impact on third party blocks.
youknowriad
changed the title
|
👌 |
diegohaz
reviewed
Mar 19, 2021
| @@ -49,6 +50,7 @@ function DropdownMenu( { | |||
| menuLabel, | |||
| position, | |||
| noIcons, | |||
| isToolbarButton = false, | |||
There was a problem hiding this comment.
Was this added for convenience? This can already be achieved with this code:
<ToolbarItem>
{ ( toolbarItemProps ) => (
<DropdownMenu toggleProps={ toolbarItemProps } />
) }
</ToolbarItem>There was a problem hiding this comment.
TIL I guess I'll refactor these to avoid the prop :)
7 tasks
6 tasks
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.
Related to #25983
This PR normlizes the toolbar of the image block by following the "meta, block level, inline" order in the block toolbar.
This is the current result
Notes
BlockAlignmentControlcomponent in addition to the existingBlockAlignmentToolbarcomponent, the only difference between the two is that one is wrapped with aToolbarGroupand the other is not. I expect that the more we dive into these normalization, the more we'll see this comes up for all the existing reusable toolbars we have.