Skip to content

List: re-anounce element when focus did not change on typing#97057

Merged
isidorn merged 5 commits intomasterfrom
isidorn/list-navigation-aria
May 8, 2020
Merged

List: re-anounce element when focus did not change on typing#97057
isidorn merged 5 commits intomasterfrom
isidorn/list-navigation-aria

Conversation

@isidorn
Copy link
Copy Markdown
Collaborator

@isidorn isidorn commented May 6, 2020

As suggested by @joanmarie here #95961
We re-announce the element if we notice the focus did not change on typing.

I have tried this and unfortunetly does not work with Orca for me.
The problem is the following, let's say I have element abc.txt:

  • If I type a the element gets focus and is nicely read
  • If I type ab fastly we set the focus and nicely alert and all is good
  • If I type abc fastly it does not work, because after user typed b we set the content of the aria-live region to be abc.txt, after user pressed c we set the content of the live region to the same content again and thus Orca does not announce anything.

So I see two ways to fix this

  1. @joanmarie is it possible to somehow force orca to re-read the live content even though it did not change. Idealy if I clear the live content element and reset the content that should work, however it does not
  2. @joaomoreno do we know when the user "stopped" typing for navigation. If yes, only then we could alert. However this would still not work 100%, since after stopping typing user could start typing the same element and nothing would be read, however it would cover most of the cases

If you want to repro:

  1. Build vscode from this branch
  2. Open Explorer and have file abc.txt and type as I explained above

@isidorn isidorn added accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues under-discussion Issue is under discussion for relevance, priority, approach list-widget List widget issues labels May 6, 2020
@isidorn isidorn self-assigned this May 6, 2020
this.list.setFocus([index]);
this.list.reveal(index);

if (index === start) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually start will be 0 when no element is focused, so we probably shouldn't announce it in this case.

Comment thread src/vs/base/browser/ui/list/listWidget.ts Outdated
Co-authored-by: João Moreno <joao.moreno@microsoft.com>
@isidorn
Copy link
Copy Markdown
Collaborator Author

isidorn commented May 6, 2020

@joaomoreno thanks for the review. As for my question: do we know when a user stopped typing: you must use some timeout somewhere. Based on my thinking from 2. above that might be the place that we sohuld actually do an alert. Though option 1 is better so let's wait for @joanmarie to reply

@joanmarie
Copy link
Copy Markdown

@isidorn: I've built your branch. Lemme play with it and Orca and see if I can come up with something. Stay tuned!

@joanmarie
Copy link
Copy Markdown

joanmarie commented May 6, 2020

tl;dr: Maybe the solution is for the user to use their screen reader's "where am I?" command rather than try to fix this in VSCode (et al.).

Details:

So I created an accessible-event listener that works without Orca to see what Orca had to work with. It appears that we're not even getting as far as "that's a duplicate so Orca's ignoring it." Here are the relevant (presentable) events I get from typing "conf" (or maybe it was "con". Anyway....):

code-oss [alert | ] object:text-changed:insert 0 12 config.guess
code-oss [alert | ] object:children-changed:add 0 0 [text | config.guess]
code-oss [alert | ] object:children-changed:add 0 0 [text | config.guess]
code-oss [alert | ] object:children-changed:add 0 0 [text | config.guess]

The last three are being ignored by Orca because the children being added are static text leaf nodes. Static text leaf nodes are extraneous objects which Orca always ignores by design. It's a long story and not limited to live regions. But changing that behavior in Orca, even if just for live regions, would probably break more things overall than would solve here.

What I was expecting to see is multiple object:text-changed:insert events (which Orca would then probably ignore due to being duplicates). I haven't looked at the Chromium code yet, but guess we're not getting them because the parent element (alert)'s content hasn't changed. And if that is indeed the case, that filtering by Chromium would be a good thing IMHO. There are so many accessibility events that getting duplicates just sucks....

As for what to do....

One possibility is the timeout idea you mentioned @isidorn.

Another idea might be to set the alert text to only be what follows the letter typed, e.g.:

  • "c" -> "config.guess"
  • "o" -> "onfig.guess"
  • "n" -> "nfig.guess"

Another approach would be to try to figure out how to get the text insertion events fired by the alert. (We'd still have the problem of Orca then ignoring those insertions, that I could take a look at.)

And there's always the possibility of the user under these conditions using the screen reader "where am I?" command and you making no changes in VSCode. For Orca's desktop keyboard layout, KP_Enter tells you where you are.

To be honest, the more I think about this the more I think the thing to do when the user interrupts speech and nothing has changed is to use the screen readers "where am I?" command. Because this use case can happen in web apps, native apps, etc.

Related aside: When I do the "where Am I?" command in Orca on that tree, I'm told config.guess is "0 of 0". That is due to a Chromium bug which was already fixed: https://bugs.chromium.org/p/chromium/issues/detail?id=1066632

@isidorn
Copy link
Copy Markdown
Collaborator Author

isidorn commented May 7, 2020

@joanmarie thanks a lot for the analysis. I do agree with you that the "where Am I?" command might be the way to go I also think that it would be nice if VS Code informs the user where he is once he stopped typing.
Due to that I have made some changes to the PR so we announce only once the user stopped typing and only if he typed more than 1 character. Since if he typed one character there will be duplication.
I tried this out and works pretty well with Orca.

@joaomoreno can you give this PR a new review, and if you think it is too ugly we do not have to merge it in. However I think this makes the list keybaord navigation much better for screen reader users, and we have lists everywhere in the UI and our screen readers users use keyboard navigation a lot so improving that experience would be pretty cool.


private automaticKeyboardNavigation = true;
private triggered = false;
private charactersTyped = 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we rather be storing the previously declared item's label? Or the element index? Then you could compare labels/index and if it hasn't changed, then we don't announce it, right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joaomoreno makes sense, I have pushed that change now. Check it out. I tested and works nice.

Comment thread src/vs/base/browser/ui/list/listWidget.ts
Copy link
Copy Markdown
Member

@joaomoreno joaomoreno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@isidorn isidorn merged commit dd9e1be into master May 8, 2020
@isidorn isidorn deleted the isidorn/list-navigation-aria branch May 8, 2020 09:24
@isidorn isidorn added this to the May 2020 milestone May 8, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jun 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues list-widget List widget issues under-discussion Issue is under discussion for relevance, priority, approach

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants