jamesplease / materialish

React components that loosely follow Material Design
https://materialish.js.org
MIT License
17 stars 5 forks source link

A11y: add aria to Avatar #288

Closed Muhnad closed 4 years ago

Muhnad commented 6 years ago

this change for A11y and this is a workaround to get the same result for <img src="" alt=""/> #232 .

Muhnad commented 6 years ago

Suggest as I see the alt & initials props are redundant so I recommend to use one of them and remove the other.

removes initials prop and make the alt is the main prop and generate initials from alt value automatically.

a quick demo for what I need to say:

{
  !image && (
    <div className="mt-avatar_initials">
      {alt.length >= 2 && alt.slice(0, 2)}
    </div>
  )
}
jamesplease commented 6 years ago

Thanks for the PR @Muhnad ! I agree that we could make the avatar more accessible, and your suggestions here are a step in the right direction.

One concern I have is that I don't think we can assume that we would be able to generate the initials from any given alt.

At Netflix (where we're using this library), we have a specific field for users called initials, and it is important that we display that specific value in the avatar for consistency with other apps in our ecosystem.

Do you think it would be accessible to pass the initials as the alt? Presumably, that would be interpreted by a screen reader in the same way that the version without the image is presented.

//cc @JPorry

Muhnad commented 6 years ago

@jamesplease I agree with ur concern about generating initials from alt but I think we'll face this issue rarely so we can work around it to handle it.

as you said the initials are important field at Netflix so we can't replace it so we can keep the initials prop and add alt prop also and when user didn't upload image we can hide initials from screen reader and user will keep read the alt value and initials in this case will not make the user confused.

{!image && <div className="mt-avatar_initials" aria-hidden="true">{initials}</div>}