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

Sign in quick pick dialog: focus movement could be improved #122546

Open
isidorn opened this issue Apr 28, 2021 · 5 comments
Open

Sign in quick pick dialog: focus movement could be improved #122546

isidorn opened this issue Apr 28, 2021 · 5 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
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Apr 28, 2021

  1. Turn on Settings sync
  2. Get the following dialog

Press tab - focus moves to checkbox. Press tab again, focus moves to the other checkbox.

Ideally the first tab would move focus to the "Sign in and Turn on" button. And not to the checkbox

Screenshot 2021-04-28 at 15 42 44

@isidorn isidorn added the accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues label Apr 28, 2021
@TylerLeonhardt
Copy link
Member

VoiceOver doesn't seem to be reading the description anymore both before and after this change... we may have to revert #122054... @isidorn can you give it a try on your end?

@TylerLeonhardt TylerLeonhardt added the quick-pick Quick-pick widget issues label May 7, 2021
@isidorn
Copy link
Contributor Author

isidorn commented May 7, 2021

Yes I can reproduce.
Well if VoiceOver was not reading the description before the change for #122054 then there is no point in reverting. Or I misunderstood something?

The issue here is that we are using aria-activedescendent to tell the screen reader what is the active element, and based on that the screen reader will read.
It would be cool if there is a way to append a label to be read by a screen reader apart from the one provided by the activedescendent. @joanmarie might you have a hint on how to do this? So A contains B, browser focus is on A, A has activedescendent pointing to B. Is there a way for A to append something to what is read by the screen reader?

@TylerLeonhardt If there is not nice way then we will have to use an aria alert to announce the placeholder

@TylerLeonhardt
Copy link
Member

I'm also concerned that if I change the focus to the Sign in and Turn on button that it might affect keybindings:

if (!visibilities.inputBox) {
// we need to move focus into the tree to detect keybindings
// properly when the input box is not visible (quick nav)
this.ui.list.domFocus();
// Focus the first element in the list if multiselect is enabled
if (this.canSelectMany) {
this.ui.list.focus(QuickInputListFocus.First);
}
}

Well if VoiceOver was not reading the description before the change for #122054 then there is no point in reverting.

You are correct.

Anyway, wanted to call that out but still interested in hearing what @joanmarie has to say :)

@joanmarie
Copy link

joanmarie commented May 7, 2021

I'm afraid I need some more context. 😄

  1. What description and what placeholder? Are these two things the same?
  2. When should these things be read, every time the active descendant changes? Just the first time? Something else?

In the meantime, I went looking for a placeholder and found an input with role combobox which is giving Orca some trouble due to the accessible events it's getting (and/or not getting). Maybe that's related to this placeholder/description?

The input displays "Select an account to sign in" and when I Tab between it and the list of sign-in sources, Orca says nothing. Here's why: When focus is in the list and I shift+tab into the input I get two accessibility events:

object:state-changed:focused(0, 0, 0)
	source: [list item | Sign in with Microsoft]
	application: [application | code-insiders]
object:state-changed:focused(1, 0, 0)
	source: [list item | Sign in with Microsoft]
	application: [application | code-insiders]

The first is the selected active descendant giving up focus, the second is that same active descendant claiming focus. Thus Orca thinks nothing has changed and doesn't say anything. But visually I see that focus is in the input. And I get the same two events when I Tab back down to the list from that input.

The above needs to be figured out and fixed wherever it's happening. Whether or not this is the same VO problem being discussed 🤷‍♀️. Again, I need more context. Sorry and thanks!

@TylerLeonhardt
Copy link
Member

@joanmarie

What description and what placeholder? Are these two things the same?

Sorry about that. I think @isidorn got those mixed up. Let me provide more context.

Based on your text "Select an account to sign in" I think we are talking about two different quick picks. We are talking about the one that says "Please sign in to synchronize your data across devices" that has a "Sign in & turn on" button next to it.

@isidorn said placeholder before because the description and placeholder visually appear to be in the same place... but the placeholder is shown when the input box is enabled, and the description is shown instead when the input box is disabled.

Settings Sync disables the input for this quick pick so we are purely talking about the description here. Sorry for the confusion.

When should these things be read, every time the active descendant changes? Just the first time? Something else?

I... am just getting ramped up on A11y-related changes and am quite the noob, sorry... so I'll let @isidorn answer it correctly but ideally, the description should be read out when the quick pick is shown... so it might look like this:

  • click the person icon in the bottom left of vscode
  • click on "Turn on Settings Sync.."
  • That brings me to the "Please sign in to synchronize your data across devices" quick pick and the screen reader reads out "Please sign in to synchronize your data across devices" before saying anything about the button or quick pick list.

It's possible that you see a different quick pick before the "Please sign in to synchronize your data across devices" quick pick... if so, let me know what that one is and we can talk about that one too. I want to fix all the screen reader issues for quick pick :)

@TylerLeonhardt TylerLeonhardt added the bug Issue identified by VS Code Team member as probable bug label Oct 4, 2021
@TylerLeonhardt TylerLeonhardt added this to the Backlog milestone Dec 6, 2022
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
Projects
None yet
Development

No branches or pull requests

3 participants