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

Using the file compare tool with screen readers needs some attention #88695

Closed
mehgcap opened this issue Jan 15, 2020 · 16 comments
Closed

Using the file compare tool with screen readers needs some attention #88695

mehgcap opened this issue Jan 15, 2020 · 16 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues diff-editor Diff editor issues feature-request Request for new features or functionality on-testplan
Milestone

Comments

@mehgcap
Copy link

mehgcap commented Jan 15, 2020

Issue Type: Bug

The problem is, basically, that screen readers don't know that comparisons are happening. (I'm using NVDA for my screen reader.) In side-by-side mode, only the editable file can be browsed; I've not found a way to jump between it and the file showing the differences. In inline mode, NVDA simply jumps over any line indicating a change, making the mode useless.

The f7 command is the only way to get meaningful information about differences (side bug: f7 and shift-f7 appear to do the exact same thing). My main problems with f7 are:

  • There's a confusing line of text at the top of the list that has some line numbers, then two numbers indicating something? Basically, I have no idea what the top item in the list means.
  • The list includes items starting with a plus or a dash, which makes sense, but it also has a bunch of extra lines that don't appear to be different. Are these for context?
  • The list items all show the line number first. If I am comparing files I know well, I don't want to hear the line number constantly. It should be at the end, so I can hear the changed line itself first.
  • Once I find a change, I can't do anything with it. How can I revert the change so my file matches what the list item is showing? Pressing enter only moves me to the line, and there is no context menu for any list item.
  • Sometimes, I am taken out of NVDA's mode that works well with web UIs like VS Code. I am jumped to a list of diffs that is different from the list I've been describing so far. Yesterday, I had the list I've been talking about; today, when I went to confirm some things, I wound up in this new list.

I'm not sure what a good, accessible diff editor might be like. I'd love a sound indicating an addition, removal, or change, with a keystroke to update the active file with the change from A or B, or insert both (in case of a line change) then let me edit them together. Sounds would be hard with a web UI, though, and don't do anything for braille users. Maybe a custom style that a screen reader could interpret? Also, commands to jump from change to change would be great (no, the ones in the Go menu don't do anything).

VS Code version: Code 1.41.1 (26076a4, 2019-12-18T14:58:56.166Z)
OS version: Windows_NT x64 10.0.18362
Remote OS version: Linux x64 3.16.0-4-amd64

System Info
Item Value
CPUs Intel(R) Core(TM) i5-2400 CPU @ 3.10GHz (4 x 3093)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
protected_video_decode: unavailable_off
rasterization: unavailable_off
skia_renderer: disabled_off
surface_control: disabled_off
surface_synchronization: enabled_on
video_decode: enabled
viz_display_compositor: enabled_on
viz_hit_test_surface_layer: disabled_off
webgl: enabled
webgl2: unavailable_off
Load (avg) undefined
Memory (System) 15.88GB (9.53GB free)
Process Argv
Screen Reader yes
VM 0%
Item Value
Remote SSH: git.domain.com
OS Linux x64 3.16.0-4-amd64
CPUs Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz (2 x 2294)
Memory (System) 15.71GB (1.22GB free)
VM 100%
Extensions (6)
Extension Author (truncated) Version
remote-containers ms- 0.94.0
remote-ssh ms- 0.48.0
remote-ssh-edit ms- 0.48.0
remote-wsl ms- 0.41.7
vscode-remote-extensionpack ms- 0.19.0
php-debug fel 1.13.0
@isidorn isidorn self-assigned this Jan 15, 2020
@isidorn isidorn added accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues diff-editor Diff editor issues labels Jan 15, 2020
@isidorn
Copy link
Contributor

isidorn commented Jan 16, 2020

