Add aria hints where labels were missing or unclear #354

secretfader · 2019-05-13T03:57:00Z

While reading the source, I noticed several links that didn't seem to have descernible text. Running the site though webhint.io confirmed my hunch. In those cases, I've added the aria-label attribute, which will be used by screen reader software.

I scanned the home, releases, and crate detail pages. These changes are fairly complete, but not exhaustive. In fact, this is the first of what I hope will be several accessibility-related PRs I make to docs.rs!

templates/releases.hbs

templates/navigation_rustdoc.hbs

GuillaumeGomez · 2019-05-16T09:06:27Z

templates/navigation_rustdoc.hbs

@@ -3,7 +3,7 @@
        <div class="pure-menu pure-menu-horizontal">
          <form action="/releases/search" method="GET" class="landing-search-form-nav">
            {{#unless varsb.show_search_form}}
-            <input class="search-input-nav" name="query" tabindex="-1" type="text" placeholder="Find crate"{{#if varss.search_query}} value="{{varss.search_query}}"{{/if}}>
+            <input class="search-input-nav" name="query" aria-label="Find crate by search query" tabindex="-1" type="text" placeholder="Find crate"{{#if varss.search_query}} value="{{varss.search_query}}"{{/if}}>


The sentence here doesn't sound very nice... What about "Input to search crate"?

Users should already know this is an input, based on the tag. The label should focus on what a user can expect to achieve by submitting the form with this field filled.

templates/navigation.hbs

secretfader · 2019-05-16T12:14:13Z

Feedback has been answered. Much of this first round seemed to ignore the accessibility specification this PR aligns with, so I would encourage future reviewers to brush up on WCAG 2.0 ("Web Content Accessibility Guidelines") before sending critiques that we will ultimately have to proceed without adopting. Odds are, if I've included a change here, it's either a critical accessibility violation or the current markup is far outside accessibility best practices. In either case, it should be changed.

Critiques like "not sure it's very useful in here..." are opinions, and don't hold value in this case. Feedback that is based on evidence and increases compatibility with WCAG is fine (and encouraged, even). However, all unfounded feedback, which is published without a link to the specification we should uphold, will be ignored from this point forward.

By no means is this PR feature complete, but it does address the critical violations and best practices suggested by Webhint, which uses axe-core to scan for accessibility issues. It's a good first step towards making our shared tools accessible to all potential members of the Rust community.

GuillaumeGomez · 2019-05-16T12:17:12Z

And now you become fully insulting, awesome... You do realize that we're humans and that by no means we know everything there is to know about everything and that we do it on our free time? I expressed my opinion, you're free to contest it by using arguments, no need to become rude like you just did.

I'll let @QuietMisdreavus handle this then, I'm clearly not contributing/reviewing on my free time to get insulted on a lack of knowledge.

Manishearth · 2019-05-16T14:34:32Z

I had a chat with @GuillaumeGomez about this: he hadn't quite interpreted things correctly. I've also talked to him about improving his review style, English isn't his native language and he tends to be terse because of this.

Your feedback and help is greatly appreciated, @secretfader, sorry about the initial hostile response here. ARIA is definitely something we should try to do well here, thanks for the pull request!

Manishearth · 2019-05-16T15:14:29Z

Oh, also, to clarify a bit:

However, all unfounded feedback, which is published without a link to the specification we should uphold, will be ignored from this point forward.

This is what kinda confused people: I'm reading it as you not accepting feedback that isn't founded in the spec (which is fine and good!), @GuillaumeGomez read it as you refusing to help with the review at all unless they read the spec.

Please try to work together on this: @GuillaumeGomez if you're not sure about spec stuff, ask questions about the spec (and maybe ask for spec links to be inserted in the PR itself, wherever necessary!). @secretfader please work with @GuillaumeGomez's questions here.

Let me know directly if y'all have issues, I can be reached on the Rust discord.

secretfader · 2019-05-16T19:14:16Z

Thanks for the explanation, @Manishearth. I hoped this PR would be a simple change that enhanced accessibility, and never imagined it would become divisive.

Unfortunately, this is the kind of opposition I expect when advocating for accessibility. For context, I've had people deny accommodations for myself and others, even when I'm out in public with only one intact leg (and in a wheelchair). I'm fine with questions (like "can you elaborate on why this ARIA role is required"?), but dismissal or insinuation that a change is unnecessary is a bit more than I'm willing to tolerate. While we're on the topic of speech or concepts that are insulting, then denying disabled people the chance to use the same platforms as our abled counterparts should be unacceptable from any cultural perspective (though in some countries this discrimination is rampant and normal, a reality I reject and I hope we can all agree is worthy of disdain).

If questions are asked without hints of an expected answer, I'm happy to elaborate. I did my best to include links (both in comments and the original PR description) for reviewers who wish to do further research, and I encourage use of them.

If my words appear overly strong, it's that when speaking in technical circles, I've found that I (and frankly, all disabled advocates) must communicate with a firm and confident tone, or risk being completely ignored. Fully exhaustive responses with links are a survival tactic in a world that is actively hostile to people like me. The initial response I received was completely in line with the kinds of dismissive, marginalizing comments that people have directed at me in the past, so I treated it as such. (It is jarring to experience this the Rust community. I hope this thread underscores how and where we can continue to improve.)

If most anyone in this thread received the amount of harassment and judgement I deal with on a regular basis as a result of being disabled, I think you would also agree the content of my responses is justified. I've spent all available mental health resources on this thread, which will limit my role in future discussion. I'll now move on and let you discuss primarily amongst yourselves, with the understanding that these changes are minor but have potentially large impact for visually impaired users.

GuillaumeGomez · 2019-05-16T19:18:10Z

I wasn't against the PR, I suggested rewording because I thought it would be better. I never said that we shouldn't add aria-label which seemed like a great add.

I hope I made my position clear(er?) and I'll try not to comment here any further (I said I wouldn't but I still did...).

Manishearth · 2019-05-16T19:48:53Z

Again, to clarify, English isn't @GuillaumeGomez's first language, and they felt "Not sure it's very useful in here" was an okay way to say "can you explain why this is needed?"

there's no actual contention around the changes made in this PR, it's just a perception of contention due to a bit of a language barrier 😄

Sorry the both of you had to deal with this, I'll try to help finish review here (I'm a bit familiar with wai-aria)

Manishearth

The aria stuff looks fine to me, @QuietMisdreavus should approve the exact text

Thanks!

QuietMisdreavus · 2019-05-16T20:04:58Z

Looking at the webhint report you linked in the PR description, one of the search boxes it flagged was rustdoc's own one. I dug a little and found a nearly three-year-old issue discussing accessibility issues surrounding it. It hasn't received a comment in a year and a half though, so i'm not sure of the current situation. (This also means that existing docs on docs.rs before that's fixed upstream will not receive the fix, since we don't rebuild crates when we update rustc.)

I think the only hangup i have about the wording is that i'm not sure how a screen reader picks up the text, and what it adds around it. For example, you stating that the word "menu" is unnecessary for the platforms menu is not an assumption i would have made, since it only appears as a link in the HTML, or that the word "search" was unnecessary, since it was already an <input> element. Past that, my only suggestion would be to err on the plural side when describing things in the label. My personal assumption when writing menu text is that it's easier to use a plural word to describe something that's potentially plural than it is to use a singular word for it. There are usually more than one platform in the platforms menu, and more than one crate returned in a search, so it feels easier for me to state that.

Thanks so much for posting this! I hope you can feel up to providing more suggestions in the future. (I'm interested in updating our HTML to use HTML5 semantic elements, though i figure that's a more involved change.)

The primary navigation doesn't use <nav>, and thus requires an accessibility landmark to always be correctly identified. Additionally, several navigation links didn't include discernible text (this also applies to form fields without a visible label). In both cases, labels were added. Signed-off-by: Nicholas Young <[email protected]>

QuietMisdreavus

Coming back to this, i think this is fine to merge. Thanks for posting this, @secretfader! I appreciate that you stepped up and added these.

secretfader marked this pull request as ready for review May 13, 2019 19:06

secretfader changed the title ~~Add aria-label to links without inner text~~ Add aria hints where existing labels are missing or unclear May 13, 2019

secretfader changed the title ~~Add aria hints where existing labels are missing or unclear~~ Add aria hints where labels were missing or unclear May 13, 2019