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

Issue 13 - SVG icons adding aria-label to improve lighthouse score #14

Conversation

dave-fink
Copy link

@dave-fink dave-fink commented Sep 15, 2023

When an author links a :svg-icon: (common use case for the brand logo in the header), the resulting HTML is an A tag without textual content or an aria-label, which incurs a lighthouse accessibility ding.

Modified the decorateIcons() function to add an aria-lable attribute with the icon name to the parent A tag when it exist.

Here's an example page - https://svg-icon-link-issue--franklin-demo--dave-fink.hlx.live/icons-link-issue

13

#13

Motivation and Context

Fixes accessibility issue impacting lighthouse score

How Has This Been Tested?

Add a SVG :icon-name: to a document and then hyperlink it. Preview the page and run a lighthouse scan with accessibility checked. You will see that there is an issue present.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@dave-fink
Copy link
Author

I have signed the Adobe CLA document - I'm not sure why it's not passing.

@kptdobe
Copy link
Contributor

kptdobe commented Sep 19, 2023

Thanks a lot @dave-fink, this is great. We just have a timing collision, the decorateIcons function has been discussed a lot recently because we are observing tons of issues. Consequence is that it is being re-implemented completely. This is still being "finalised" (see #15) and your PR will become soon obsolete.
But I definitively see the need for accessibility improvements and ideally, your PR could be updated to fix the issue in the context of the new implementation.

@dave-fink
Copy link
Author

@kptdobe Thank you Alexander for the update - let me know if I can be of any help once the new implementation is ready. Shall I close this PR?

@kptdobe kptdobe changed the title Issue 13 - SVG icons adding aria-lable to improve lighthouse score Issue 13 - SVG icons adding aria-label to improve lighthouse score Sep 27, 2023
@kptdobe
Copy link
Contributor

kptdobe commented Sep 27, 2023

Thanks a lot @dave-fink for raising this issue.

The new implementation is in and it invalides this PR. But... it might be worth now in terms of accessibility since it creates an image with the icon but does not add any title nor alt text. Opened #18 to follow up and closing this PR.

@kptdobe kptdobe closed this Sep 27, 2023
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.

2 participants