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

Compact explorer tree nodes don't narrate "Expanded" and "Collapsed" #86414

Closed
alexr00 opened this issue Dec 5, 2019 · 10 comments
Closed

Compact explorer tree nodes don't narrate "Expanded" and "Collapsed" #86414

alexr00 opened this issue Dec 5, 2019 · 10 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues bug Issue identified by VS Code Team member as probable bug file-explorer Explorer widget issues on-release-notes Issue/pull request mentioned in release notes verified Verification succeeded
Milestone

Comments

@alexr00
Copy link
Member

alexr00 commented Dec 5, 2019

Using NVDA to test #85039: clicking on any of the compact items expands and collapses the node. However, it doesn't read "Expanded" and "Collapsed" the way non-compact nodes do.
Reads "Expanded" and "Collapsed" when I expand and collapse, respectively.
noncompactnode
Does not read "Expanded" and "Collapsed" when I expand and collapse, respectively.
compactnode

@isidorn
Copy link
Contributor

isidorn commented Dec 5, 2019

Good feedback. Thanks. Assigning to december to investigate.

@isidorn isidorn added accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues bug Issue identified by VS Code Team member as probable bug file-explorer Explorer widget issues labels Dec 5, 2019
@isidorn isidorn modified the milestones: November 2019, December 2019 Dec 5, 2019
@isidorn
Copy link
Contributor

isidorn commented Dec 12, 2019

We do not set all the needed aria properties on the subnodes. One of them being the expanded and collapsed state.
So idealy we would set the aria-expanded somewhere here
Missing issue: currently the navigationController has no event to listen to update the expanded flag.
I do not think it is a big issue, thus assigning to backlog candidates until users complain.

@davidackroyd99
Copy link

Hi, please could I have this issue? Saw this ad on Reddit and thought I should get involved!

@isidorn
Copy link
Contributor

isidorn commented Mar 30, 2020

@davidackroyd99 this issue is not so trivial, however if you would like to take a stab feel free. Once you have something working ping me on the PR. How the tree sets the activedescendent for the compact subnodes might be a good example how this could potentialy be done.

@davidackroyd99
Copy link

cheers :) will let you know when I have something!

@Neurrone
Copy link

Neurrone commented May 9, 2020

Pending a proper fix, could explorer.compactFolders default to false if not otherwise set if screen reader usage is detected? It causes some difficulty when trying to navigate in the explorer view.

@isidorn
Copy link
Contributor

isidorn commented May 11, 2020

@Neurrone let's assign to next milestone so we look into a proper fix. Thanks for bringing this up.
If we do not fix it, we can disable the compactFolders for screen readers.

@isidorn isidorn modified the milestones: Backlog, June 2020 May 11, 2020
@isidorn
Copy link
Contributor

isidorn commented Jun 18, 2020

Pushed a fix for this such that we now properly set the aria-expanded and aria-level property on the compact nodes. I tested it and seems to work fine for me.
@Neurrone try it out and let us know what you think. Thanks!

@alexr00
Copy link
Member Author

alexr00 commented Jul 2, 2020

There's still something weird happening here. I get an extra "collapsed" when I click on the rightmost folder to expand it. I just expanded it, should it read collapsed like that (it doesn't for non-compact nodes).
recording (6)
narrates:
level 6 classification not selected collapsed
expanded

Still, it is much better, so it could still be considered verified.

@alexr00 alexr00 reopened this Jul 2, 2020
@alexr00 alexr00 added the verification-found Issue verification failed label Jul 2, 2020
@isidorn
Copy link
Contributor

isidorn commented Jul 2, 2020

Thanks @alexr00 for looking into this. I just checked the corner case you mentioned and I can indeed reproduce this. However since screen readers users do not use the mouse, and this issue can not be reproduced with the keyboard I am fine living with this.
Thus closing this and adding verified label as you said that it can be considered verified.

@isidorn isidorn closed this as completed Jul 2, 2020
@isidorn isidorn added verified Verification succeeded on-release-notes Issue/pull request mentioned in release notes and removed verification-found Issue verification failed labels Jul 2, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Aug 18, 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 bug Issue identified by VS Code Team member as probable bug file-explorer Explorer widget issues on-release-notes Issue/pull request mentioned in release notes verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants
@joaomoreno @isidorn @Neurrone @davidackroyd99 @alexr00 and others