accessibility: hyperlink with icon as content needs required icon label

[RichCaloggero]

Currently, icon as link content is announced as "link" by the screen reader.

Needs to be a required label prop or similar for this usage. THe label could be attached via aria-label which would cause the screen reader to read the value of the attribute as the link's content, ignoring the icon, which is what you want. No need for off-screen text, etc.

[fysheets]

Hi @RichCaloggero -

Sorry for the delay in response here. I have a few questions to ensure that I am understanding the issue correctly. Is this an issue with the Icon component or the Hyperlink component? Can you provide a sample code snippet of how you are using the component so that we can better understand what is trying to be achieved and we can use it as a test case when determining a potential solution?

Thank you!
Farhanah

[RichCaloggero]

Is this an issue with the Icon component or the Hyperlink component?

Not exactly sure, but it would seem the issue is with the link component.

From the demo site ( https://edx.github.io/paragon/?selectedKind=Icon ):

Here is how to make an icon with screen reader text:

<Icon id="SampleIcon" className={["fa", "fa-smile-o", "fa-4x"]} screenReaderText="Happy Icon" />

However, looking at the HyperLink demo, this is how they suggest making a link with icon content (no way to specify screen reader text here):

<Hyperlink destination="https://www.edx.org" content={<span />} />

I'd go a step further an suggest that:

• the icon component should require the screenReaderText property; icons with no screen reader text are not perceivable at all via those using screen readers

Seems like you should be able to do either

• <Hyperlink destination="https://www.edx.org" content={<Icon id="SampleIcon" className={["fa", "fa-smile-o", "fa-4x"]} screenReaderText="Happy Icon" />} />

If this is possible, should be shown on the demo site.

Again, the screenReaderText property should be required on icons, as without it, they are completely useless to screen reader users!

[fysheets]

Hi Rich,

Thank you for the feedback and catching a missed update on our end! I believe the HyperLink component was created prior to the Icon, and it looks like we didn't appropriately update the HyperLink demos to include the Icon component as you suggest in the second bullet point example. That functionality should be available and as you noted should be preferred.

I can also comment as to why the decision was made not to make the screenReaderText required on the Icon. We worked with our Accessibility Specialist at the time and decided to make that field optional in order to support the case when the Icons are used purely as decoration and provide no additional context/content for screen reader users. Much as the alt attribute should not be used or left blank for image tags when images are purely decorative.

Here is a PR to update our demo for HyperLink and update of the Icon README*, I hope this answers your questions!

*Edit: included README update for Icon as well.

[RichCaloggero]

To facilitate the case where no screen reader text is wanted, consider:

• if react can distinguish between missing props, and props whose value is explicitly the empty string, then require the attribute, and those who need null can use the empty string
• require the screenReaderText prop and add option boolean prop "decorative" or "noDescription" or something similar

[AlasdairSwan]

I agree with the decision to not make screenReaderText a required field because icons are commonly used decoratively beside text that is displayed for all users.

I think one concern is that when used in conjunction with the Hyperlink component it will be possible to have a link that is just an Icon without screen reader text. However I don't think that Paragon should be responsibile for making this impossible. I think it is enough to promote and encourage the development of accessible code, but there needs to be some responsibility from developers to use the library in a responsible manner.

[RichCaloggero]

I agree with this, however it is the responsability of the library to make it more likely the author will create accessible code. Have an explicit "noScreenReaderText" prop does not make it impossible to produce an icon with no text, it just makes the author have to think about it a bit. Most people want to "do the right thing" when it comes to accessibility; they just don't think about it because they do not need to use a screen reader etc on a daily basis. Including this prop would cause them to have to think a bit, and actively decide whether they really want a bare icon with no associated screen reader text.

[wittjeff]

I think both perspectives are valid on this point but it does not seem to be above activation threshold for a breaking change at this point. Let's revisit this for a break changes release. I'm not entirely sold then either, and will think about it in context of how heavy-handed we want to be with the rest of the library components.

[abutterworth]

The Hyperlink component now uses children instead of a prop for content so the mechanics of this issue don't apply anymore. The new implementation does not enforce the addition of screenReaderText on an Icon, but doesn't prevent it either.

I think we're okay to close this issue in a few days if no one has any concerns.