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

a11y: Add missing aria labels on connection config component #40

Closed
wants to merge 3 commits into from

Conversation

fridgepoet
Copy link
Member

@fridgepoet fridgepoet commented Dec 29, 2021

Accessibility improvement: tracked on grafana/grafana#41206

Closes grafana/grafana#42706

Hey @sarahzinger here's a go at adding aria-labels (indirectly?) for CloudWatch. Not sure at all if that's correct. Let me know what you think.

@fridgepoet fridgepoet marked this pull request as draft December 29, 2021 13:57
@fridgepoet fridgepoet marked this pull request as ready for review December 29, 2021 14:46
@fridgepoet fridgepoet marked this pull request as draft January 4, 2022 12:17
@fridgepoet fridgepoet marked this pull request as ready for review January 4, 2022 17:41
@fridgepoet fridgepoet requested review from a team and yaelleC and removed request for a team January 6, 2022 08:48
Copy link

@yaelleC yaelleC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

Just checking before approving: any reason why Fastpass is complaining about the select elems but not the inputs (Credential Profile Name for instance)? Or maybe they were not visible when you ran it?

@fridgepoet
Copy link
Member Author

Thanks @yaelleC you're right, the voiceover is not reading the titles of those other inputs.
I think FastPass is passing them because they have placeholder attributes...
What do you recommend? Should I just add more aria-labels?

@yaelleC
Copy link

yaelleC commented Jan 7, 2022

Thanks @yaelleC you're right, the voiceover is not reading the titles of those other inputs. I think FastPass is passing them because they have placeholder attributes... What do you recommend? Should I just add more aria-labels?

Indeed! Turns out support of placeholders for screen readers is not consistent :( (see: https://www.davidmacd.com/blog/is-placeholder-accessible-label.html) So aria labels could be a good quick-fix solution and we can look into a better one is the issue you raise. 👍

@sunker sunker mentioned this pull request Jan 11, 2022
@sunker
Copy link
Collaborator

sunker commented Jan 11, 2022

I believe this PR needs to be moved to the new frontend repo for aws - https://github.com/grafana/grafana-aws-sdk-react/.

@fridgepoet
Copy link
Member Author

Thanks
These aria-labels are already present in the new repo, closing!

@fridgepoet fridgepoet closed this Jan 12, 2022
@fridgepoet fridgepoet deleted the a11y branch January 25, 2023 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessibility: Fix Cloudwatch config page fastpass issues
4 participants