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
Fix accessibility issue in Quick starts page-Buttons must have discernible text #9339
Fix accessibility issue in Quick starts page-Buttons must have discernible text #9339
Conversation
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.
If this page now passes axe, can we add it to the pages we test a11y for in cypress? If not, how hard is it to address the remaining issue?
The easiest way to add a11y test coverage is to add this page to the other routes test here:
@@ -55,6 +55,7 @@ const QuickStartTileDescription: React.FC<QuickStartTileDescriptionProps> = ({ | |||
e.preventDefault(); | |||
e.stopPropagation(); | |||
}} | |||
aria-label={t('quickstart~Prerequisites icon')} |
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.
Imagine a screen reader telling you that you've navigated to a button and then it tells you "Prerequisites icon". Do you know what the button does now?
An aria-label is just as important as visible text. Button text should be actionable.
aria-label={t('quickstart~Prerequisites icon')} | |
aria-label={t('quickstart~Show prerequisites')} |
Don't forget to regenerate the locale files.
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.
Thanks for explaining, I've updated the pr
…e discernible text
48cab9d
to
e060273
Compare
/retest |
fyi @jschuler |
@spadgett Critical issues from the Quick Starts catalog page has been removed by this pr and I can see one moderate issue remaining |
/lgtm I think we can further improve this with other aria attributes to indicate a popup but for now this addresses the immediate concern. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: christianvogt, sanketpathak The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes:
https://issues.redhat.com/browse/ODC-6065
Analysis / Root cause:
Getting Critical Accessibility violation when scaning the Quick Starts page using axe tool
Solution Description:
Added aria-label to the prerequisite button on quick start tile
Screen shots / Gifs for design review:
UI is unchanged
Before:
After:
Unit test coverage report:
Unchanged
Test setup:
Go to Quick Starts catalog page and scan page using axe devtools in developer console
Browser conformance: