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

[Feature Request] Accessibility Improvement / Tooltip Support #477

Closed
veeru153 opened this issue Jan 19, 2022 · 10 comments · Fixed by #485
Closed

[Feature Request] Accessibility Improvement / Tooltip Support #477

veeru153 opened this issue Jan 19, 2022 · 10 comments · Fixed by #485
Labels
⚛️ react package Lucide React Package

Comments

@veeru153
Copy link

I shifted to Lucide from Feather after having no way to add tooltips directly. A similar issue (feathericons/react-feather#67) exists for Feather which references this project but I didn't see any request or any implementation here yet. The gist of this issue/request is the ability to add alternative text like this:

<Icon alt="Navigate to Report" />

This would allow browsers to display a tooltip when you hover the cursor over the icon like this:

image

The linked issue goes into more detail. The icons work really well as buttons and this addition could improve the accessibility of the websites using these icons directly.

@ericfennis ericfennis added the ⚛️ react package Lucide React Package label Jan 19, 2022
@ericfennis
Copy link
Member

@veeru153 Thanks for opening.
Is <Icon title="Delete" /> not working mentioned in that issue?

@veeru153
Copy link
Author

Thank you for the quick response.

No this does not work. After some devtooling, the reason seems to be that when we add a title prop to the icon, it gets passed as an attribute to the SVG element like this:

<svg ... title="Delete">...</svg>

In order to render the tooltip, the text should be a Title element nested inside the SVG element like this:

<svg ...>
  <title>Delete</title>
  ...
</svg>

@ericfennis
Copy link
Member

ericfennis commented Jan 20, 2022

@veeru153 Allrighty I get it. Also read some more about it. It is also good to increase accessibility.

My proposal to accomplish this, is that we make it possible to pass children in icons. That creates also new possibilities to render something "over" the icon. This also keeps it really flexible and easy to implement.

Example title and description

<Icon>
  <title>My awesome text</title>
  <desc>More detailed description about the icon</desc>
</Icon>

Example rendering something over the icon, for example amount of items.

<ShoppingCart>
  <text>5</text>
</ShoppingCart>

@veeru153
Copy link
Author

veeru153 commented Jan 20, 2022

Ooh yes! While this is different from the traditional image alt attribute, this seems to be a better approach for the reasons you mentioned. I am assuming this would allow nesting icons inside icons too? Something like:

<Book>
  <Code />
</Book>

@ericfennis
Copy link
Member

@veeru153 Yess correct!

@ericfennis
Copy link
Member

ericfennis commented Feb 20, 2022

Wanted to release this feature, but with a final test it doesn't work as expected.
I tested it wrong before, I tested it by using the browser inspector and changed the position of the title element below the children in the DOM. That worked for some reason, the tooltip showed when hovering. A bit stupid but lesson learned, always test on initial render. So @veeru153 you were right, <title> and <desc> need to be placed before the children to make it work.

Here a test environment:
https://codesandbox.io/s/tooltip-does-not-work-5jjh2e?file=/App.svelte

TLDR:
Current implementation doesn't work, we need to have a new solution.

@ericfennis ericfennis reopened this Feb 20, 2022
@TheodorRene
Copy link

So there is currently no way to make the icons accessible to screen readers?

@tommy4st
Copy link

tommy4st commented Dec 9, 2022

So there is currently no way to make the icons accessible to screen readers?

It's actually possible by using the aria attributes. For example, you can use aria-label instead of alt or for decorative icons you can use aria-hidden. But I would still prefer to set the alt attribute.

@eloiqs
Copy link

eloiqs commented Mar 30, 2023

@tommy4st Can you share the aria-label solution you mentionned? I just came from reading this article and they don't seem to mention aria-label as an option. They mostly refer to title, desc, aria-labelledby, and role="image". I'm currious if you tested it with screen-readers?

@ericfennis I'm a bit confused about that statement:

So @veeru153 you were right, <title> and need to be placed before the children to make it work.

Currently if we pass a <title> child to an icon it gets added after the svg markup. This causes problem with screen-readers?
image

I'm also having doubts about supporting this through children. I'm not finding a reason why <title> couldn't just be included in the svgs by default. I'm thing that if you're concerned about users wanting to overwrite it, a custom title prop could be added. Of course if there's not enough bandwith on your side currently, any workaround / intermediary solution would be much appreciated!

Thanks.

@karsa-mistmere
Copy link
Member

Resolved by #2122 & (partly) #2158

@karsa-mistmere karsa-mistmere closed this as not planned Won't fix, can't repro, duplicate, stale Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚛️ react package Lucide React Package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants