I’m not sure where to put the names, as it’s currently like this:
<li class="notifications">
<a class="icon" href="#" data-ember-action="2" data-notifications="notifications-dropdown" id="user-notifications" title="notifications of @name mentions, replies to your posts and topics, private messages, etc"><i class="fa fa-comment"></i></a>
</li>
note: adding title attributes to links is not helpful and may result in duplication of information for some users. See Using the HTML title attribute for details.
Hi folks!
As @stevefaulkner said, using the title attribute is not desirable for many reasons for screen reader users. In the example of the notifications, putting a concise text bit in either an aria-label attribute on the href element or as a child of that in the form of a span with text that is positioned off-screen so only screen readers can read it, would be much much better.
The same goes, for example, for all the buttons except the Reply button to any post. So the Like, Bookmark, etc. buttons below each post don’t get their function spoken by screen readers automatically, either. Only the title attribute conveys some information, but you usually have to press an extra key to read that. As the article linked by Steve points out: The title attribute has been abused so badly over the years that screen readers and browsers simply don’t rely too much on it any more when calculating what we call the Accessible Name of any element. So alt, aria-label or using hidden text are the preferred ways to name elements.
To see how hidden text can be accomplished, see, for example, here.
I would really like to see this improved in Discourse, since this is one of the major pain points I have myself. However, I do know nothing about Ruby and thus would have a very steep learning curve when trying to fix this bug. However, if someone else volunteers to fix the bug, I will be happy to review the proposed change before a PR is merged. I can see from looking at code if it does the right things (most of the time), I only cannot implement this myself.
I really want to make some progress here, I just added aria-label to the buttons in the toolbar, does that improve things.
The “labels” feel a bit long to me, should we have different labels to the titles or perhaps just shorten both.
@codinghorror to be honest I don’t think “share a link to this post” is really any better than “share a link” , a lot of pointless words in the button titles.
@MarcoZehe ideally I would like to avoid the hidden text trick if possible, it may have some performance concerns as it introduces a bunch of workaround rules.
@sam, shortening some of the button labels would make sense. For example: “Begin composing a reply to this post” is quite long. “Reply to this post” would be totally sufficient. The important thing is to keep the “this post” part, since there is also an over-all general reply button at the bottom that adds a new reply, but doesn’t specifically link to any of the other replies.
In general, the labels, what’s put in aria-label, should be concise and to the point. Title can always be used to augment that, if the concise label leaves any doubt as to what the function of the control is.
Using aria-label is fine, the suggestion to use hidden text was merely an exit route in case you didn’t want to use aria attributes for some reason. All the browsers you cite in the requirements support this attribute, so do modern or semi-modern versions of common screen readers.
HTH!
Marco
Thanks heaps, if you can give me any more specific examples of spots you would like fixed (and aria-labels added) let me know.
I kind of oppose this, as that statement is meant to help people learn that they can compose while they read, e.g. the composer does not block reading, or even navigating to new topics with the editor open. “Reply to this post” implies that is all you can do, e.g. it is a modal reply form that whisks you away to a dialog or a different page.
Perhaps better is “Show the post editor”?
“Begin replying to this post” ?
“Show the post editor” is omitting the fact it is also attaching current post to the editor.
@sam, I agree with this. And I like this new label suggestion. And while we’re at it, I wasn’t aware that you could even switch to a different thread and still keep the current post in the editor open. So my suggestion was indeed not totally appropriate.
Oh yes, I just discovered that all items in the editor like inserting links and stuff have the same problem. I’m still in the beginnings of learning Discourse, so there may be more over time I stumble upon, but this is one that I just found while looking around the actual text field.
@sam, found another: Hit the Share This Post button below any post. The links following the text field which contains the link, they all only have the topic title as the title attribute set. And no other text or aria-label. A screen reader user does not know which link is for which social network or other sharing means.
I added aria-labels to both sharing links and composer, it should be deployed here in the next 10 minutes.
@MarcoZehe can you confirm it looks good?
https://github.com/discourse/discourse/commit/4191d97d48e2451ff583ed808dee081544a34d14
https://github.com/discourse/discourse/commit/4f53b85ee2bf293af6ff3e9f67f83f02700cccbe
Niiiiice! The labels for the composer buttons also contain the keyboard shortcuts! This is marvelous!
Just one nit: The Close button in the Sharing panel is the only one I found that is still missing the label. But all the other sharing options now speak properly, too! Great stuff! 
With these enhancements in the past few days, the major functionality of Discourse became much more accessible! I will probably run into other little things as I continue to use various instances of Discourse, and will report them ASAP. But if all these are so easy to fix, we should be in good shape! 
Marco
Out of interest: When will these changes be deployed elsewhere? E. g. when will self-hosted Discourse instances get these enhancements? I assume you are in control over the ones you run/sell, but am wondering about others, too.
I added an aria-label here too. The timing of these fixes is perfect because I was just about to fix these for someone who uses a screen reader.
Self-hosted instances that follow the tests-passed branch will get these fixes when they upgrade.
Here’s another enhancement that would immensely increase navigability for screen reader users. Screen readers render pages in what we call a virtual document. Basically multi-column layouts are reformatted into a logical order, mostly following the DOM order of elements. Other semantic information is exposed such as lists, tables, headings etc. We also have navigation keys available that are not browser shortcuts, but shortcuts specific to screen readers. One of them allows us to move quickly from heading to heading.
In Discourse, there is a H1 at the start of the topic, but no subsequent headings for each post. There are other headings strewn about, like for “suggested topics”, which are H3, and some links like to Github commits, which are H4. It would be very very very helpful if at the start of each post within a thread, there was a heading 2 to jump to so each post start can be found very easily. The author name is a possible candidate to be enclosed in such a h2 tag. It’s already there as a link, and that link would just have to be wrapped. Alternatively, text could be added that would contain text such as “post #5”. Not sure how that fits in the current design, but semantically, headings for each post within a thread make a lot of sense I think.
Oh, a friend found another unlabeled button. The one that shows or hides the topic details. To make this really cool, a contextual aria-label would be fantastic. One that says “Show topic details” when they’re hidden, and “Hide topic details” when they’re showing. 
Alternatively, if there can only be one label, which I don’t suppose since the showing/hiding needs to be done anyway, aria-expanded=“true” if the details are showing, and aria-expanded=“false” when they’re hidden, placed on the button, would also be a way to indicate the state. But the contextual label is definitely preferred.
Hm, I just tried to do this, and it seems that bind-attr doesn’t recognize aria-label, or something of the sort. Any ideas @eviltrout?