-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[AvatarGroup] Add max avatar prop #19853
Conversation
@oliviertassinari Can you take a look at this when you get a chance? |
@material-ui/lab: parsed: +1.01% , gzip: +1.05% Details of bundle changes.Comparing: f27c78f...69f9942 Details of page changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great start. I don't know if we need a standalone demo in the documentation for this feature, but we definitely need a new test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Playing with the prop descriptions. (It might inspire further tweaks. @joshwooding ?)
Also, it would be good to add a demo for the new functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there's a lot of room for off-by-one errors we should add some tests for various combinations of children.length
and max
.
@GFynbo Let us know if there is any blockers to complete the pull request :). |
@oliviertassinari Yup! Just some personal issues came up so I've been a little behind but will be finishing this up and pushing Friday/Sat. Sorry! |
Co-Authored-By: Matt <[email protected]>
Co-Authored-By: Matt <[email protected]>
fcb310a
to
0546c18
Compare
a46a63e
to
d008c8e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GFynbo I have tried to apply the feedback suggested in the review. Let me know if it looks all right :)
d008c8e
to
69f9942
Compare
@oliviertassinari Looking through it now. Sorry for the delay!! The coronavirus is throwing things off. |
@oliviertassinari It looks good to me! Only consideration is to give a different name to the people in the docs example but it doesn't really matter since it's not showing up |
@GFynbo Nice work! |
This adds the max avatars displayed prop to the
AvatarGroup
for automatically adding an avatar with+x
functionality. The default value is5
and can be overridden withmax=x
. The docs and examples have been updated.References #18869