@mehgcap thanks a lot for creating this issue and for providing this fantastic feedback.
Our idea behind F7 is for it to be an accessible way to look at Diffs. While you provided valuable feedback about improving the diff view alltogether, I would first focus on fixing the issues in the F7 experience. And once we fix those we can discuss if that make F7 useful or we would need a different approach.

  1. F7 and shift + F7 doing the same -> I can reproduce
  2. I agree that the top line of text does not have enought context. I believe those are line differences but @alexdima can correct me
  3. Yeah the extra lines are for context
  4. Reading the line numbers at the end makes perfect sense
  5. For reverting a change or staging it we could introduce context menu actions. @alexdima other ideas?
  6. Being taken out of NVDA mode: I do not understand this issue, this does not sound diff view specific? @mehgcap can you maybe create a brand new github issue with steps so we try to figure out what is goind on here
  7. I do not believe we have Electron API to produce diferent sounds

This mielstone we have already planned, but I believe we might have time next milestone in February to try to address most of the issues you outlined.

@isidorn isidorn added this to the February 2020 milestone Jan 16, 2020
@mehgcap
Copy link
Author

mehgcap commented Jan 16, 2020

Thanks for the quick response.

  1. Keystrokes would be ideal, but a context menu is only one extra keypress, so works for me. It would be a great start, and may well end up being the best way to do this overall. I know nothing about Electron, so don't know if keystrokes can be altered while in the f7 menu.

  2. I can't reproduce this consistently either. All was well when I tested compare functionality on day 1, then I ran into the problem when trying to confirm some things on day 2. If I ever figure out how this happens, I'll open a new issue.

  3. That's okay. As I said, it wouldn't help braille users anyway. Plus, as you said, the f7 menu is the focus, not redoing the entire experience. In that menu, the plus and minus indicate what happened just fine.

@isidorn
Copy link
Contributor

isidorn commented Jan 17, 2020

  1. Yes, we would do a context menu action and probably a keybinding. We can customise keybindings any way we want and add them for any command - VS Code allows that
  2. Yeah a new issue would be best thanks
  3. Makes sense

We will have more details once we start workign on this in February. Thanks a lot!

@isidorn
Copy link
Contributor

isidorn commented Feb 19, 2020

Unfortunetly did not have time to tackle this feature this milestone. Thus pushing to March as I plan to look into this then.

@isidorn
Copy link
Contributor

isidorn commented Mar 19, 2020

Ok, starting to look into this, my todo list after processing the above

  • Fix F7 and Shift + F7 doing the same thing
  • Provide more context in top of the line content. Not sure what context could be helpful here @alexdima let me know if you have ideas
  • Get line numbers to be read at the end not at the start
  • Introduce commands for reverting a change or staging a command (either context menu or just actions which the user can attach keybindings to)

@mehgcap please let me know if there is more you have hit in the meantime, or if my action list does not cover something
fyi @jvesouza and @ajborka for potential ideas

@mehgcap
Copy link
Author

mehgcap commented Mar 19, 2020 via email

@alexdima
Copy link
Member

@isidorn The line at the top follows the format of patch files used e.g. when using git diff on the console. e.g. when typing a space in a file, git diff outputs the following:

