Skip to content

Conversation

@albingroen
Copy link
Contributor

First pull request here. Not sure if I might be braking some design rules. If so, I would gladly discuss this with you with the goal to achieve a better solution.

If you want (which you more or less always want, although maybe not on github.com) to use a round Avatar component, you currently have to supply inline styling. From my short time with building with primer components (outside of github.com) I sense this is an anti-pattern.

My solution here is to add a shape prop to the Avatar component that can either be passed in as square or round. If passed in as square, everything is as normal, but if passed in as round, the borderRadius will be set to 50% to achieve a round avatar image.

Screenshots

Before
image

After
image

Merge checklist

  • Added or updated TypeScript definitions (index.d.ts) if necessary
  • Added/updated tests
  • Added/updated documentation
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

@vercel
Copy link

vercel bot commented Apr 16, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/primer/primer-components/drqvphzqa
✅ Preview: https://primer-componen-git-fork-albingroen-albingroen-avatar-fo-790951.primer.now.sh

@vercel vercel bot temporarily deployed to Preview April 16, 2020 21:23 Inactive
@emplums
Copy link

emplums commented Apr 17, 2020

Hey @albingroen thanks for the PR! Did you read our minds?? 😂 We we actually planning on doing this work very soon 🙌 I've got to double check with the designer but I think this API should work well :)

@albingroen
Copy link
Contributor Author

Hey @albingroen thanks for the PR! Did you read our minds?? 😂 We we actually planning on doing this work very soon 🙌 I've got to double check with the designer but I think this API should work well :)

Pure coincidence! 😄

I've been using primer for a little bit and I've felt a couple of features been missing, and then I realised I can just build them myself 🙂

Let's hope the API works. Maybe a boolean type could work as well, since there probably won't be many more shapes than square and round? 🤷‍♂️

@emplums emplums changed the base branch from master to avatar-tweaks April 21, 2020 17:51
Copy link

@emplums emplums left a comment

Choose a reason for hiding this comment

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

Approving! I'm going to merge this into another branch avatar-tweaks to make a few changes responding to some design feedback, and then I'll merge that branch into the release branch when it's ready! Thanks again for taking this on! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants