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: Mark document-like outputs with role="document" for easier screen reader navigation. #93087

Closed
MarcoZehe opened this issue Mar 20, 2020 · 17 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@MarcoZehe
Copy link
Contributor

While it is appropriate to treat large parts of VS Code as an application for screen readers, there are areas where it makes sense to suggest to screen readers to use their browse mode (AKA virtual cursor) mode navigation paradigm instead. Places I've found this to be the case are, among possibly more:

  • The Welcome page.
  • Details for an extension.
  • After an extension has been installed, the ReadMe HTML output.

All of these are places where structured HTML output is generated for pure consumption and possibly following links or initiating things like installing or uninstalling an extension, etc., but not more complex interactions.

I found this while trying out Code for the first time for real, using current 1.44 Insider builds.

@LeonarddeR
Copy link

Additional info:

  • NVDA is using an appModule to disable browse mode by default in VS Code. If we don't do this, browse mode is enabled automatically in many parts of the application where it shouldn't be active.

I would say that an application role on the body element can help a lot, as long as all documents such as extension information, welcome page and other document like content have the document role.

@isidorn
Copy link
Contributor

isidorn commented Mar 20, 2020

Thanks for filling this. Agree with your feedback. We already set the role=application on the top level element.
I wll look into setting role=document on our readonly content. Like the welcome page, extension details and other read only content we might have.

fyi @chrmarti @sandy081

@isidorn isidorn added the accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues label Mar 20, 2020
@isidorn isidorn added this to the March 2020 milestone Mar 20, 2020
@LeonarddeR
Copy link

According to the NVDA developer info, the application role is set on the div with class="monaco-workbench". However, the status bar and the monaco-alert are outside that application div.

Having said that, the document role is set on an element that is the parent of the body element, so may be this is an electron thing.

@isidorn
Copy link
Contributor

isidorn commented Mar 20, 2020

@LeonarddeR I do see that monaco-alert is outside the application div. @bpasero why did we put it out of the workbench, does it make sense to move it inside the workbench div? We create it here

However for the status bar I see it is inside the monaco-workbench div. @LeonarddeR are you sure for you it does not belong to the monaco-workbench div?

@LeonarddeR
Copy link

according to NVDA, there is a monaco-status in the monaco-aria-container, sibling of the alert container. This is based on what NVDA gets from IA2, though.

@bpasero
Copy link
Member

bpasero commented Mar 20, 2020

I do see that monaco-alert is outside the application div.

No specific reason as far as I can remember.

@isidorn
Copy link
Contributor

isidorn commented Mar 25, 2020

I have pushed two changes:

  1. monaco-alert is now inside the role=application div
  2. I have changed the welcome page and the extension details page to use role=document

@MarcoZehe @jvesouza @ajborka please let me know if there are other places in VS Code where the role=document would be more fitting?

  1. How about for editors which are read only? Is the current role textbox fitting?
  2. How about for Outputs? For example F1 > Show Git Output
  3. How about for Terminal which uses the rool toolbar, should we use a more correct role there? F1 > toggle integrated termianl to get it.

@isidorn isidorn added feature-request Request for new features or functionality verification-needed Verification of issue is requested labels Mar 25, 2020
@MarcoZehe
Copy link
Contributor Author

@isidorn What about the display of Readme files from installed extensions? These are HTML-ified .md files AFAIK, and the role =„document“ would be appropriate there, too.

As for the terminal, the input is a textbox, the output is definitely not a toolbar. A toolbar is a container for buttons. The output is either a read-only textbox or, if links are provided etc., a document role would also be appropriate here, as this would then kind of act like an HTML document.

@isidorn
Copy link
Contributor

isidorn commented Mar 25, 2020

@MarcoZehe from installed extensions are markdown preview editors. For those I am in discussion with @mattbierner so we change the role of markdown preview to document.

As for the terminal it is not a toolbar I misread the html element. It currently does not have a role.
Though I see that the each element in the terminal has a listitem role. @Tyriar should we then assign a role of list to the terminal container element?

Output is currently textbox, is there an aria way to mark a textbox readonly? @MarcoZehe

@ajborka
Copy link

ajborka commented Mar 25, 2020

@LeonarddeR your concept of turning off browse mode on the entire vscode application might have a flaw in it. When I start NVDA, then start vscode insiders 1.44, the explorer window now automatically turns off browse mode. I would like it to remain in browse mode because navigation is easier. I shouldn't be required to turn forms mode on whenever vscode starts. All you need to do as a user is create a custom profile for vscode, then in browse mode under settings, tell browse mode to remain turned off. @isidorn it might be difficult navigating readonly text boxes with Orca. Right now, an Orca user has to press F7 in order to read readonly edit fields. I think document role would work in Windows, but not sure how it will work in Linux. Also, the markdown previews should have the document role.

@LeonarddeR
Copy link

@ajborka the vs code app module will be considered if we have a viable solution. I will test Insiders tomorrow to see how it behaves and will give appropriate feedback.

@MarcoZehe
Copy link
Contributor Author

@MarcoZehe from installed extensions are markdown preview editors. For those I am in discussion with @mattbierner so we change the role of markdown preview to document.

That sounds right, since the preview is always going to show the result in HTML, and that is always easier if the screen reader applies the web reading techniques rather than the application interaction model.

As for the terminal it is not a toolbar I misread the html element. It currently does not have a role.

Yes, I tested that briefly and found that only the buttons are in a tool bar, which is fine.

Though I see that the each element in the terminal has a listitem role. @Tyriar should we then assign a role of list to the terminal container element?

The direct container of all these list items should be a list. There is another div that gets focus when you shift-tab twice from the input textbox. The first tabstop is a div with class focus-trap or so directly preceding the input. The second shift-tab, a div container to all the output items, is what I would consider the candidate for role="document".

Output is currently textbox, is there an aria way to mark a textbox readonly? @MarcoZehe

Yeah, but that's probably not what we want, not if we also linkify stuff in there. And it would mean we'd still need arrow/caret navigation, which a read-only div doesn't give us. So instead of a textbox for the output, as stated above, document is more appropriate.

@MarcoZehe
Copy link
Contributor Author

@LeonarddeR your concept of turning off browse mode on the entire vscode application might have a flaw in it. When I start NVDA, then start vscode insiders 1.44, the explorer window now automatically turns off browse mode. I would like it to remain in browse mode because navigation is easier.

Question, how is file explorer easier in browse mode, AKA with the virtual cursor? It is a tree view, and a set of buttons related to the selected file or folder, and a tree view would require focus/forms mode anyway. I'd say the global role="application" as is already the case is exactly what we want, with some selected bits getting the document role as this issue originally discussed. Tomorrow's insider build will have @isidorn's changes, so we can see how they affect things. I'm almost certain this will be positive.

@ajborka
Copy link

ajborka commented Mar 25, 2020 via email

@ajborka
Copy link

ajborka commented Mar 25, 2020 via email

@MarcoZehe
Copy link
Contributor Author

I can confirm the document role being present on the welcome page and extension details outputs in the latest insider build. This issue can be marked as verified. We should maybe file separate issues for more items that benefit from role "document" when we come across them. Now we can go and fix the VS Code app modules in NVDA, @LeonarddeR .

@isidorn
Copy link
Contributor

isidorn commented Mar 26, 2020

@MarcoZehe Sounds good, thanks for letting us know. Adding verified label here.
Matt already opened a PR for adding document label for markdown preview docuemnts #93435

For using the list role in the terminal I have opened this issue #93480
For using the document role in the output I have created this issue #93481

For all othere element please create separate issues.
Thank you very much

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 feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants