Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add screen reader support using aria-label attribute #137

Merged
merged 7 commits into from May 21, 2016
Merged

Add screen reader support using aria-label attribute #137

merged 7 commits into from May 21, 2016

Conversation

chinchang
Copy link
Owner

@chinchang chinchang commented Apr 18, 2016

Carrying forward @jdan original PR #128 to support screen reader a11y by using aria-label attribute.

Copy from #128:

Copying some notes from my original issue comment:

You'll see that using aria-label instead of data-hint has the same results:

screen shot 2016-02-01 at 10 24 25 am

But the tooltip's contents are now accessible to screen-readers:

screen shot 2016-02-01 at 10 25 06 am

These changes follow the assumption that the tooltip is labeling the element it is annotating. This means that screen-readers will not read off the actual content of the element itself, but only its tooltip.

Consider the following:

image

In the above scenario, "Checkout" will be omitted in favor of "You will be charged $5.00" by the screen-reader.

This PR now puts in default support for aria-label attribute to define tooltip content along with the currently supported data-hint attribute. So any of the attributes will work.

Opening this PR against dev where I can finally work on the related docs and examples for this to get ready for master.

Note: Ignore the CSS files in the diff :)

jdan and others added 3 commits Mar 2, 2016
This changeset modifies `hint-core.scss` to use `aria-label` as its source so that the tooltip content can be read by screen-readers. This pattern is used in GitHub's "Primer" framework (see http://primercss.io/tooltips/)

Doing so required all base styles to be applied to the `hint--`-prefixed classes, since `data-hint` is no longer used.

To test these changes I modified `demo.html` to use the new `aria-label` pattern, then modified the "bottom-right" demo button so that its tooltip read off "Hello, world." I confirmed that the sentence was read off by a screen-reader (VoiceOver in Safari, as well as ChromeVox in chrome).

These changes come with the assumption that tooltips are in fact a label for the element they are annotating - meaning that the contents of the element will be skipped over by the screen-reader in favor of the tooltip's contents. If the tooltips are primarily used to annotate icons, this is perfectly acceptable.
Populate tooltip content using the aria-label attribute
@chinchang
Copy link
Owner Author

chinchang commented Apr 18, 2016

Hey @tfoxy, I have used a similar technique you had suggested in #81, to support both attributes. Would be great if you could review this.

@tfoxy
Copy link

tfoxy commented Apr 26, 2016

I tested it and I think this PR is great.

I would suggest mentioning aria-label in the readme, saying that this is preferred instead of data-hint, as the former has support for screen readers.

@chinchang
Copy link
Owner Author

chinchang commented May 2, 2016

@tfoxy Yeah. Will update the docs.

@chinchang chinchang self-assigned this May 2, 2016
@rossholdway
Copy link

rossholdway commented May 19, 2016

@chinchang Do you know if this will make it into the next release / when that will be!? This really would be a great feature to have!

@chinchang
Copy link
Owner Author

chinchang commented May 19, 2016

@rossholdway I am working on the final changes for this to get merged into master. This will be live in the coming release this weekend.

}
// If the `data-hint` attribute is present, use it.
// This might be deprecated in future in support of a11y.
&[data-hint]:after {
Copy link
Contributor

@jdan jdan May 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool trick @tfoxy @chinchang :)

@chinchang chinchang force-pushed the a11y branch 2 times, most recently from ca9aaed to 9317acf Compare May 21, 2016
@chinchang
Copy link
Owner Author

chinchang commented May 21, 2016

Merging into dev.

@chinchang chinchang merged commit 68d7ae0 into dev May 21, 2016
@chinchang chinchang deleted the a11y branch May 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants