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

Test: Changes view with screen reader #93301

Closed
2 tasks done
isidorn opened this issue Mar 24, 2020 · 9 comments
Closed
2 tasks done

Test: Changes view with screen reader #93301

isidorn opened this issue Mar 24, 2020 · 9 comments
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues testplan-item
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Mar 24, 2020

Refs: #88695

Complexity: 3


Note: if you are assigned windows, please use the NVDA screen reader. If you are assigned any OS please either use macOS with VoiceOver or Linux with Orca screen reader.

We have improved our difference view with regards to accessibility. Have a git tracked repostiroy, some file with multiple changes in it and turn on your screen reader. Open the changes view of that file.

Verify:

  • F7 and shift+F7 navigate you through all the diffs and the screen reader correctly reads the added and removed content
  • When you enter the accessible Diff view screen reader mentions to use "Stage/unstage/revert selected ranges"
  • As you navigte through the content, the content is first read out and at the end the line number is read out
  • You can use "Stage/unstage/revert selected ranges" to stage / unstage/ revert the currently active diff
  • Pressing Esc takes you out of this view and you are navigated to the regular changes view on the correct position with the right change selected
  • The whole flow makes sense. And any feedback on improving this is very welcome
@isidorn isidorn added testplan-item accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues labels Mar 24, 2020
@isidorn isidorn added this to the March 2020 milestone Mar 24, 2020
@jvesouza
Copy link

@isidorn Please correct me if I'm wrong, but to open the changes view can I choose a file from the list of modified files and press the enter key? If so, I think it would be interesting for the focus to be moved to the open editor as soon as the key is pressed. It took me a while to discover that the editor with the changes was open and I had to use the tab key to find it.

@isidorn isidorn changed the title Test: Change view with screen reader Test: Changes view with screen reader Mar 25, 2020
@isidorn
Copy link
Contributor Author

isidorn commented Mar 26, 2020

@jvesouza agree with you 100%. Filled this issue to track this #93493

@jvesouza
Copy link

Verify:

 `F7` and `shift+F7` navigate you through all the diffs and the screen reader correctly reads the added and removed content

It works like a charm.

 When you enter the accessible Diff view screen reader mentions to use "Stage/unstage/revert selected ranges"

orca doesn't say anything about it.

 As you navigte through the content, the content is first read out and at the end the line number is read out

Correct.

 You can use "Stage/unstage/revert selected ranges" to stage / unstage/ revert the currently active diff

I didn't find a way to do that.

 Pressing `Esc` takes you out of this view and you are navigated to the regular changes view on the correct position with the right change selected

Correct.

 The whole flow makes sense. And any feedback on improving this is very welcome

It seems that I can only navigate the content using the up and down arrow. It would be interesting if we could use left and right, home and end in addition to ctrl + home and ctrl + end.

@isidorn
Copy link
Contributor Author

isidorn commented Mar 26, 2020

@jvesouza thanks a lot for trying it out!
You should be able to use F1 > stage selected range if your file is under git source control to stage changes.

Better navigation makes sense. I have created a feature request to track that #93498
As for Orca not reading the aria label - I will have to double check that. Thanks

@jvesouza
Copy link

You should be able to use F1 > stage selected range if your file is under git source control to stage changes.

@isidorn Thanks for the clarification. Do you think it would make sense to create shortcuts to these functions? Something like: 's' for stage and 'u' for unstage ....

@isidorn
Copy link
Contributor Author

isidorn commented Mar 27, 2020

@jvesouza users can define whatever shorcuts he wants for these.
But yes I do agree with you, since this is a readonly view we could have some easy shortcuts.
I have created this issue to track this #93564

@miguelsolorio miguelsolorio removed their assignment Apr 1, 2020
@miguelsolorio
Copy link
Contributor

There's just a few issues with navigating through the list that feel inconsistent like when you have a single range of edits, F7 goes down line-by-line but when you have multiple ranges that shortcut toggles between the different ranges.

It also felt weird that the shortcut to enter this view is also not the same one to exit (you have to hit esc. Not sure if this is common in other modes but we use the same shortcut to toggle various things (sidebar, activity bar, etc.) and would suggest to maybe stick to a single shortcut to toggle and then another to navigate between changes.

Also, should this mode have a setting so users can always set this?

Besides those two things and the other issues i already files, this seems like a really cool mode.

@isidorn
Copy link
Contributor Author

isidorn commented Apr 2, 2020

@misolori thanks a lot for feedback.
Furrenlty we use F7 which cycles through all the changes, similar to how F8 cycles through all the errors. Esc exits both of these. And I think it is common for keyboard users to use Esc to get out of things. So that I would not change atm.

I would be open to add a setting to always look at difs in this mode. Or even better if we detect there is a screen reader we could automatically go into this mode. Just an idea, feedback would be great @jvesouza @mehgcap

@jvesouza
Copy link

jvesouza commented Apr 2, 2020

@isidorn The idea of ​​automatically jumping into this mode when a screen reader is detected can be interesting, although I believe it is not a crucial thing.

@github-actions github-actions bot locked and limited conversation to collaborators May 16, 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 testplan-item
Projects
None yet
Development

No branches or pull requests

6 participants
@rebornix @isidorn @jvesouza @JacksonKearl @miguelsolorio and others