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

fix: avatar list accessibility #14

Merged
merged 2 commits into from
Jun 14, 2019
Merged

Conversation

jsomsanith-tlnd
Copy link
Contributor

What is the problem this PR is trying to solve?
Avatar list component is not accessible

non-a11y

(SR = Screen Reader)

  1. The semantic is not right. It's a list of elements but it's a succession of divs. SR doesn't announce that as a list. In fact it should announce that as a list of users.

  2. On big lists with ellipsis, the ellipsis is a text (example: + 97). But it's not understandable, out of context. And with the previous issue about semantic, the context (which is a list of users) is not here.

What is the chosen solution to this problem?

  1. Use ul/li to create the list. Add an aria-label users, so SR
  • will announce a list "users",
  • tell the index of the focused element,
  • the number of elements in the list
  1. Add an aria-label with a more complete string (example: +97 more user(s))

a11y

@domyen
Copy link
Member

domyen commented Jun 14, 2019

Nice this is amazing. According to Chromatic here there's extra space added to the bottom of this component which could affect visual alignment. This usually happens if there's extra whitespace inside of an inline/inline-block element (in this case I think it's the li).

Can you try removing the whitespace from around the li in the code if there is any?

Alternatively a few CSS hacks (which are OK by me):

  • set li to display: inline-flex
  • set li to font-size:0 which eliminates the whitespace

@domyen domyen merged commit bd941d5 into master Jun 14, 2019
@domyen domyen deleted the jsomsanith/fix/avatar_list_a11y branch June 14, 2019 22:27
@jsomsanith-tlnd jsomsanith-tlnd restored the jsomsanith/fix/avatar_list_a11y branch June 15, 2019 12:19
@jsomsanith-tlnd jsomsanith-tlnd deleted the jsomsanith/fix/avatar_list_a11y branch June 15, 2019 12:20
@kylesuss
Copy link
Contributor

🚀 PR was released in v0.0.23 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants