feat: Add AvatarPair#1052
Conversation
doistbot
left a comment
There was a problem hiding this comment.
Thanks pawelgrimm for your contribution 😊. The new AvatarGroup and AvatarPair components bring robust, size-responsive mask styling and solid test coverage.
Few things worth tightening:
- Component architecture: Migrate away from the deprecated
polymorphicComponentutility in favor of standardReact.forwardRefpatterns as per the project guidelines. - Child constraints: Narrow the child types or add a runtime guard to prevent fragments or wrappers from injecting 3+ DOM nodes and breaking the CSS
:first-child/:last-childselectors. - Button defaults: Explicitly default to
type="button"when the component is renderedas="button"to prevent accidental form submissions.
I also included a few optional follow-up notes in the details below.
Optional follow-up notes (4)
- [P3] src/avatar/avatar-pair.tsx:20:
--reactist-avatar-pair-rounded-radiusis defined here and populated ingetAvatarPairStyle, but it is never consumed inavatar.module.css(unlike its counterpart inAvatarGroup). It can be safely removed fromAvatarPairStyle, the return object below, and the.avatarPairblock in the CSS file. - [P3] src/avatar/avatar-pair.tsx:24:
AVATAR_PAIR_MASK_BY_SIZEis a byte-for-byte copy ofAVATAR_GROUP_MASK_BY_SIZEinavatar-group.tsx. That gives us two sources of truth for the same avatar geometry, so any future size/mask tweak has to be updated in both files or the overlap components will drift. Please hoist the shared mask lookup intosrc/avatar/utils.tsand reuse it here and in AvatarGroup. - [P3] src/avatar/avatar-pair.test.tsx:56: This only asserts the rounded branch. Since the default circle variant uses a different shape class and is the main rendering path, add a small sibling test for the default render (
avatarPairShape-circle) so a regression there doesn’t slip through unnoticed. - [P3] src/avatar/avatar-pair.test.tsx:67: The API promise here is “exactly two” children, but this test only covers the “not one child” side. Add a second
@ts-expect-errorcase with three avatars; otherwise a typing regression that allows extra children would still pass while breaking the pair layout selectors.
| * avatar (positioned bottom-right); the second is the diagonal overlay | ||
| * (positioned top-left, masked where it overlaps the first). | ||
| */ | ||
| children: readonly [React.ReactElement, React.ReactElement] |
There was a problem hiding this comment.
This tuple only guarantees “two React elements”, not “two direct Avatar children”. A fragment or wrapper component still type-checks here and can render 3+ DOM children, but the CSS only positions
:first-child and :last-child, so the extra nodes stack incorrectly. Narrow the child type to Avatar elements or add a runtime guard for non-direct children.
| ref, | ||
| ) { | ||
| return ( | ||
| <Box |
There was a problem hiding this comment.
| * Displays two Avatar children with the second avatar positioned diagonally | ||
| * above-left of the first avatar. | ||
| */ | ||
| const AvatarPair = polymorphicComponent<'div', AvatarPairOwnProps, 'omitClassName'>( |
There was a problem hiding this comment.
5c61134 to
faa2256
Compare
faa2256 to
5bfcc1f
Compare
- AvatarPair JSDoc spells out "exactly two children" and the foreground vs. overlay roles - mdx mirrors that, notes the count renders as +N, and points consumers at aria-label on the group root when it's a labeled entity - corrects stale --reactist-avatar-pair-rounded-radius default (4px → 5px) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tighten getContributor / getWorkspaceName return types and extract the workspace helper so callers don't need ! everywhere. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add file-level + per-rule comments explaining the back/front mask cutout. Rename --mask to --mask-thickness, collapse identical --first-center-x/-y into one var. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Inline /* layer N */ comments after gradient list items got shuffled onto the wrong items by prettier. Replaced with one comment block above the rule documenting the 1:1 layer ordering. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5bfcc1f to
576560c
Compare
Short description
Adds
<AvatarPair>for two overlapping avatars.PR Checklist
References
📸 Demo