Closed nickcernis closed 6 years ago
We can probably model the changes here: https://github.com/copyblogger/simple-social-icons/pull/23
This has been requested again.
Just wanted to update that this is being included in the 1.1 release. Is there anyone who is passionate about this that might be willing to review the change for us?
Testing reports:
Tested using NVDA ( https://www.nvaccess.org/ ) on Windows 10 and share icons for Google and Facebook still seem to be picked up by screen reader as "share". Extra text in span tag with class "screen-reader-text" doesn't seem to get picked up.
@dreamwhisper how does this compare to the simple social icons that this is modeled after?
Tested using VoiceOver on Mac. Twitter reads "Tweet" and Pinterest reads "Pin". According to initial report, the expected result would be "Share on Twitter" and "Share on Pinterest".
"Tweet" and "Pin" are clear verbs. The issue was all the buttons that just say, "share" and require a visual clue to indicate the service.
"Tweet" and "Pin" are clear verbs.
Based on the initial report and for consistency, it was not clear whether it should remain that way.
NVDA on Windows 10 does something different for me than reported by the tester. This is what I get when I tab through:
If I hover on elements, it is different (not necessarily unexpected).
@dreamwhisper thanks. I'm trying a change to use aria-labeledBy to see if it solves this for hover elements.
I did spin up NVDA on a Win10 virtual box and it was no where near as nice as the reader in OS X. For some reason when I tried turning on the reader it turned it on twice so it would start reading to me then about half way through the first line it would start echoing itself. I found it hard to hear anything past the first couple and last couple words. It happened everywhere though, so that doesn't appear to be an issue with this.
I think the aria label should work. It is working for me on the Mac. I have to push it to a production environment though, because my local doesn't work in the virtual box so I'll have to verify it in a few.
@NicktheGeek Tested the new version and it's better, but I think we can do more if you're willing to keep going on this.
With VoiceOver I now get this on focus:
Suggest that we:
Wrap the first 'Share' in an aria-hidden=true
span to stop the stuttering of 'share':
<a class="share" href="#" onclick="return false;" aria-labelledby="googleplus-before-1-label">Share<span class="screen-reader-text" id="googleplus-before-1-label">Share on googlePlus</span></a>
<a class="share" href="#" onclick="return false;" aria-labelledby="googleplus-before-1-label"><span aria-hidden="true">Share</span><span class="screen-reader-text" id="googleplus-before-1-label">Share on googlePlus</span></a>
Modify the count spans to give context of which numbers mean what, as Jen suggested:
<a class="count" href="#"><span>0</span></a>
<a class="count" href="#"><span>0</span><span class="screen-reader-text" id="stumbleupon-before-1-label-count">StumbleUpon shares</span></a>
Change “Tweet” to <span aria-hidden="true">Tweet</span>
and add <span class="screen-reader-text" id="twitter-before-1-label">Share on Twitter</span>
.
Change “Pin” to <span aria-hidden="true">Pin</span>
and add <span class="screen-reader-text" id="pinterest-before-1-label">Share on Pinterest</span>
.
Correct “linkedin” to “LinkedIn” and “stumbleupon” to “StumbleUpon”, as this affects how the words are pronounced (UK VoiceOver tries to pronounce them as one word: “stumblooopen” and “linkerdin”; camel casing fixes that to make it less jarring).
Those changes alter the VoiceOver experience to this:
That seems more consistent and easier to understand to me.
@nickcernis I did most of this. I made some changes. instead of Shares on {service}: %s
it reads %s shares on {service}
. This flows a bit better.
Also, pin
and tweet
are the correct verbs for Pinterest and Twitter respectively. As I understand it, we should not show "pin" visually and say "share" for screen reader text. The goal is to provide additional context where the on screen text fails due to requiring visual queues to provide context, e.g. "share" becomes clear because of the Facebook icon next to it.
So I've modified the reader text to make use of the correct verbs but provide some slight improvement on the function of the link. For example, "Tweet" now reads, "Tweet this" and the Pinterest count should say, "%s pins."
Since the verb/noun is descriptive of the service, it is not required to add additional descriptives like, "9 pins on Pinterest" where "shares" is vague so it is important to have "9 shares on Facebook"
I could be wrong on the nature of correct a11y support so would welcome advice from an expert but based on what I'm reading, the goal is to improve clarity while remaining consistent. Since we say "tweet" instead of "share" for Twitter we should stick with that...
Oh, and I updated how the translation works to make it easier for i18n so I think that is a win for that usage as well.
@NicktheGeek The screen reader text spans have taken the aria-hidden attribute in the latest update (ea56b50, caches cleared):
<span aria-hidden="true" class="screen-reader-text" id="googleplus-before-1-label">Share on Google Plus</span>
They're not being read as a result.
@nickcernis which reader are you using? I was getting double reads before but when I added the changes so it was hidden but kept the labeled by attribute it read it correctly for me on OS X.
VoiceOver. It doesn't currently read anything at all for me. (Just 'link', without the span text.)
@nickcernis I can remove it but then I get it to read the content to me twice. What happens if you move the aria-labelled-by to the link instead of the span?
Moving aria-labelled-by
to the link works and the text is then read.
The span that's going to be read should not need aria-hidden
, though; that's designed for elements that should not be read. For VoiceOver in Chrome I can remove it and only hear the label once.
@nickcernis issue I'm having is the element is read twice for me in Safari. I made some additional changes. I moved the labels outside the div and hid them using the hidden class. I also moved the labeled by to the link and everything seems to be working much better for me in Safari and Chrome. I need to test this in Windows. As I remember, that was a real bear and part of why certain things were done the way they were because the screen reader wasn't really behaving well in Windows but maybe these changes will work better.
Nice work, Nick — it's now working as expected in Chrome and Safari for VoiceOver, passes Chrome's a11y audit, and is keyboard-accessible for me.
Testing in Windows/NVDA seems worthwhile but otherwise confirmed.
I tested this in Windows and it works great in win10 using Chrome, Safari, and FF. Edge is a total crap fail though. I tested with NVDA and the built in accessibility support for Win10 with the same results. (well NVDA says "clickable" a whole lot)
Current behaviour
For VoiceOver users, all share buttons currently read as “share” except for twitter (which says “tweet”).
Expected behaviour
Buttons should read “share to Facebook”, “share to LinkedIn” etc. instead of just “share”.
Here's the feedback from the user who reported this: