-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
fix(ui): avatar overlap #7834
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(ui): avatar overlap #7834
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Pull Request Overview
This PR fixes an avatar overlap issue by removing the expandable condition and updating the CSS to ensure proper spacing between avatars regardless of their count.
- Removed the conditional addition of the "expandable" class in AvatarGroup/index.tsx.
- Updated CSS to use the new "first:ml-0" syntax for adjusting the first avatar's margin.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/ui-components/Common/AvatarGroup/index.tsx | Removed conditional "expandable" class to simplify avatar container rendering. |
packages/ui-components/Common/AvatarGroup/index.module.css | Updated margin settings using new CSS syntax "first:ml-0" for consistent avatar overlap. |
Comments suppressed due to low confidence (2)
packages/ui-components/Common/AvatarGroup/index.module.css:11
- Please verify that the use of the new CSS syntax 'first:ml-0' is fully supported in all target browsers and build environments to avoid potential compatibility issues.
@apply -ml-2
first:ml-0;
packages/ui-components/Common/AvatarGroup/index.tsx:45
- Removing the conditional 'expandable' class simplifies the component, but please confirm that this change does not affect the dynamic behavior for different avatar counts, particularly in edge cases with very few or many avatars.
<div className={classNames(styles.avatarGroup, styles[size])}>
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #7834 +/- ##
==========================================
- Coverage 75.47% 75.43% -0.04%
==========================================
Files 101 101
Lines 8309 8305 -4
Branches 218 218
==========================================
- Hits 6271 6265 -6
- Misses 2036 2038 +2
Partials 2 2 ☔ View full report in Codecov by Sentry. |
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.
LGTM !
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.
LGTM! Thank you, Caner! BTW, I'm more than happy to fast-track this if needed, but there's no real rush here.
Lighthouse Results
|
Description
As discussed in the Slack thread;
When there are fewer avatars or when viewed on the blog listing or blog pages, the avatars were not overlapping. With this PR, the issue where the first avatar was getting cut off has been fixed, and now the avatars properly overlap regardless of the number of items
Validation
Before
After