You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
Note: as done for the previous change in #65131, I guess this will warrant a dev note.
What?
While using one of the new button size variants makes totally sense, the new 'small' size of 24 pixels is unnecessarily too small.
Why?
The target size of any interactive control should be as big as possible for better usability and accessibility.
How?
Switches the size from 'small' to 'compact' (32 pixels).
Aside: adjusts the spacing on the left of the close button to 8 pixels for consistency with the spacing between buttons used acrosso the editor. This can be seen in the Classic block modal dialog.
Testing Instructions
Open a few modal dialogs in the editor, for example:
The keyboard shortcuts modal dialog.
The modal dialog to rename a block e.g. a Group block.
The Classic block modal dialog.
Observe the X close button size is now 32 pixels.
In the Classic block modal dialog, observe the spacinf between the 'Enter fullscreen' and 'Close' buttons is now 8 pixels.
Testing Instructions for Keyboard
Screenshots or screencast
Highlighting the buttons with a red background to better illustrate:
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 props-bot label.
If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
Flaky tests detected in 2ca94dc.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.
The reason will be displayed to describe this comment to others. Learn more.
I'm going to tag @WordPress/gutenberg-design for review in case there are other considerations, since the close button size was already spec'ed in the Figma. (Designers: Please also confirm what the spacing should be between the header buttons.)
Code changes look good, and I'm ok with these changes as long as there were no other design considerations for the button size. If there were other considerations, we can weigh that against having a larger target size, since the current size technically already meets WCAG AA.
the current size technically already meets WCAG AA.
That is the whole point of this PR. As explained in #66758 WordPress should not aim for just the mimimum technical compliance. Whenever possible, when there is enough space, the target size should be reasonably bigger than the minimum required of 24 pixels.
Aiming for only the minimum requirement is an approach that relegates accessibility to a remediation level, while WordPress should aim to provide the best experience for all users instead.
Thanks @jameskoster for your feedback.
I'm not opposed to leave this PR available for testing for a few days so that anyone can test when they have a chance. On the other hand, it's a very little and self-contained change that could be easily reverted, if need be.
Merging now would expose the change to more eyes, which is always welcome to catch potential issues. Thanks.
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 #66758
Note: as done for the previous change in #65131, I guess this will warrant a dev note.
What?
While using one of the new button size variants makes totally sense, the new 'small' size of 24 pixels is unnecessarily too small.
Why?
The target size of any interactive control should be as big as possible for better usability and accessibility.
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Highlighting the buttons with a red background to better illustrate:
Before:
After: