-
Notifications
You must be signed in to change notification settings - Fork 7
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
Reskin
| Update deterministic avatar generation library
#1607
Conversation
…ent with ecosystem
✅ Deploy Preview for fractal-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
borderRadius={'50%'} | ||
overflow={'hidden'} |
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.
To make them circles
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.
Couple minor suggestions, but looks good!
return ( | ||
<Box | ||
h={avatarSizes[size]} | ||
w={avatarSizes[size]} | ||
borderRadius={'50%'} |
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.
We don't need to wrap in curly braces here, it could be just borderRadius="50%"
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.
Fair, my autocomplete sticks those braces in. Will remove.
return ( | ||
<Box | ||
h={avatarSizes[size]} | ||
w={avatarSizes[size]} | ||
borderRadius={'50%'} | ||
overflow={'hidden'} |
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.
Same here
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.
Same
> | ||
<Jazzicon address={address} /> | ||
<img |
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.
Do we wanna use plan <img />
tag here? I think usually we're using <Image />
component from @chakra-ui/react
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.
Good callout
Reskin
| Update deterministic avatar generation library
Replaces the "Jazzicon" avatars with classic "Blockie"-style avatar, to be consistent with the ecosystem.
Testing
Fractal:
Etherscan: