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

dave-fink commented 1 year ago

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

https://github.com/adobe/aem-lib/issues/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

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

Checklist:

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

dave-fink commented 1 year ago

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

kptdobe commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 https://github.com/adobe/aem-lib/issues/18 to follow up and closing this PR.

adobe / aem-lib