Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Dashboard a11y tests #58122
Dashboard a11y tests #58122
Changes from 12 commits
d99cb06
924fc62
06a7eea
ed518fe
f29d5b3
37b6bdb
3520d1b
c998cec
4be666b
d67877b
154a40c
1976b66
59fd20f
919221b
f9224cd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I'm not 100% sure where this is getting rendered so I wasn't able to test it but while poking around this pointed to a much larger issue (that I think this is covering up) of, why is so much of dashboard wrapped in a
aria-hidden="true"
and with a screen reader only warning saying it's not accessible?This is concerning on several levels...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, (almost?) all visualizations have
area-hidden=true
attribute. Their DOM structure is usually just a bunch of divs and an svg. Input control visualization, however, has this innerinput
field, which was focusable before and the test was failing:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean that much of dashboard is wrapped in "area-hidden=true"? If the point here is that dashboard is basically useless when all the visualizations are hidden in accessibility mode, I completely agree. I think it doesn't make much sense to show the dashboard in the first place, but I kinda assumed you and Bhavya had already discussed that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't done an a11y deep dive into the Dashboard page before so this is the first that I'm seeing it... I guess it's too large to address in this PR though. I'll focus on helping you merge this so that we can get the tests merged and I'll take a note to revisit it later. Might ping you about it later depending on how investigation goes.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment above to make focusable again when #59039 is fixed