2832 add aria labels to header elements#2834
Conversation
|
|
||
| const AppLogo = ({ size, strings, onClick }) => ( | ||
| <StyledLink to="/" onClick={onClick}> | ||
| <StyledLink aria-label="Go to the Open Dota homepage" to="/" onClick={onClick}> |
There was a problem hiding this comment.
This is needed to provide clarity to screen reader users. Currently because css is used to transform the text to uppercase, screen readers spell out the link as Less than, O-P-E-N-D-O-T-A backslash greater than. With this aria label screen readers will now provide more accurate context to users who utilize them.
VoiceOver
| <form onSubmit={this.formSubmit} style={{ width: small ? '280px' : 'auto' }}> | ||
| <TextField | ||
| id="searchField" | ||
| aria-label={strings.search_title} |
There was a problem hiding this comment.
Visual users have the hint text to go off of, but because this field lacks an associated label, screen reader users currently only hear edit text blank. With this aria label screen reader users now have the same context visual users have.
Current
With fix
|
Added updates to sponsor links and app download links. |
micahchiang
left a comment
micahchiang
left a comment
There was a problem hiding this comment.
Added a few more a11y updates that don't include any visual changes.
| style={{ position: 'relative', left: '13px', top: '12px' }} | ||
| > | ||
| <img src="/assets/images/google_play_store.png" alt="" height="46px" /> | ||
| <img src="/assets/images/google_play_store.png" alt="download the android app on google play store" height="46px" /> |
There was a problem hiding this comment.
Functional links should meet W3 accessibility standards by providing alt text that accurately describes what the link does.
Current behavior
With updated alt text
| style={{ position: 'relative', left: '20px', top: '5px' }} | ||
| > | ||
| <img src="/assets/images/apple_app_store.png" alt="" height="31px" /> | ||
| <img src="/assets/images/apple_app_store.png" alt="download the iOS app on the app store" height="31px" /> |
There was a problem hiding this comment.
Functional links should meet W3 accessibility standards by providing alt text that accurately describes what the link does.
Current behavior
With updated alt text
| <span id="app-description">{strings.app_description}</span> | ||
| {' - '} | ||
| {strings.app_powered_by} | ||
| <a href="http://steampowered.com" target="_blank" rel="noopener noreferrer"> | ||
| <span id="app-powered-by">{strings.app_powered_by}</span> | ||
| <a href="http://steampowered.com" aria-describedby="app-description app-powered-by" target="_blank" rel="noopener noreferrer"> |
There was a problem hiding this comment.
Links should include proper context for screen reader users. Visual users can see the description, but currently screen reader users only hear http://steampowered.com. Adding aria-describedby adds context for screen readers.
| <span id="bg-image-description">{strings.home_background_by}</span> | ||
| <a | ||
| href="//www.artstation.com/artist/mikeazevedo" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| aria-describedby="bg-image-description" | ||
| aria-label="Mike Azevedo on artstation.com" | ||
| > Mike Azevedo | ||
| </a> |
There was a problem hiding this comment.
Even though screen reader users may not experience the background image the same way visual users do, because there is a focusable link to the artist's profile, more context should be provided as to why a screen reader is dictating the link to the user. Currently screen readers will only read Mike Azevedo, leaving it up to the user to try and understand the meaning of the link. Adding an aria-describedby provides a little more meaning to screen reader users.
|
I've fixed most of the "serious" issues that aXe has flagged on the home page. The remaining ones can be left in my opinion, as screen readers provide accurate context for those links:
|
| <img src="/assets/images/vp-logo.png" alt="VP Game home" /> | ||
| </a> | ||
| <a | ||
| href="https://www.openai.com/" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| <img src="/assets/images/openai-logo.png" alt="" /> | ||
| <img src="/assets/images/openai-logo.png" alt="Open AI home" /> |
There was a problem hiding this comment.
Without the alt text, screen readers will read off the href very quickly, which may be challenging for users with cognitive disabilities. Providing alt text that reads a little more semantically helps reduce cognitive overhead.
|
Thanks for improving accessibility for our users! |
|
No problem, the pleasure is mine. I'll continue to do a sweep through the different parts of the web application and submit additional PRs where fixes are warranted. I'll do my best to leave the existing UI unaffected. |
2 participants
Description
This pull request is in reference to this open issue regarding accessibility improvements. Specifically this pull request adds aria labels to several key elements in the OpenDota web application header. It's important to note that this pull request doesn't have any visual impact on the existing UI. Comments are left on each update describing why it was necessary.
This pull request fixes 3 site-wide critical accessibility issues detected by aXe.
Existing critical issues
With fixes, 0 remaining critical issues
Edit: I've gone ahead and included several more updates in the sponsor and footer sections of the homepage, since they were simple to add.