-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ShadCN]: Migrate Avatar to ShadCN #14749
base: dev
Are you sure you want to change the base?
[ShadCN]: Migrate Avatar to ShadCN #14749
Conversation
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing. LGTM @TylerAPfledderer
ref={ref} | ||
style={ | ||
{ | ||
"--avatar-base-shadow-color": "hsl(var(--primary-low-contrast))", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we just use shadow-primary-low-contrast
? or define it in the theme shadow-avatar
?
Description
Migrates
Avatar
to ShadCN in the design system.Important:
IssuesList
,FileContributors
, andLeaderboard
components all useAvatar
directly from Chakra, not from the custom component. This usage is missing the use of the avatar as a link (nohref
prop used).In addition, the general usage of the avatar in prod does not match intent from the design system.
IssuesList
, the avatar is accompanied by static text of the contributor's name, and not served as a link to the contributor's profileLeaderboard
, the design of the list item is too complex to be able to use the avatar from the design system.FileContributors
it is also a little too complex for the avatar from the design system, as it adds static text around the contributor linkWould need some direction on how to further proceed with this PR to address the above use cases, or resolve them in a separate PR.
Related Issue
#13946