Fix the block variations transforms labeling and improve UI clarity #59916
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
afercia
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @jarekmorawski. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +147 B (+0.01%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
afercia
force-pushed
the
fix/block-variations-transforms-labeling-clarity
branch
from
2b9101c
to
eadd817
Compare
|
Rebased on top of latest trunk and fixed a few typos in the PR description. |
|
Thank you, @afercia! I added this to my list and will try to review it early next week. |
Mamaduka
approved these changes
Jul 15, 2024
jasmussen
requested changes
Jul 15, 2024
There was a problem hiding this comment.
The addition of a separator and the added spacing makes the inspector unreasonably longer, pushing downwards the primary block controls. Let's hear from others if there's a better way to solve this, that makes better use of the available space. If a better option presents itself, feel free to dismiss my review.
|
I agree with @jasmussen. Adding more doesn't always equate to a more understandable interface. |
|
For what it's worth, here's how we solved a similar problem in the Product Collection block in WooCommerce. The dropdown uses a tertiary button style and sits in the same column as the block's description.
|
afercia
force-pushed
the
fix/block-variations-transforms-labeling-clarity
branch
from
eadd817
to
148b64f
Compare
|
@jasmussen @richtabor thanks for your feedback. To me, this is a valid accessibility issue. It's not clear what those buttons are because there's nothing that explains it. Any group of controls should be clearly identified, either by a visible legend of a fieldset, or a visible label, or a heading, depending on the nature of the controls. In this case I opted for a heading. I'm glad to remove the separator or make visual adjustments based on your design feedback. It's totally okay if you don't agree on the visual design change but in that case I'm kindly asking you to provide an alternative solution to a valid issue. Just saying 'no' without providing an alternative solution isn't acceptable. Thanks. @jarekmorawski thanks but unfortunately the tertiary buttons aren't accessible. In absence of color, they just look like text. They should be refactored but that's another issue. |
How does this differ from icons say in the top toolbar, or block toolbar? |
I measured the 'unreasonably'. It's about 40 pixels. See screenshot below. I don't think 40 pixels are unreasonable. I'd appreciate some more objective feedback because stating that 40 pixels make the inspector 'unreasonably longer' doesn't sound fair to me. Before and after:
I'd be happy to add visible headings in the top toolbar and block toolbar, even though I'd think those are slightly different cases because the context helps understand what those controls are. |
…and legend styling consistent.
afercia
force-pushed
the
fix/block-variations-transforms-labeling-clarity
branch
from
148b64f
to
46a8a82
Compare
richtabor
requested changes
Jul 27, 2024
There was a problem hiding this comment.
While I appreciate the effort, the design changes add additional complexity to the Inspector/Block Card; complexity that is not worth the perceived benefit.
For one, the Inspector controls are further down the sidebar.
Second, the additional label detracts from the primary intention of the Block Card: to clearly inform you of the selected block. As proposed, there is no primary intention, only two secondary: the selected block and its transforms.
And third, with this proposal, the Styles and Settings tabs conflict visually with the transform icon buttons, as the border above makes the transform icon buttons seem like one interaction unit with the Tabs.
The previous design, without these changes, was much more simplistic and purposeful. Once you interact with the transforms, you know what they do.
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.
Fixes #59898
What?
The block variations in the block inspector may use 3 different components based on some conditions. All three components use less than ideal labeling. Also, the UI doesn't really make clear what the icon buttons are about.
Minor: when rendered as a dropdown button, the gap between the toggle button and the dropdwon menu is too large.
Why?
How?
aria-labelthat uses the same text of the visual label. This hasn't changed, it's how the component works.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Before:
After: