Hide Settings button to screen readers #38360
Closed
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
alexstine
added
[Type] Enhancement
alexstine
requested review from
talldan,
ciampo,
david-szabo97 and
tellthemachines
alexstine
requested review from
adamziel,
draganescu,
getdave,
kevin940726 and
noisysocks
as code owners
|
Size Change: +176 B (0%) Total Size: 1.14 MB
ℹ️ View Unchanged
|
MarcoZehe
suggested changes
Jan 31, 2022
There was a problem hiding this comment.
This is problematic. aria-hidden removes this from screen reader views, but it is still a focusable element. When tabbing or arrowing through this toolbar, this element isn't skipped, but focus is put on an inaccessible element. In focus mode, this results in silence. In NVDA's reading (virtual cursor) mode, this leads to two non-speaking tab stops, because after the inaccessible element that focus moves to, the next tab causes NVDA to try to recover, but while doing so, not speak the next focused element properly.
aria-hiding a focusable element is almost never a good idea unfortunately.
Perhaps the better solution here would be to give this a more explicit label. So instead of saying "Settings", say something like "Settings side bar", at least for screen readers. That way, it is clear that this is not an immediate sub menu that follows, but an element which gets toggled here, but which is in a different place.
|
@MarcoZehe I am not sure you are aware, but the issue is the Settings Sidebar opens up super far away in the DOM. If you jump by landmark, it is labeled Editor Settings. My concern is I am not sure how to label this in a way users will understand. I could say something like "Settings: Opens Settings Sidebar below". That really isn't explaining to users where the Settings Sidebar is or how to get to it. This is one of my biggest complaints in Gutenberg, having things so far away in the DOM from the originator. In practice, it seems to me like it would make more sense if Settings placed focus on the Settings Sidebar but it doesn't appear to. Maybe that is what I should fix instead? If you open Settings, at least focus would be properly managed. Quite honestly, not sure why that doesn't work already... |
|
I totally get where you're coming from. I always found that button up there very confusing, and can imagine that it is confusing even for sighted people. Especially when, even when that side bar is closed, there still is a button to open it in its vicinity, too. That extra Settings button up in the main toolbar just feels... superfluous. Unfortunately, just hiding this focusable element is not the way to solve it I think. Can you try moving focus to the Settings panel when the button is pressed, and see what the experience is? |
draganescu
reviewed
Jan 31, 2022
There was a problem hiding this comment.
In addition to @diegohaz 's comment of having an inviisible focusable thing as a result, the code addition itself is straightforward.
However, it feels weird that we're just toggling off visibility everywhere that component is used. I wonder when do we need to set hideToggleToScreenReader to true?
|
@draganescu The new plan is to figure out how to manage focus when Settings is selected. I am having a terrible time finding where the current focus is being handled when Open Document Settings is selected. It's easy enough to throw in a new effect but I'd rather modify what is already managing focus if that is a possibility. Lot of moving parts in the Sidebar. |
|
I think I am going to close this out for now. I can't find an easy way to just target the Settings button and I don't think the improvement is worth the work to separate from the interface complementary sidebar. The fill method with children is great until you need to customize one, then it becomes not worth it. :( I may revisit this at some point down the line but this PR was the wrong solution anyway. |
|
This has been a problem since the early days of Gutenberg, and somehow it never got fixed. It's tracked in #15322, and there's even agreement on a solution - moving focus directly from the top toolbar button into the Settings panel - but it hasn't been implemented yet. |
|
@tellthemachines I understand what needs to happen and may be able to lead this from a technical perspective, but I am not sure how things like styling would come in to play. Essentially, it would be really nice to start treating the Document Settings/Block Settings Sidebars like the List View/Inserter Sidebars. This way, we can use refs and other nice React magic to hold complete control over it. There really isn't a good way to do it in it's current state without a lot of hacking. Would kind of hurt portability if a bunch of class names needed hard coding for focus. This would probably be a better task for someone other than me as it seems like this whole thing needs massive refactor. |
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.
Description
Because the Settings Sidebar opens so far away, I've hidden the Settings button to screen readers to avoid confusion. You can still access the Settings Sidebar if it is closed by selecting Open Document Settings button.
Testing Instructions
Screenshots
Types of changes
Enhancement
Checklist:
*.native.jsfiles for terms that need renaming or removal).