danawoodman / react-fontawesome

A React Font Awesome component.
https://www.npmjs.com/package/react-fontawesome
MIT License
668 stars 72 forks source link

[#61] Fixes aria-label Usage #62

Closed JustinNusca closed 6 years ago

JustinNusca commented 6 years ago

Related Issue

61 — AriaLabel Prop is Effectively Broken

Overview

This PR fixes usage of the ariaLabel prop, which previously provided labels that were hidden behind a parent element with the aria-hidden attribute, and were inaccessible to screenreaders as a result.

I removed the hidden span in favour of using an actual aria-label attribute on the parent element, and toggling the presence of the aria-hidden attribute based on the presence of the ariaLabel prop. As screen readers should announce an element’s aria-label instead of its text content, it should be safe to remove the aria-hidden attribute here.

I preserved the existing behaviour of adding the aria-hidden attr on the parent element where possible (ie. in the absence of an ariaLabel prop), since it seems like a sensible default. It provides a baseline level of improved accessibility, and is otherwise an attribute that many developers are likely to overlook (and when they don’t, they are likely to set it to true, anyway) — but I'm not opposed to also making this configurable via a prop, as well.

Changes

danawoodman commented 6 years ago

Sorry for the long delay, I was out of the country for a while! Thanks for the contrib 🤘