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

Font Awesome <svg> arrow a11y #1320

Closed
4 of 5 tasks
inspector508 opened this issue Mar 7, 2020 · 7 comments · Fixed by #3556
Closed
4 of 5 tasks

Font Awesome <svg> arrow a11y #1320

inspector508 opened this issue Mar 7, 2020 · 7 comments · Fixed by #3556
Assignees
Labels
a11y issues that need to account for accessibility concerns uids_base

Comments

@inspector508
Copy link

inspector508 commented Mar 7, 2020

arrow should use role "presentation", not "img". Assigning "img" role triggers ALT requirement, but ALT text is unnecessary as the purpose and content of the link are satisfied by the child text node (e.g., "SOFA"). Note: this is probably a better solution than FontAwesome in this use case.

FontAwesome recently updated to to not strip out role="presentation" from the SVG icon that is rendered. FortAwesome/Font-Awesome#14791

Proposed Solution

  • Make sure our fontawesome dependency is up to date
  • Audit our existing usage to make sure that classes don't break with the update
  • Updated our markup to include role="presentation" and test to make sure it is not longer stripped out.

Follow-up

  • Create a WYSIWYG Filter plugin that adds role presentation attribute to spans with the fa class.
  • Update filtered_html and minimal to allow role attribute on span
@richardbporter richardbporter removed their assignment Mar 26, 2020
@bspeare bspeare changed the title uiowa.dev.drupal.uiowa.edu <svg> arrow Font Awesome <svg> arrow a11y Apr 2, 2020
@bspeare bspeare transferred this issue from uiowa/uids Apr 2, 2020
@bspeare bspeare added uiowa.edu Issues related to the homepage. a11y issues that need to account for accessibility concerns labels Apr 2, 2020
@joewhitsitt
Copy link
Contributor

Currently, the role cannot be controlled with the markup we use to implement the icons. This is hardcoded with fontawesome svgs. There is an issue to work on this: FortAwesome/Font-Awesome#14791

Since there is no guarantee that the solution will be implemented/controlled as the example in the issue suggests, I think we should hold off on this issue for now.

@joewhitsitt joewhitsitt added the blocked Blocked by another issue label Jun 9, 2020
@joewhitsitt
Copy link
Contributor

This is showing up in SiteImprove results

@joewhitsitt joewhitsitt added uids_base and removed uiowa.edu Issues related to the homepage. labels Jun 16, 2020
@joewhitsitt
Copy link
Contributor

Maybe consider switching from the svg implementation of fontawesome in order to try and control the markup more.

@joewhitsitt
Copy link
Contributor

We can upgrade to 5.8.2 and update our implementations to override the default:

FortAwesome/Font-Awesome#14791 (comment)

@joewhitsitt joewhitsitt removed the blocked Blocked by another issue label Jan 29, 2021
@joewhitsitt
Copy link
Contributor

Oops. Looks like we already have this but need to update our markup
#2991

@pyrello
Copy link
Contributor

pyrello commented May 11, 2021

Suggested method of resolution: Add an icon.html.twig file that takes in the icon class as an argument an replace all the instances of the icon in other templates with that template.

@joewhitsitt joewhitsitt self-assigned this May 21, 2021
@joewhitsitt
Copy link
Contributor

curious @pyrello what the icon template would look like. trying to understand if the juice is worth the squeeze if the majority of the markup is specifying the classes. I see the benefit of making mass changes, but I doubt this would change much.

<span role="presentation" class="fas fa-user"></span>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y issues that need to account for accessibility concerns uids_base
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants