-
Notifications
You must be signed in to change notification settings - Fork 31
Identicons #17
Identicons #17
Conversation
Adds ethereum-blockies-base64
src/dataDisplay/Identicon/index.tsx
Outdated
|
|
||
| type Props = { | ||
| address: string; | ||
| diameter: number; |
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.
Diameter values should be handled through the theme, as we do for Text size for example.
src/dataDisplay/Identicon/index.tsx
Outdated
|
|
||
| const Identicon = (props: Props) => { | ||
| const { address, diameter } = props; | ||
| const style: StyleProps = getStyleFrom(diameter); |
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.
No need to define style: StyledProps, the type it's inferred.
src/dataDisplay/Identicon/index.tsx
Outdated
| useEffect(() => { | ||
| const image = generateBlockieIdenticon(address, diameter); | ||
|
|
||
| if (!identicon || !identicon.current) { |
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.
you can simplify the condition in this way: if(!identicon?.current)
Also, you could move this block to the first sentence, so if it is not identicon attached, the image shouldn't be created.
|
Hey @nicosampler I simplified the component, should be better now , let me know what do you think :) |
|
great, I think we should change this: https://github.com/gnosis/safe-react-components/pull/17/files/ca3b021928af9b8d8028f2f8153dcb38ffddf189..d9afd368f19d93d35dbb20a74c06ea6324ffd189#r438738235. We should try to avoid using numbers for sizes as much as possible. |
It's better now |
|
Should be ok now, please check this out @nicosampler |
Description