Skip to content
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

Don't have any accessibility mode specific behavior of quickpick #176081

Closed
TylerLeonhardt opened this issue Mar 3, 2023 · 11 comments
Closed

Don't have any accessibility mode specific behavior of quickpick #176081

TylerLeonhardt opened this issue Mar 3, 2023 · 11 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues bug Issue identified by VS Code Team member as probable bug quick-pick Quick-pick widget issues verified Verification succeeded
Milestone

Comments

@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Mar 3, 2023

Right now, we depend on different behavior of the quick pick when using a screen reader so that the quick pick is accessible. However, the change we made last iteration to make it accessible, also made it less usable.

Lots of good info in this thread:
#166920 (comment)

The overarching problem is that a quick pick cannot have a placeholder and set active items at the same time because the placeholder must be read out by a screenreader to be accessible.

We should try out using aria-live instead of aria-activedecendant to make this work.

@TylerLeonhardt TylerLeonhardt added bug Issue identified by VS Code Team member as probable bug accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues quick-pick Quick-pick widget issues labels Mar 3, 2023
@TylerLeonhardt TylerLeonhardt added this to the Backlog milestone Mar 3, 2023
@TylerLeonhardt TylerLeonhardt self-assigned this Mar 3, 2023
@TylerLeonhardt TylerLeonhardt changed the title Make setting active item accessible Don't have any accessibility mode specific behavior of quickpick Apr 3, 2023
@TylerLeonhardt TylerLeonhardt modified the milestones: Backlog, April 2023 Apr 3, 2023
@TylerLeonhardt
Copy link
Member Author

To @joanmarie and @LeonarddeR, @isidorn gave me your names to ask about ARIA. Here's the craziness I am trying to accomplish. I have:

<div id="a" aria-live="polite"></div>

and then under that (as child), I add in "randomly":

<div id="b" aria-labelledby="list_item_0"></div>

This causes the aria-label on list_item_0 to be read, which is great! This alternative also reads the aria-label... I have my same outer div:

<div id="a" aria-live="polite"></div>

and then adding aria-owns="list_item_0" to this div with an id of a also causes the live region to trigger and reads the aria-label of list_item_0

My question is... it seems like the role on the list_item_0 element is ignored. I was hoping that I could set the role to link or option or something else that might make sense and then the screen reader will read out the fact that that element is a link or whatever. Do you know why this is? Is this a limitation of live regions? A limitation of NVDA?

NOTE: At no time, do these elements that I have mentioned have dom focus. Dom focus is entirely in a different part of the UI and doesn't change when this does happen. This is what I want. I don't want dom focus to leave where it is.

@rperez030
Copy link
Contributor

I think you are expecting aria-live to work for something which is beyond its purpose. The role of the element with id ="list_item_0" is not announced because the purpose of aria-live regions is simply to announce changes in text content. In fact, for the fix you are trying to implement, there is no need for the role of the object to be announced. You are not trying to simulate a focus event. Think of it as announcing search suggestions instead. If the role was announced, screen reader users may be led to believe that keyboard focus is where it isn't.
I would suggest you try with something simpler than that.

<div id="a" aria-live="polite"> text to be read by the screen reader </div>

Avoid nested divs with additional aria attributes which can lead to unexpected behavior. I'm actually surprised that your child div gets read at all. I suppose since it is an empty div that Chrome is calculating the accessible name based on aria-labelledby. You may want to apply an sr-only class to that div so that it is not visible.

  • to

@TylerLeonhardt
Copy link
Member Author

@rperez030 your suggestion also works and is something I tried. My concern with all these options is that the role of the selected item in the quick pick is lost. If I want to mark that an item in the quick pick acts as a link instead of just a regular option, then there is no way for me to do that in a way that a screen reader picks it up.

  • I can't give the input box aria-activedescendant because then no details about the input box are read out.
  • I can't have an aria-live label announce that the active list item has changed because it doesn't include info about the role

I can't figure out the right way to have these requirements:

  • dom focus in the input
  • placeholders & text operations (selection, deletion, etc) in the input read out
  • fake focus in the list which will read out the label of the current active item
  • fake focus in the list which will read out the role of the current active item

Please let me know if you have any ideas how to do this, otherwise I'll get almost there and use your simpler suggestion which gets us all of those bullets except for the last one.

@rperez030
Copy link
Contributor

hi @TylerLeonhardt. I will comment on some of your points and suggest a simpler approach in the end.

  • the role of the active item had never been announced because the active element is inside of a list, and NVDA treats each element inside of a list as having the role LISTITEM.
  • The 2 roles you mention, link and option, have the same keyboard behavior, so announcing them in this context is irrelevant. If someone can come up with something that requires a different keyboard behavior inside of that list, we will likely have bigger problems.
  • An aria live region is good to announce elements as we type, but when arrow keys are used to navigate, it is necessary that focus is managed inside the list either by aria-activedescendant or true keyboard focus, otherwise the specific screen reader commands related to the element with focus will not work.

Simpler approach:

As @scottaohara mentioned to me in a side conversation we are having, all of this can be avoided simply by replacing the current accessible name of the widget "Quick Input" with the placeholder text. I don't remember if there was a reason not to do that in the first place. This will resolve the problem that started the entire thread, allow us to continue using aria-activedescendant and eliminate the need to have a screen reader optimized widget. The name "Quick Input" is not helpful anyway. The only inconvenient of this method is that we cannot track the activity in the edit field using a screen reader, but we couldn't do that with the previous behavior either. It is perhaps a smaller price to pay. We still could allow something like that if we can remove aria-activedescendant when shift +Tab is pressed, for example, and add it back if the user presses down arrow key or the Tab key.

Does it make sense?

@TylerLeonhardt
Copy link
Member Author

TylerLeonhardt commented Apr 5, 2023

Yes that makes sense. #179217 this PR will set the aria-label of the list to the placeholder which from my testing works nicely.

I haven't implemented that shift +Tab behavior you suggest... but it's something we should do... tracked here #101183

@TylerLeonhardt
Copy link
Member Author

Fixed in #179217

@isidorn
Copy link
Contributor

isidorn commented Apr 5, 2023

I just tried out the Quick Pick behaviour on Windows with NVDA and what you did feels good to me.
So the first element gets nicely focused. And we read the placeholder via alert. This feels intuitive and good to me -> verified

@isidorn isidorn added the verified Verification succeeded label Apr 5, 2023
@Fm530755

This comment was marked as off-topic.

@Fm530755

This comment was marked as abuse.

@starball5
Copy link

@rperez030
Copy link
Contributor

This is due to changes in the previous iteration for the component. I'm surprised this was promoted to production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues bug Issue identified by VS Code Team member as probable bug quick-pick Quick-pick widget issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants