fix(a11y): do not assign aria-labelledby attribute to icons unless there is an associated title

[stern-shawn]

See docs team ticket for context.

It looks like Paste Icons assign an aria-labelledby attribute to all icons, even if there is no <title /> element that can be referenced by the matching titleId. This is throwing warnings in the 'wave' a11y tool mentioned in our QA ticket for the Twilio Docs but you can see the same issue in the Paste docs or anything else that uses Paste Icons and/or the Button with link functionality (which renders a button containing a decorative arrow icon).

My fix is to only assign this aria attribute if there will be a valid <title /> element to reference, avoiding the a11y error/warning.

Without this change (a page with lots of Buttons as links, which render an <ArrowForwardIcon decorative /> internally:

With this change made directly to the Paste icons source in node_modules/:

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

• I acknowledge that all my contributions will be made under the project's license.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[changeset-bot]

🦋 Changeset detected

Latest commit: fc171e9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@twilio-paste/icons	Patch
@twilio-paste/website	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

@stern-shawn is attempting to deploy a commit to the Twilio Team on Vercel.

A member of the Team first needs to authorize it.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit fc171e9:

Sandbox	Source
@twilio-paste/nextjs-template	Configuration
@twilio-paste/token-contrast-checker	Configuration

[serifluous]

Hey @Sstern ! @nkrantz looked into this yesterday and we add aria-hidden to all decorative icons, meaning they're not visible to screen readers since the content of the icon should be duplicated by its context.

On the WAVE site, they explain this a bit more:

How does WAVE handle "hidden" elements?

WAVE detects accessibility issues in all page elements, even those that are hidden using CSS, the hidden attribute, aria-hidden="true", and/or tabindex="-1". This is by design. In most cases, hidden elements are revealed to users at some point. We believe it's important that these potential end user accessibility issues in drop-down menus, tab panels, dialog boxes, carousels, and other "hidden" elements be identified.

In most cases the WAVE interface will provide an indication of icons that are relevant to hidden elements—the icon in the sidebar will be slightly dimmed and will present an informative tooltip. These elements may be examined by turning off the Styles option in the WAVE sidebar.

If WAVE has detected an error in an element that is hidden and that is never presented to the end user, or that is perhaps modified to address the error before being presented (something WAVE cannot detect), the WAVE error can be ignored - or you may instead choose to adapt the code to ensure the element is accessible in its hidden state.

In short, if we err on this point, we err on the side of users with disabilities. In other words, we have chosen to identify issues in hidden content that may be presented to users, rather than suppress these errors in cases where advanced development techniques have been implemented to potentially hide that content from users.

^ Let us know if ignoring the error works for you like they suggest. If not, we'll do a bit more digging into the best way forward.

[stern-shawn]

That sounds good to me! I had noticed that aria-hidden was being applied and was surprised that WAVE was flagging the elements anyway. Instead of looking at their docs I assumed there was a code fix (which there is via this change, but this change isn't necessary if the error is ignorable 😄 ).

If this is something you'd like to pursue, I'm happy to tweak this PR as needed, otherwise I can report this back to my team and close this!