Fix accessibility of the edit site global styles Variation #59508
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
|
Size Change: -36 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
afercia
force-pushed
the
fix/presets-font-variant-buttons
branch
from
e226a6e
to
d8ae148
Compare
|
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 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. |
This was referenced Mar 7, 2024
scruffian
force-pushed
the
fix/presets-font-variant-buttons
branch
from
d8ae148
to
0a8a142
Compare
|
I rebased this. |
|
This makes sense to me, but it's a change in the design so I'd like to hear from someone with a design opinion |
|
The problem with the checkmark to add to the shape change is that this checkmark is used for bulk selecting in the media library:
Here's an alternative: 
|
I'm not sure that's accurate. Actually, the checkmark in the meia grid is used to indiacate a selection both for a single selection, for example when inserting an image, or for multipel selection when inserting a gallery. So it's not jspecific to bulk slection, it's just 'selection'. Screenshots:
Anyways, I'm not sure I understand how the checkmark can be 'a problem'.
The entire point of using a checkmark (it could also be another shape) is to not use an outline to indicate selection. The outline is universally used to indicate focus. It should not be used for other purposes. Additionally, in the alternative mockup above:
Overall, I'm not sure the alternative proposal solves the issue. |
|
The styles variations have changed a lot on latest trunk at the point I'd prefer to start over with a new PR, when I have time. Closing this one. |
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 #59459
What?
The global styles Varaition are made with foucsable
divelements instead of native buttons.When using
divrather than a button, all the native button features and intereaction should be re-implemented. That appears to not be the case with the current implementation.Why?
Overall, given the amount of code that would be required to make a focusable
divelement fully accessible and behave like a button, it's way better to use a button element. There's no reason to not use a button element a smodern browsers allow to fully style them.More importantly, custom implementation should be avoided as they defeat the purpose of building a library of reusable component in the first place. It's unlikely that custom implementations have the same level of accessibility and the same design consistnecy of the base components. Instead of implementing custom interactive controls in the UI, the base component should be used and if a new variant of a base component is desired that should be added to the base component.
This PR replaces the focusable
divelements with a Button compoennt, becuase the Button brings in all the accessibility, interaction, and base styles like focus style that we need to make a better usable and accessible UI.How?
divcustom implementation with a Button component.Testing Instructions
src/wp-content/themes/twentytwentyfour/styles/maelstrom.jsonfile and add a description to the style variant e.g.:"description": "This is the Maelstrom description"Maelstromvariant button does have a visually hidden descriptionThis is the Maelstrom description.aria-current="true"attribute, this hasn't changed.Maelstromvariant button does have a visually hidden description.Testing Instructions for Keyboard
Screenshots or screencast
Before with focus and active states. Notice the states difference is basically only a color change of the visual outline (although there's a barely noticeable change in the outline thickness). Basically this is color alone, which is insufficient.
Before with focus and active states: