react icon indicating copy to clipboard operation
react copied to clipboard

Add subtle border to the avatar to match the CSS implementation

Open maximedegreve opened this issue 3 years ago • 5 comments

The avatar was missing a border compared to the css implementation.

Screenshot 2022-08-02 at 15 23 08

The change is best visible in dark modes with @colebemis's avatar.

<Avatar src="https://avatars.githubusercontent.com/u/4608155?s=100&v=4" size={50} />

Merge checklist

  • [ ] Added/updated tests
  • [ ] Added/updated documentation
  • [ ] Tested in Chrome
  • [ ] Tested in Firefox
  • [ ] Tested in Safari
  • [ ] Tested in Edge

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

maximedegreve avatar Aug 02 '22 13:08 maximedegreve

🦋 Changeset detected

Latest commit: bc7b85a1f6d9497a6306878d1c09fd75b4ee1c37

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 Patch

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

changeset-bot[bot] avatar Aug 02 '22 13:08 changeset-bot[bot]

size-limit report 📦

Path Size
dist/browser.esm.js 76.32 KB (+0.02% 🔺)
dist/browser.umd.js 76.93 KB (+0.02% 🔺)

github-actions[bot] avatar Aug 02 '22 13:08 github-actions[bot]

Hi!

Do you have access to the chromatic build to make sure all the visual changes are intentional: https://www.chromatic.com/build?appId=61a90feace7802003a4d9c45&number=1009

Side note: In case you want, you can review all the different themes at once now: https://primer-6e414db3be-13348165.drafts.github.io/storybook/?path=/story/tokens-avatartoken--default-token&globals=colorScheme:all

Side note 2: You might need to update jest snapshots by running npm run test:update

siddharthkp avatar Aug 02 '22 15:08 siddharthkp

Hey @siddharthkp

I forgot about Chromatic, it's so good! 🥹 The visual changes look good.

I've tried to update the snapshots but I got into an error:

Screenshot 2022-08-02 at 16 39 41

maximedegreve avatar Aug 02 '22 15:08 maximedegreve

I'm not 100% sure, but I think running npm install and npm build before updating snapshots should fix this

siddharthkp avatar Aug 02 '22 16:08 siddharthkp