-
Notifications
You must be signed in to change notification settings - Fork 30.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Settings editor accessibility improvements prototype feedback #104318
Comments
hello @roblourens thank you for ping. the only inconvenience is that when tabbing to the setting, its name and info are read again. is it possible to fix it? |
Great, I'm glad to hear that. Yes, I can clean that up. |
Option A: This works really well. It's nice and easy to navigate and feels like a natural part of the interface. Option B: This is more like what I would expect when coming to a settings page for the first time, but I think it presents a slightly inferior user experience in that it is both slightly more verbose (at least with my NVDA configuration), and just requires more tabbing around (especially if you're not a screen-reader user). Ultimately I think that option a is the superior interface, even if it does cause a small amount of confusion (and I think for most users it will be very small - like "oh, tab doesn't work, let me try arrows") when first encountered. |
Thanks for that feedback @SaschaCowley! |
I actually prefer variant of option b, to be honest, although just testing the current insiders and the option a approach works. The only thing I find weird is that the actual settings tree (the one with all settings) can be used to navigate between settings categories, that is sort of redundant. If option a would be implemented and would stay, I'd likely prefer the settings tree to contain only settings for the current category. |
@webczat, for me, new controls seem to be scrolling into view when I tab to them, but it is manually paged (which is the intended behaviour). |
@webczat Note that I am a screen-reader user, I was just adding the bit about non-SR users since the main focus of these improvements seems to be us, but the UX for everyone else should be considered too. |
@webczat I am open to scoping the settings list to the selected category for option A, as well. Maybe behind a setting or with some UI toggle. But I didn't put it out there for this experiment just to limit the number of changes that we have people evaluating. In option B, yeah it scrolls to reveal the element that you tab to, relying on native browser behavior for this rather than the virtualized list tabbing hack we have to do to make it work currently. |
now when the option A is accessible I see no reason to limit the settings to one category. |
I mean, when you go beyond one page, does it autoscroll to next page scoped in a category? for that case (in a single category) it would actually make sense. |
Well, it is a bit confusing and such. You have a tree with just categories, then another super tree with everything, like unnecessary duplication. Even though views synchronize so it is not missleading, but still a little bit confusing. |
No, you have to trigger the next page navigation action manually. Some other feedback we have gotten indicates that screenreader users don't like content changing dynamically under them. I think the model with the two trees is not totally original, we even use the same model for navigation on our website - there is a similar "table of contents" on the right side of every page. But for the reasons you give, I'm open to giving the option to change it. |
content changing under us matters in browsemode, yes. on the other hand I don't feel comfortable pressing something like the load more button instead of just being able to go to all the settings in a category with a tab, I would use browse mode only/mostly for emergency. Ofc other people may use it often and then it may be worse. |
I've pushed this back into Insiders starting tomorrow, and I intend to stabilize and ship it this month (option A). |
Continuing in #106897 |
Over the past couple iterations, we have been listening to feedback and exploring options for making the settings editor easier to use with a screen reader. Here are some of the issues we've heard:
We investigated two possible solutions to these issues and have produced two prototypes, and now we are looking for as much feedback as possible, from those who use screen readers as well as those who don't.
Option A: "Focusable Rows"
In this mode, navigating the settings editor becomes more like navigating a list. A setting row becomes focused, and the settings list can be navigated with the arrow keys. The user can navigate to a setting then tab into its control to edit it.
Some advantages to this approach:
We've made another improvement which is currently only implemented for Option A, although it is not tied to this option and will make it into the settings editor regardless. You can press "escape" when focus is in the settings list to move focus back to the category list, and then press "escape" again to move focus back to the search bar. This solves the issue of having difficulty moving focus out of the settings editor, and emphasizes the hierarchical nature of these 3 main parts.
Updated Sep 2 - This has been removed from the Insiders build for our August release, please try this in the builds below
Updated Aug 17 - This is currently in the Insiders build, and will be until the end of the August iteration. https://code.visualstudio.com/insiders/Here are the links to builds including option A:
Option B: "Non-Virtualized List"
In this mode, navigating the settings editor becomes more like navigating a typical form, not a "list". We swap out the virtualized setting list for a non-virtualized list - just normal HTML on a page. This means that screen readers will be able to navigate the list using browse mode. We also introduce pagination with a limit of 20 settings on the page at a time. This ensures that the settings list can actually be tabbed through within the length of a typical human lifetime.
Some advantages to this approach:
Here are the links to builds including option B:
Previous discussion: #97567
The text was updated successfully, but these errors were encountered: