2832 accessibility updates round two#2843
Conversation
| } | ||
| <h1>{strings.api_title}</h1> | ||
| <h3>{strings.api_subtitle}</h3> | ||
| <h2>{strings.api_subtitle}</h2> |
There was a problem hiding this comment.
This and the above css change were made to ensure heading levels only progress by one.
Why it matters
- Sighted users have little problem determining the structure of a page based on what they see. Screenreader users rely on assistive technology to provide the structure of a page. Most screenreaders provide the option to navigate by heading levels, which are rendered in app as a sort of table of contents.
- More in-depth explanation here
Screenshots for visual integrity
Before
After
| <a href={`${process.env.REACT_APP_API_HOST}/login`}> | ||
| <RaisedButton primary label={strings.api_login} style={{ margin: '5px 5px' }} /> | ||
| </a> | ||
| <RaisedButton primary href={`${process.env.REACT_APP_API_HOST}/login`} label={strings.api_login} style={{ margin: '5px 5px' }} /> |
There was a problem hiding this comment.
This change fixes keyboard navigation on the two buttons (Login to access API & Read the Docs).
Currently keyboard navigation will first focus on the a tag, and then on the embedded button tag, requiring 4 instead of the expected 2 keystrokes to navigate. Visual users can see that these are just actionable buttons / links, but screenreader users may be thrown off by the way keyboard navigation progresses.
The RaisedButton component can accept an href as one of its props, where it will then render an a tag using the existing styles.
Screenshot showing no visual change
| <a href="//docs.opendota.com" target="_blank" rel="noopener noreferrer"> | ||
| <RaisedButton label={strings.api_docs} style={{ margin: '5px 5px' }} /> | ||
| </a> | ||
| <RaisedButton href="//docs.opendota.com" target="_blank" rel="noopener noreferrer" label={strings.api_docs} style={{ margin: '5px 5px' }} /> |
There was a problem hiding this comment.
See above comment on keyboard navigation.
| : <div /> | ||
| } | ||
| <h3>{strings.api_header_table}</h3> | ||
| <h2>{strings.api_header_table}</h2> |
There was a problem hiding this comment.
See above comment on heading level progression.
This css change ensures the h2 tags are the same font-size as the previous h3 tags.
| <tr> | ||
| {/* eslint-disable-next-line jsx-a11y/control-has-associated-label */} | ||
| <th /> | ||
| <th aria-hidden="true"/> |
There was a problem hiding this comment.
This needs to be hidden from screenreaders as it doesn't contain any discernible text for them to read.
| } | ||
|
|
||
| @media only screen and (max-width: 960px) { | ||
| padding: 20px 25px 15px; | ||
| flex-direction: column; | ||
| @media only screen and (max-width: 960px) { | ||
| padding: 20px 25px 15px; | ||
| flex-direction: column; | ||
|
|
||
| & .links, | ||
| & .cheese { | ||
| width: 100%; | ||
| } | ||
| & .links, | ||
| & .cheese { | ||
| width: 100%; | ||
| } | ||
|
|
||
| & .cheese { | ||
| margin-top: 20px; | ||
| } | ||
| & .cheese { | ||
| margin-top: 20px; |
There was a problem hiding this comment.
This looks big, but it's just un-nesting some styles, not any changes to the actual styles.
| <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"> | ||
| <IconSteam /> | ||
| <div className="links"> |
There was a problem hiding this comment.
Similar to the above comment about un-nesting styles, this change removes the main tag from the footer.
Why it matters
- Screen readers provide their users the option to navigate by landmark. Typically, there should be one main content landmark which houses the body of the app content. Visual users don't rely semantic landmark tags to understand page layout, but screenreader users do. Currently, if a screenreader user were to navigate to the
<main>tag, they would be actually navigating to the footer. - More in-depth explanation here
Screenshots for visual integrity
Current footer
With main tag removed
| <h1>{strings.app_name}</h1> | ||
| </HeadlineDiv> | ||
| <DescriptionDiv> | ||
| {strings.app_description} | ||
| <h2>{strings.app_description}</h2> |
There was a problem hiding this comment.
Currently, screenreader users aren't able to navigate by headings or gain proper context for the homepage because no headings exist. This wraps the heading strings in proper heading levels so screenreaders can properly parse them. Computed font-sizes remain identical.
Screenshots showing visual integrity
Current homepage
With new headers
| & h1 { | ||
| all: inherit; | ||
| } | ||
|
|
There was a problem hiding this comment.
This css is added to the h1 inherits everything from its parent.
| & h2 { | ||
| all: inherit; | ||
| } | ||
|
|
There was a problem hiding this comment.
This css is added to the h2 inherits everything from its parent.
2 participants
Description
As part of an ongoing effort to improve the overall accessibility of the OpenDota web client, this pull request adds some additional accessibility fixes to various parts of the application including:
Things to know