Allow customization of AvatarStack hover behavior and size #3466

mperrotti · 2023-06-30T20:17:03Z

Allows consumers to disable the behavior where the stack expands to show all avatars on hover
Allows consumers to customize the size for the avatars in the stack. This can be done two ways:
- pass a size prop to AvatarStack
- pass a size prop to the Avatars that are direct children of AvatarStack
Does not allow an AvatarStack to render it's children at different sizes. The smallest size will always be picked.

Closes https://github.com/github/primer/issues/2366

Screenshots

Kapture.2023-06-30.at.16.09.20.mp4

Merge checklist

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

changeset-bot · 2023-06-30T20:17:06Z

🦋 Changeset detected

Latest commit: 356bf65

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2023-06-30T20:21:13Z

size-limit report 📦

Path	Size
dist/browser.esm.js	102.31 KB (+0.69% 🔺)
dist/browser.umd.js	102.88 KB (+0.71% 🔺)

broccolinisoup

Love it! 💖 Just left a tiny comment - not a blocker.

broccolinisoup · 2023-07-03T08:52:51Z

src/AvatarStack/AvatarStack.tsx


 type StyledAvatarStackWrapperProps = {
  count?: number
 } & SxProp

+const findSmallestNumber = (numbers: number[]): number => {


Would Math.min(...numbers) work here? 🤔 Just an idea - not a blocker at all.

Great suggestion! Soooo much easier.

maximedegreve

This is great @mperrotti 🎉

One thing that's missing in this PR is support for breakpoints.

Otherwise developers will have to get around this issue using:

<Box sx={{display: ['block', 'block', 'none']}}>
   <AvatarStack size={26}/>
</Box>
<Box sx={{display: ['none', 'none', 'block']}}>
   <AvatarStack size={20}/>
</Box>

Instead of:

<AvatarStack size={[20,20,24]}/>

Without this support it will also make working with HTML grids hard because the Box is never removed from the DOM and will create an additional grid cell.

siddharthkp · 2023-07-03T11:41:47Z

src/AvatarStack/AvatarStack.tsx

@@ -127,21 +163,42 @@ const transformChildren = (children: React.ReactNode) => {

 export type AvatarStackProps = {
  alignRight?: boolean
+  disableExpand?: boolean


Personal opinion, not strongly held: disableExpand is a double negative, which can feel confusing

What do you think about expand: boolean where default is true?

<AvatarStack /> <AvatarStack expand={false} />

It feels strange to have to explicitly pass propName={false} because boolean props are normally evaluated as false if the prop is excluded.

If we didn't set expand to true by default, then <AvatarStack /> would be the same as <AvatarStack expand={false} />

It feels strange to have to explicitly pass propName={false}

true true!

because boolean props are normally evaluated as false if the prop is excluded

not sure about that tho 😅

// default true <AvatarStack/> = <AvatarStack expand /> = <AvatarStack expand={true} /> // disabling default: <AvatarStack expand={false}/>

buuuut, also happy to go with what you feel strongly about

…sable-expand

mperrotti · 2023-07-06T02:26:58Z

@broccolinisoup and @maximedegreve - I have your suggestions implemented.

This should be merge-able once I:

Update tests and stories
Fix an annoying TypeScript error when trying to pass getBreakpointDeclarations() to merge() (@broccolinisoup - maybe we can pair on this?)
Other misc code cleanup
Comment any complicated code I couldn't simplify

maximedegreve · 2023-07-06T11:55:56Z

@mperrotti Oh no!

mperrotti · 2023-07-06T12:42:50Z

Haha thanks for sharing that @maximedegreve - I'll be fixing that while I make my other changes today.

maximedegreve · 2023-07-06T21:52:45Z

LGTM 🚀💪

dusave · 2023-07-06T22:53:50Z

Broader question, not a blocker for this PR: Are we adding any sort of prefers-reduced-motion rule in the animation definitions of components to disable them users automatically? This could be supplemented with the explicit prop (the consumer might want it disabled for all users), but for accessibility reasons, adding the css check could make it much easier to work with.

broccolinisoup

Looks great! Just left a comment about the type stuff but feel free to ignore if it is not helpful.

🚀 🚀 🚀

broccolinisoup · 2023-07-07T02:12:53Z

src/Avatar/Avatar.tsx

  ref,
 ) {
-  return <StyledAvatar ref={ref} alt={alt} size={size} square={square} {...rest} />
+  const avatarSx = isResponsiveValue(size)
+    ? merge(


Sorry I missed your ping about the type stuff.

I think we could also do

merge<BetterSystemStyleObject>(...)

instead of typing each input as?

Also totally optional maybe for readability

const avatarSx = isResponsiveValue(size) ? getBreakpointDeclarations( size, '--avatar-size' as keyof React.CSSProperties, value => `${value || DEFAULT_AVATAR_SIZE}px`, ) : {'--avatar-size': `${size}px`}

and then in the StyledAvatar

sx={merge<BetterSystemStyleObject>(avatarSx, sxProp)}

Just a suggestion - however you prefer.

I think I remember there being an performance issue with passing merge() directly to to sx, so I'm going to keep it as-is. Definitely going to use a generic instead of as though. Great suggestion!

Ohh I didn't know about the performance impact on using merge in sx! Thanks for letting me know!

mperrotti · 2023-07-07T13:37:50Z

@dusave

Are we adding any sort of prefers-reduced-motion rule in the animation definitions of components to disable them users automatically?

We do use prefers-reduced-motion rules in some components, but not this one. I'm going to check with a11y design to see if it makes sense for this small transition.

mperrotti · 2023-07-07T15:18:52Z

@dusave - I checked with a11y, and it looks like this is safe to keep enabled even when the user prefers reduced motion.

…sable-expand

…ct into mp/avatarstack-disable-expand

mperrotti added 3 commits June 29, 2023 15:32

adds a prop to prevent avatars from spreading on hover

af5de05

enables AvatarStack to render any size avatar

0f1a410

updates tests and stories

b56fb75

mperrotti requested review from a team and langermank June 30, 2023 20:17

mperrotti and others added 2 commits June 30, 2023 20:18

Update generated/components.json

51c6798

Create eighty-islands-join.md

dc66fd8

Merge branch 'main' into mp/avatarstack-disable-expand

aaa91bc

github-actions bot temporarily deployed to storybook-preview-3466 June 30, 2023 20:24 Inactive

github-actions bot temporarily deployed to storybook-preview-3466 June 30, 2023 20:25 Inactive

mperrotti temporarily deployed to github-pages June 30, 2023 20:27 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-3466 June 30, 2023 20:27 Inactive

github-actions bot temporarily deployed to storybook-preview-3466 June 30, 2023 20:28 Inactive

broccolinisoup approved these changes Jul 3, 2023

View reviewed changes

maximedegreve approved these changes Jul 3, 2023

View reviewed changes

maximedegreve reviewed Jul 3, 2023

View reviewed changes

siddharthkp reviewed Jul 3, 2023

View reviewed changes

mperrotti added 2 commits July 5, 2023 18:55

Merge branch 'main' of github.com:primer/react into mp/avatarstack-di…

e9ac0b5

…sable-expand

replace custom function to find smallest number with Math.min

1c6d74c

mperrotti temporarily deployed to github-pages July 5, 2023 23:02 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-3466 July 5, 2023 23:03 Inactive

makes Avatar and AvatarStack 'size' prop responsive

b791f1c

mperrotti temporarily deployed to github-pages July 6, 2023 02:34 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-3466 July 6, 2023 02:34 Inactive

updates stories and tests, fixes bugs

538585e

github-actions bot temporarily deployed to storybook-preview-3466 July 6, 2023 20:41 Inactive

mperrotti added the update snapshots label Jul 6, 2023

test(vrt): update snapshots

ffd7719

github-actions bot removed the update snapshots label Jul 6, 2023

primer bot temporarily deployed to github-pages July 6, 2023 21:13 Inactive

github-actions bot temporarily deployed to storybook-preview-3466 July 6, 2023 21:14 Inactive

broccolinisoup approved these changes Jul 7, 2023

View reviewed changes

mperrotti added 3 commits July 7, 2023 11:24

typescript improvements

efbea09

Merge branch 'main' of github.com:primer/react into mp/avatarstack-di…

0ff5a97

…sable-expand

Merge branch 'mp/avatarstack-disable-expand' of github.com:primer/rea…

1fb8e97

…ct into mp/avatarstack-disable-expand

mperrotti enabled auto-merge July 7, 2023 15:25

mperrotti temporarily deployed to github-pages July 7, 2023 15:33 — with GitHub Actions Inactive

github-actions bot temporarily deployed to storybook-preview-3466 July 7, 2023 15:34 Inactive

convert clasnames to clsx

356bf65

mperrotti deployed to github-pages July 7, 2023 15:55 — with GitHub Actions View deployment

github-actions bot deployed to storybook-preview-3466 July 7, 2023 15:55 View deployment

mperrotti added this pull request to the merge queue Jul 7, 2023

Merged via the queue into main with commit 11b54ac Jul 7, 2023

mperrotti deleted the mp/avatarstack-disable-expand branch July 7, 2023 16:13

This was referenced Jul 7, 2023

Release tracking #3493

Merged

Release tracking (next major) #3147

Closed

maximedegreve assigned mperrotti Aug 14, 2023

skemex mentioned this pull request May 24, 2024

[Snyk] Upgrade @primer/react from 35.26.1 to 35.32.2 skemex/wpe-actions-workshop#45

Open

rbragaru mentioned this pull request Aug 14, 2024

[Snyk] Upgrade @primer/react from 35.15.1 to 35.32.2 rbragaru/rocketdex#4

Open

skemex mentioned this pull request Sep 24, 2024

[Snyk] Upgrade @primer/react from 35.26.1 to 35.32.2 skemex/wpe-actions-workshop#55

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow customization of AvatarStack hover behavior and size #3466

Allow customization of AvatarStack hover behavior and size #3466

mperrotti commented Jun 30, 2023

changeset-bot bot commented Jun 30, 2023 •

edited

Loading

github-actions bot commented Jun 30, 2023 •

edited

Loading

broccolinisoup left a comment •

edited

Loading

broccolinisoup Jul 3, 2023

mperrotti Jul 5, 2023

maximedegreve left a comment •

edited

Loading

siddharthkp Jul 3, 2023 •

edited

Loading

mperrotti Jul 5, 2023

siddharthkp Jul 6, 2023 •

edited

Loading

mperrotti commented Jul 6, 2023

maximedegreve commented Jul 6, 2023

mperrotti commented Jul 6, 2023

maximedegreve commented Jul 6, 2023

dusave commented Jul 6, 2023 •

edited

Loading

broccolinisoup left a comment

broccolinisoup Jul 7, 2023

mperrotti Jul 7, 2023

broccolinisoup Jul 8, 2023

mperrotti commented Jul 7, 2023

mperrotti commented Jul 7, 2023

Allow customization of AvatarStack hover behavior and size #3466

Allow customization of AvatarStack hover behavior and size #3466

Conversation

mperrotti commented Jun 30, 2023

Screenshots

Merge checklist

changeset-bot bot commented Jun 30, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Jun 30, 2023 • edited Loading

size-limit report 📦

broccolinisoup left a comment • edited Loading

Choose a reason for hiding this comment

broccolinisoup Jul 3, 2023

Choose a reason for hiding this comment

mperrotti Jul 5, 2023

Choose a reason for hiding this comment

maximedegreve left a comment • edited Loading

Choose a reason for hiding this comment

siddharthkp Jul 3, 2023 • edited Loading

Choose a reason for hiding this comment

mperrotti Jul 5, 2023

Choose a reason for hiding this comment

siddharthkp Jul 6, 2023 • edited Loading

Choose a reason for hiding this comment

mperrotti commented Jul 6, 2023

maximedegreve commented Jul 6, 2023

mperrotti commented Jul 6, 2023

maximedegreve commented Jul 6, 2023

dusave commented Jul 6, 2023 • edited Loading

broccolinisoup left a comment

Choose a reason for hiding this comment

broccolinisoup Jul 7, 2023

Choose a reason for hiding this comment

mperrotti Jul 7, 2023

Choose a reason for hiding this comment

broccolinisoup Jul 8, 2023

Choose a reason for hiding this comment

mperrotti commented Jul 7, 2023

mperrotti commented Jul 7, 2023

changeset-bot bot commented Jun 30, 2023 •

edited

Loading

github-actions bot commented Jun 30, 2023 •

edited

Loading

broccolinisoup left a comment •

edited

Loading

maximedegreve left a comment •

edited

Loading

siddharthkp Jul 3, 2023 •

edited

Loading

siddharthkp Jul 6, 2023 •

edited

Loading

dusave commented Jul 6, 2023 •

edited

Loading