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

Two unlabelled .monaco-status divs with role=complementary breaks accessibility compliance #2448

Closed
casey-speer opened this issue Apr 20, 2021 · 7 comments

Comments

@casey-speer
Copy link

The latest Monaco editor is triggering accessibility errors in cypress-axe: 0.12.2 + axe-core: 4.1.4, specifically the landmark-unique rule, due to there being two .monaco-status divs with role=complementary but no aria-label or aria-labelledby to distinguish them.

Distinguishing the landmarks is required for level A accessibility compliance. See, e.g.

I believe this also causes failures for earlier versions of axe-core / cypress-axe.

monaco-editor version: 0.23.0
Browser: Chrome 89.0.4389.128
OS: MacOS 10.15.7
Playground code that reproduces the issue: Run document.querySelectorAll('.monaco-status[role="complementary"]') in the browser console on the monaco or vscode. Two div appear with a role="complementary" but no aria-label to distinguish them.

@hediet hediet added the bug Issue identified by VS Code Team member as probable bug label May 11, 2021
@alexdima alexdima added this to the April 2021 milestone May 12, 2021
@alexdima
Copy link
Member

@isidorn this was added via microsoft/vscode@3edd0f9

@alexdima alexdima removed this from the April 2021 milestone May 12, 2021
@isidorn
Copy link

isidorn commented May 12, 2021

I can add the aria-label to distinguish the elements. However these labels will never be read out by the screen reader, IMHO this will make no impact on the user experience.
Due to that I will simply fix this for May and I suggest that the editor only picks this up in the next release since I think it is not critical.

@isidorn isidorn added this to the May 2021 milestone May 12, 2021
@isidorn
Copy link

isidorn commented May 25, 2021

After further pondering I will do no action here. Reason is that even if we were to add aria-labels as a way to distinguish between these element the aria labels should not be read out.
These elements are just used as a way to alert the user to something using aria-live. We have two containers so multiple equal announcments would be read out.

@casey-speer if there is an exact user scenario with a screen reader that you think could be improved with this issue please let me know and I can reopen this.

@isidorn isidorn closed this as completed May 25, 2021
@isidorn isidorn added as-designed and removed bug Issue identified by VS Code Team member as probable bug labels May 25, 2021
@isidorn isidorn removed this from the May 2021 milestone May 25, 2021
@casey-speer
Copy link
Author

After further pondering I will do no action here. Reason is that even if we were to add aria-labels as a way to distinguish between these element the aria labels should not be read out.
These elements are just used as a way to alert the user to something using aria-live. We have two containers so multiple equal announcments would be read out.

@casey-speer if there is an exact user scenario with a screen reader that you think could be improved with this issue please let me know and I can reopen this.

@isidorn Sorry I'm late getting back to you. I'm not positive how assistive technologies across the board will treat this and I see the point of not adding the label since the two divs are a workaround, but FWIW it is breaking the compliance checks.

Maybe role="status" would be appropriate but looks like there is a history of having tried that and moved to complementary for other reasons.

I'm able to work around it, but would be nice if there was a way to allow the duplicate messages etc and somehow still please axe. Perhaps we cannot have our cake and eat it too in this case 😂

@isidorn
Copy link

isidorn commented Jun 22, 2021

All screen readers treat this as we want, we tested it out.
If you find a paritcualr use case that is being broken by this please let us know. As for just the compliance checks I will do not change atm

@clintonc
Copy link

Reading microsoft/vscode@3edd0f9:

The issue discussion at microsoft/vscode#99466 seemed to consider setting aria-hidden on unused containers; can we add that? That would satisfy conformance testing and avoid unwanted labeling.

@isidorn
Copy link

isidorn commented Jan 26, 2022

@clintonc what conformance testing and unwanted labelling are you talking about? I am not sure I follow, sorry.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants