Small improvements for the sidebars and Close buttons labelling #49614
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
requested review from
kevin940726,
ntwb,
nerrad,
ajitbohra,
draganescu,
talldan,
noisysocks,
tellthemachines,
adamziel,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
|
Size Change: -101 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 91b9e04. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4637467675
|
afercia
force-pushed
the
update/better-sidebar-close-button-labels
branch
from
ab31ac3 to
740d13a
Compare
|
@afercia My only concern with open labels is how spread out in the DOM these buttons are from the sidebars they show.
There is still a huge problem in Gutenberg with panels opening so far away. I think in this case, keeping the sidebar is a good idea for the users that might understand enough terminology to find what just opened. The only way I would be completely okay with this change is if there was true commitment from devs to start fixing these DOM inconsistencies. I will not hold my breath on it. For some reason, this pattern is insanely overused in React. |
|
I added back the word 'sidebar' to the keyboard shortcuts descriptions used in the Keyboard shortcuts modal dialogs so that they both read:
@alexstine I totally share your concerns about the distance in the DOM between the opener buttons and the various sidebars / panels they open. That was remarked several times across the history of this project. Unfortunately there's no consensus on that and seems other needs prevailed over accessibility needs. Still today, that's one of the most serious accessibility barriers in the Editor. However, that applies to the controls that open sidebars and the like. Instead, most of the changes in this PR are about the controls that close. A quick recap of the changes in this PR:
I think these changes are OK, as the ARIA region is labelled 'Navigation'. I'm going to create a new issue about the labelling of all the editor ARIA regions. Will ping you there. Thanks! |
|
@afercia Can you give more specific descriptions as to where I can find these labels? I could figure out some of them but not all. I am not familiar with the site editor at this point. I might be the wrong person to fully review this.
|
|
@alexstine there you go:
|
|
I think these changes are good but this PR is quite large to review. Could really use a review from someone who knows more of the codebase. I am not very familiar with the internal workings of the edit-site package. @annezazu Can you put eyes on this one and try to get us some tech review here? Thanks. |
|
Thanks so much for the tag in @jeryj or @tellthemachines can you all take a look? |
|
Thanks for improving this labelling @afercia! It all looks good to me. There is a conflict with trunk in To get this PR merged, I think we should remove the change from |
|
Thanks @jeryj .
I'd rather go the other way around for the reasons explained in #50067 (comment), keep the label use 'navigation' nad coordinate with #50067. |
afercia
force-pushed
the
update/better-sidebar-close-button-labels
branch
from
91b9e04 to
5c3e907
Compare
|
Hat tip to @aristath for pointing me to one more reason to avoid the term 'sidebar': on small screens, most of these UI sections aren't 'sidebars' any longer. They're more full-width overlay panels. Still, they're referenced to as 'sidebars'. Screenshots: List view:
Styles:
|
afercia
force-pushed
the
update/better-sidebar-close-button-labels
branch
from
5c3e907 to
a03e4e9
Compare
skorasaurus
added
[Focus] Accessibility (a11y)
aristath
approved these changes
May 3, 2023
This was referenced May 8, 2024
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 #49613
Follow-up to #49555
What?
Close.Why?
Closebutton are better for all users, as long as the context is clear.How?
Shortens / updates a few translatable strings.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast