Skip to content

feat: Add AvatarPair#1052

Draft
pawelgrimm wants to merge 9 commits into
pawel/feat/avatar-groupfrom
pawel/feat/avatar-pair
Draft

feat: Add AvatarPair#1052
pawelgrimm wants to merge 9 commits into
pawel/feat/avatar-groupfrom
pawel/feat/avatar-pair

Conversation

@pawelgrimm
Copy link
Copy Markdown
Contributor

@pawelgrimm pawelgrimm commented May 28, 2026

Short description

Adds <AvatarPair> for two overlapping avatars.

PR Checklist

  • Added tests for bugs / new features
  • Updated docs (storybooks, readme)
  • Reviewed and approved Chromatic visual regression tests in CI

References

📸 Demo

CleanShot 2026-05-27 at 22 57 28@2x

Copy link
Copy Markdown
Member

@doistbot doistbot left a comment

Choose a reason for hiding this comment

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

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 polymorphicComponent utility in favor of standard React.forwardRef patterns 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-child selectors.
  • Button defaults: Explicitly default to type="button" when the component is rendered as="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-radius is defined here and populated in getAvatarPairStyle, but it is never consumed in avatar.module.css (unlike its counterpart in AvatarGroup). It can be safely removed from AvatarPairStyle, the return object below, and the .avatarPair block in the CSS file.
  • [P3] src/avatar/avatar-pair.tsx:24: AVATAR_PAIR_MASK_BY_SIZE is a byte-for-byte copy of AVATAR_GROUP_MASK_BY_SIZE in avatar-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 into src/avatar/utils.ts and 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-error case with three avatars; otherwise a typing regression that allows extra children would still pass while breaking the pair layout selectors.

Share FeedbackReview Logs

* 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]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

P2 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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

P2 When this renders with as="button", it gets the HTML default type="submit". Using AvatarPair inside a form will submit the form on click unless every caller remembers to override it. Default the root to type="button" when as === 'button', while still letting an explicit type prop win.

* Displays two Avatar children with the second avatar positioned diagonally
* above-left of the first avatar.
*/
const AvatarPair = polymorphicComponent<'div', AvatarPairOwnProps, 'omitClassName'>(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

P2 polymorphism.ts and polymorphicComponent are marked as deprecated in the repository guidelines (CLAUDE.md). New components should avoid this utility and instead use React.forwardRef with a named function matching the component name (using asChild or standard DOM wrappers if custom tags are needed).

@pawelgrimm pawelgrimm force-pushed the pawel/feat/avatar-pair branch from 5c61134 to faa2256 Compare May 28, 2026 03:36
@pawelgrimm pawelgrimm changed the title feat: Add AvatarGroup feat: Add AvatarPair May 28, 2026
@pawelgrimm pawelgrimm self-assigned this May 28, 2026
@pawelgrimm pawelgrimm added the 🙋 Ask PR Used for PRs that need a review before merging. label May 28, 2026
@pawelgrimm pawelgrimm marked this pull request as draft May 28, 2026 03:59
@pawelgrimm pawelgrimm force-pushed the pawel/feat/avatar-pair branch from faa2256 to 5bfcc1f Compare May 28, 2026 04:37
pawelgrimm and others added 9 commits May 28, 2026 09:48
- 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>
@pawelgrimm pawelgrimm force-pushed the pawel/feat/avatar-pair branch from 5bfcc1f to 576560c Compare May 28, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🙋 Ask PR Used for PRs that need a review before merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants