fix(accessibility): improve accessibility of all components #967
Conversation
Codecov Report
@@ Coverage Diff @@
## master #967 +/- ##
==========================================
+ Coverage 33.78% 33.84% +0.05%
==========================================
Files 79 79
Lines 3481 3475 -6
Branches 752 752
==========================================
Hits 1176 1176
+ Misses 1814 1808 -6
Partials 491 491
Continue to review full report at Codecov.
|
|
@kasperisager this is a godsend, thank you!! cant wait to give this a closer look tomorrow |
| <input | ||
| value={this.state.value} | ||
| onChange={this.handleChange} | ||
| type="text" | ||
| placeholder={this.props.placeholder} | ||
| aria-label={this.props.placeholder} |
There was a problem hiding this comment.
Why use the placeholder here? Shouldn't it be Search Input?
There was a problem hiding this comment.
It really shouldn't be the placeholder, that's for sure. Placeholders are no replacement for proper labels though they are quite often (mis)used as such, which is what I was guessing was the case here. Seeing how the placeholder was therefore really used as a label, I wanted to make sure that this label was communicated to assistive technology.
While the placeholder attribute itself should be used when computing the accessible name exposed to assistive technology, this functionality was only introduced recently and is still not widely supported: w3c/html-aam#167. I therefore opted to expose the placeholder explicitly through an aria-label.
A better way to go about this would be to add a proper label to the search input, though this would require a slightly more involved change to the design of the interface.
|
@kasperisager this looks wonderful so far! I was wondering, is there a reason why |
| @@ -43,7 +43,7 @@ function renderType(type, onClick) { | |||
| ); | |||
| } | |||
| return ( | |||
| <a className="type-name" onClick={event => onClick(type, event)}> | |||
| <a className="type-name" onClick={event => {event.preventDefault(); onClick(type, event)}} href="#"> | |||
There was a problem hiding this comment.
here, event.preventDefault() is added, whereas elsewhere it is removed, any particular reason?
There was a problem hiding this comment.
With the exception of form submission buttons, there’s no default behavior to prevent for <button> elements. For <a> elements, their default behavior is however to navigate, which we need to prevent if we want to use it as a sort of button. The TypeLink is actually a case where this is somewhat appropriate as it is used for internal links within the documentation explorer.
It is also worth pointing out that <a> elements need an href attribute for them to have any behavior. This has an effect on both keyboard operability and assistive technology as an <a> element without an href attribute is neither focusable nor communicates any semantics.
|
I've evaluated the netlify build extensively and everything looks great, looks like you resolved almost all of the non-codemirror accessibility issues! thank you for your excellent contribution @kasperisager |
|
My pleasure! 😄 |
This pull request brings a baseline of accessibility to the various GraphiQL components with a focus on making them usable by keyboard and/or screen readers. Some highlights worth mentioning:
The various sections of the interface are now implemented as regions (using
<section>elements) with proper accessible names (usingaria-label) for identifying their purpose. This is especially important for users who rely on screen readers as it's otherwise very difficult to identify and navigate the different parts of the interface, in particular the query editor and the results viewer.All components are now navigable by keyboard, with especially the history pane benefitting from this. Previously, the history pane could not be navigated by keyboard at all as interactions relied solely on mouse events (
mouseenterandmouseleave).In extension of the previous point, all interactive components are now implemented as
<button>elements, ensuring that they correctly communicate to users of assistive technology that they can be interacted with and that standard keyboard interaction works as expected. This was especially critical for the history pane components, which were previously implemented as<span>elements without any indicators that users could interact with them beyond a cursor pointer.All interactive icons now have accessible names, ensuring that their behaviour is communicated to users of assistive technology. Previously, one would have to guess what these components did when interacted with as the only thing announced were words such as "cross" for the "close" icon or "neuter" for the "search" icon.
The results viewer is now a live region, ensuring that users of screen readers are made aware of changes to results when queries are evaluated.
I'd love to hear from @cmoody if there are other critical issues not addressed in this pull request that could help solve #954.