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

[Avatar] Use structured / semantic markup for avatars and avatar groups #33994

Merged
merged 10 commits into from Aug 31, 2022

Conversation

paulschreiber
Copy link
Contributor

@paulschreiber paulschreiber commented Aug 19, 2022

Use structured / semantic markup for avatars and avatar groups. Improves experience for screen reader users.

  • Add component overriding to the AvatarGroup component.
  • Have AvatarGroup use ul by default. (consider in v6)
  • Have Avatar use li by default. (consider in v6)

Fixes #33993.

@mui-bot
Copy link

mui-bot commented Aug 19, 2022

Details of bundle changes

Generated by 🚫 dangerJS against 413e16e

@paulschreiber
Copy link
Contributor Author

Need some help updating the .d file to add the propType.

@danilo-leal danilo-leal changed the title Use structured / semantic markup for avatars and avatar groups [Avatar] Use structured / semantic markup for avatars and avatar groups Aug 22, 2022
@danilo-leal danilo-leal added component: avatar This is the name of the generic UI component, not the React module! accessibility a11y labels Aug 22, 2022
@siriwatknp
Copy link
Member

@paulschreiber Thanks for the PR but can you fix only the component prop issue for AvatarGroup first?

I think changing the semantics should be in v6. cc @mnajdova

@paulschreiber
Copy link
Contributor Author

@siriwatknp I couldn't figure out the right component prop change. Can you make a push to the branch or GitHub code suggestion?

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please apply the suggested changes and run:

yarn proptypes
yarn docs:api
yarn prettier

We can't change the structure of the components in a minor, mainly because the avatar can be used outside of a group, which means that li as a tag would be wrong in that case.

paulschreiber and others added 3 commits August 25, 2022 09:31
Co-authored-by: Marija Najdova <mnajdova@gmail.com>
Signed-off-by: Paul Schreiber <github@paulschreiber.com>
Co-authored-by: Marija Najdova <mnajdova@gmail.com>
Signed-off-by: Paul Schreiber <github@paulschreiber.com>
@paulschreiber
Copy link
Contributor Author

@mnajdova @siriwatknp I pushed the other changes but am not sure what to do for the component prop.

@paulschreiber
Copy link
Contributor Author

@mnajdova @siriwatknp component prop is updated.

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks for the fix!

@siriwatknp siriwatknp merged commit 660b9da into mui:master Aug 31, 2022
daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y component: avatar This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AvatarGroup and Avatar use unstructured markup
5 participants