ambassify / react-avatar

Universal avatar makes it possible to fetch/generate an avatar based on the information you have about that user.
MIT License
643 stars 118 forks source link

No possibility to set empty alt and title #248

Closed konhi closed 2 years ago

konhi commented 2 years ago

Thank you for this great library. I'm happy to use it in my project! I stumbled upon one little imperfection.

Expected behavior

<Avatar alt="" title="" name="ambassify" -> <img alt="" title="" />

Current behavior

<Avatar alt="" title="" name="ambassify" -> <img alt="ambassify" title="ambassify" />

It's because "" == false and it's being checked here

Why is it bad?

It leads to repeated value which is bad for accessibility

<h1>Ambassify</h1>
<img alt="ambassify" title="ambassify"/>

"Ambassify" is being repeated 2 times!

Possible non-breaking solution

Add option to pass parameter which will cause in empty alt and title, e.g. noDescription, noAccessibility, noAlternativeText, noAlt

<Avatar alt="" title="" name="ambassify" noDescription -> <img alt="" title="" />

JorgenEvens commented 2 years ago

Hi @konhi ,

I would consider the empty string as a non-empty value, and would accept a PR that adds a arg === '' check to the line you referenced.

Since your primary issue was that the alt and title were showing the same value might I suggest another solutions. In cases where it does not have to strictly generate alt="" but removing the alt attribute entirely works as well, you could set alt to false rather than the empty string and it would not get rendered into your HTML at all.

ashandileo commented 2 years ago

Hi @JorgenEvens

I can't assign a false value to the title/alt attribute in typescript I get the error message Type 'boolean' is not assignable to type 'string'

Screen Shot 2022-07-21 at 13 24 06
JorgenEvens commented 2 years ago

This has been resolved in release 5.0.3. Thanks for the PR @ritchiejacobs !