$ git diff
diff --git a/src/vs/editor/common/model/textModel.ts b/src/vs/editor/common/model/textModel.ts
index 31a929f0aa..dd8ddae14a 100644
--- a/src/vs/editor/common/model/textModel.ts
+++ b/src/vs/editor/common/model/textModel.ts
@@ -1344,7 +1344,7 @@ export class TextModel extends Disposable implements model.ITextModel {
        private _doApplyEdits(rawOperations: model.ValidAnnotatedEditOperation[], computeUndoEdits: boolean): model.IValidEditOperation[] {

                const oldLineCount = this._buffer.getLineCount();
-               const result = this._buffer.applyEdits(rawOperations, this._options.trimAutoWhitespace, computeUndoEdits);
+               const result = this ._buffer.applyEdits(rawOperations, this._options.trimAutoWhitespace, computeUndoEdits);
                const newLineCount = this._buffer.getLineCount();

                const contentChanges = result.changes;

The output of git diff is the source of inspiration for this entire UI, as it was my understanding that there was no accessible git tool, and folks simply used the terminal and the git command line to do git. So my assumption was that people are familiar with the format from the git command line.

@isidorn
Copy link
Contributor

isidorn commented Mar 19, 2020

@alexdima thanks for clarification, I also found this great comment from your past self which explains pretty much everything


So based on that comment I will just make the aria label a bit richer.

@isidorn
Copy link
Contributor

isidorn commented Mar 19, 2020

Note that for now I pushed two small changes:

  • Make sure the Go to next / previosu diff work when focus is not in the editor (this would happen when command triggered via command palette)
  • Changed that we announce: + LINE_CONTENT modified line LINE_NUMBER. So we first announce a + or - simbol, than the line content and at the end the modified or the original line number. @mehgcap you can try it out tomorrow and we can fine tune of course

@mehgcap
Copy link
Author

mehgcap commented Mar 19, 2020 via email

@ajborka
Copy link

ajborka commented Mar 19, 2020

@isidorn , I am not very familiar with diffs. Can you provide a simple example to work through now and after your push goes live to insiders?

@isidorn
Copy link
Contributor

isidorn commented Mar 19, 2020

@mehgcap yeah, tomorrow insiders should have my initial changes. Tomorrow I will hopefully tackle the rest, so the Insiders from Friday should be even better.
@ajborka diffs are mostly used with Source Control Providers, for example Git. It is a convenient way for users to look at their exact changes - differences before they commit their code.
If your repository is under source control VS Code for any file you can F1 > open changes, that opens the changes view which should be navigated via F7 and Shift + F7 in an accessible way and we are looking now into improving this.
More about Git and vscode can be found here https://code.visualstudio.com/docs/editor/versioncontrol

@isidorn
Copy link
Contributor

isidorn commented Mar 20, 2020

@mehgcap I have enriched the aria label of the header to expalin a bit better what the encoding at the top means.
I have also changed that as you navigate through diffs, that the underlying selection in the modifed editor changes. Thus users can nicely use Git: Stage | Unstage | Revert Selected Ranges commands. Please note that this commands do not have a default keybinding so you would have to bind something to it. To make it more discoverable that these commands should be used I have set the ariaLabel of the top container to mention these commands. I am open for suggestions on how to further imrpove this.

@mehgcap Once I fix the navigation sometimes not navigating issue I plan to close this and it would be great if you can try this out from next week's insiders and provide feedback.

@isidorn
Copy link
Contributor

isidorn commented Mar 20, 2020

I have pushed a change which fixed "Go to Next Difference" and "Go to Prev Difference" when invoked from the command palette.
@mehgcap now the "got to next" and "go to prev" always correctly work for me. It would be great if you try out vscode insiders from next week onward when you find time and let us know if there are stil issues so we tackle them.

@alexdim fyi previosuly the diff review mode would automatically close when the origianl or the modified editor would get focused, making the whole interaction with Quick Open rather tedious (quick open refocuses the editor on close). Also this automatic hidding was probably causing confusion. Since I think it is easy to hide it via esc or hitting the X I have removed this auto hide behavior.

@cannona
Copy link

cannona commented Apr 5, 2020

This is working great for me with Jaws 2020 and the latest insiders release! Thanks for doing this @isidorn.

@isidorn
Copy link
Contributor

isidorn commented Apr 6, 2020

@cannona thanks for letting us know.
Please note that there are still some issue we are working on in this area. You can find them here
https://github.com/microsoft/vscode/issues?q=is%3Aopen+is%3Aissue+label%3Aaccessibility+label%3Adiff-editor

Feel free to provide feedback and if there are other issues you are hitting. Thanks

@github-actions github-actions bot locked and limited conversation to collaborators May 4, 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 diff-editor Diff editor issues feature-request Request for new features or functionality on-testplan
Projects
None yet
Development

No branches or pull requests

5 participants