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

Accessibility: Accounts and Manage items role and keyboard problems #116473

Closed
MarcoZehe opened this issue Feb 11, 2021 · 7 comments
Closed

Accessibility: Accounts and Manage items role and keyboard problems #116473

MarcoZehe opened this issue Feb 11, 2021 · 7 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes verified Verification succeeded
Milestone

Comments

@MarcoZehe
Copy link
Contributor

MarcoZehe commented Feb 11, 2021

Issue Type: Bug

  1. Open a workspace.
  2. With a screen reader on, press F6 until you land in the upper/left action bar of collapsible items such as File Explorer, Extensions etc. To the right of that, there are two items: Accounts and Manage. Accounts allows to sign in for Settings Sync, Manage opens a menu where you can go into Settings, Restart if there's an update, and some more items. There are two problems here.
    • The items should have menu button roles (role "button" with aria-haspopup="menu"). Screen readers, however, announce them as "tab selected". These are inappropriate roles for these two button type controls.
    • Keyboard focus using left and right arrows can alternate between the action bar and this mini toolbar of two items. However, tab and shift+tab treat these as two distinct containers. If they are distinct containers, left and right arrow should not move between this small toolbar and the action bar. However, this is not so critical if these are conceptually part of the same toolbar area, and tab is meant as a quick way to get to these two specifically. But it still feels a bit inconsistent.

VS Code version: Code - Insiders 1.54.0-insider (93f705a, 2021-02-11T05:44:58.419Z)
OS version: Darwin arm64 20.4.0

System Info
Item Value
CPUs undefined
GPU Status 2d_canvas: enabled
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
protected_video_decode: unavailable_off
rasterization: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
webgl: enabled
webgl2: enabled
Load (avg) 2, 3, 2
Memory (System) 16.00GB (0.15GB free)
Process Argv --crash-reporter-id 4ea43554-4a6d-4f7c-b2fb-6de1a3610d98
Screen Reader yes
VM 0%
Extensions (23)
Extension Author (truncated) Version
npm-intellisense chr 1.3.1
gitignore cod 0.6.0
dart-code Dar 3.19.2
flutter Dar 3.19.0
vscode-markdownlint Dav 0.38.0
vscode-eslint dba 2.1.14
gitlens eam 11.2.1
EditorConfig Edi 0.16.4
vscode-npm-script eg2 0.3.15
prettier-vscode esb 5.9.1
vscode-pull-request-github Git 0.23.0
beautify Hoo 1.5.0
jstl jsx 1.0.4
flutter-intl loc 1.13.0
vscode-docker ms- 1.9.1
vscode-edge-devtools ms- 1.1.3
python ms- 2021.2.529263813-dev
vscode-pylance ms- 2021.2.1
jupyter ms- 2020.12.414227025
remote-containers ms- 0.158.0
debugger-for-edge msj 1.0.15
nunjucks ron 0.3.0
vscode-webhint web 1.5.11
A/B Experiments
vsliv695:30137379
vsins829:30139715
vsliv368:30146709
vsreu685:30147344
python383:30185418
vspor879:30202332
vspor708:30202333
vspor363:30204092
vstry244:30244315
pythonvsdeb440:30224570
pythonvsded773:30223139
pythonvspyt600cf:30251589
core-portspanel:30233467
coreuntitledfile:30249963

@MarcoZehe
Copy link
Contributor Author

CC @isidorn, this is a follow-up to the fix for #106441

@isidorn isidorn self-assigned this Feb 11, 2021
@isidorn isidorn added accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues bug Issue identified by VS Code Team member as probable bug labels Feb 11, 2021
@isidorn isidorn added this to the February 2021 milestone Feb 11, 2021
@isidorn
Copy link
Contributor

isidorn commented Feb 11, 2021

@MarcoZehe thanks for feedback!

  1. This makes good sense, we can change the roles and I can look into it now
  2. I understand and I agree with your argument, however visually they are vertical one below the other. So the first toolbar is grouped at the top and the second small toolbar is grouped at the bottom. They should probably just be one toolbar, but it was just easier to implement using two. That is why we added arrow navigation between them. I understand this might not be intuitive but for now I would not change this, unless we receive more feedback similar to yours

@isidorn
Copy link
Contributor

isidorn commented Feb 11, 2021

@MarcoZehe I noticed that around VS Code we mostly use aria-haspopup=true, and not aria-haspopup=menu. Do you think we should change to set it to menu everywhere? Is there any difference in user experience?
Also I went through the elements where this aria-haspopup makes sense and I think we are already ok. Let me know if there are some other elements which could be improved.

@MarcoZehe
Copy link
Contributor Author

@MarcoZehe I noticed that around VS Code we mostly use aria-haspopup=true, and not aria-haspopup=menu. Do you think we should change to set it to menu everywhere? Is there any difference in user experience?

No, leave it on true for consistency. It is also the best supported attribute of those available to aria-haspopup, see this entry.

Also I went through the elements where this aria-haspopup makes sense and I think we are already ok. Let me know if there are some other elements which could be improved.

No, these two were the only ones I had noticed wrong roles and no menu indication where they would have been appropriate.

@isidorn
Copy link
Contributor

isidorn commented Feb 11, 2021

Thanks for the article, so let's go with true for consistency. Even though in the article the table says that true and menu should be equally supported.

@rzhao271 rzhao271 added the author-verification-requested Issues potentially verifiable by issue author label Feb 25, 2021
@isidorn
Copy link
Contributor

isidorn commented Feb 26, 2021

I believe @MarcoZehe told me in a chat that this is good now, so adding verified label and Marco can correct me if something can still be improved here

@isidorn isidorn added verified Verification succeeded on-release-notes Issue/pull request mentioned in release notes labels Feb 26, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2021
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 author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants
@MarcoZehe @isidorn @rzhao271 and others