feat: New safe app card design #3804
Conversation
…component to our Providers file
|
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
DaniSomoza
requested review from
usame-algan,
mmv08,
iamacook,
katspaugh,
dasanra,
DiogoSoaress and
yagopv
| const safeAddress = extractSafeAddress() | ||
| const appsPath = generateSafeRoute(SAFE_ROUTES.APPS, { | ||
| shortName: getShortName(), | ||
| safeAddress, | ||
| }) |
There was a problem hiding this comment.
Before merging, let's use this hook (#3826) or I'll merge main into that PR and edit it there (depending on which one makes it through QA first).
Please leave this comment unresolved.
| <SafeAppContainer size={size} layout initial={{ opacity: 0 }} animate={{ opacity: 1 }} exit={{ opacity: 0 }}> | ||
| <StyledAppCard size={size}> | ||
| <LogoContainer size={size}> | ||
| <SafeAppLogoSK size={size} /> | ||
| </LogoContainer> | ||
| <DescriptionContainer size={size}> | ||
| <SafeAppTitleSK /> | ||
| <SafeAppDescriptionSK /> | ||
| <SafeAppDescriptionSK /> | ||
| </DescriptionContainer> | ||
| </StyledAppCard> | ||
| </SafeAppContainer> | ||
| ) | ||
| } | ||
|
|
||
| return ( | ||
| <SafeAppContainer size={size} layout initial={{ opacity: 0 }} animate={{ opacity: 1 }} exit={{ opacity: 0 }}> |
There was a problem hiding this comment.
Apart from size, you're using the same values on both SafeAppContainer components. Are we using this container elsewhere or can we abstract them?
There was a problem hiding this comment.
mmm what do you mean with abstract them?
There was a problem hiding this comment.
Perhaps just have them fixed in the component so we don't have to pass them via props at all, or create a safeContainProps and spread them onto both. What do you think?
There was a problem hiding this comment.
I think that the best option here is to keep it as it is now. The size prop is needed to render the correct card design:
There was a problem hiding this comment.
For size, yes, but I meant the others. Can we not just hardcode those to the component itself? They are the same values for both.
There was a problem hiding this comment.
do you mean this?
const SafeAppContainer = styled(motion.div).attrs({
layout: true,
initial: { opacity: 0 },
animate: { opacity: 1 },
exit: { opacity: 0 },
})`
position: relative;
display: flex;
instead of this?
<SafeAppContainer size={size} layout initial={{ opacity: 0 }} animate={{ opacity: 1 }} exit={{ opacity: 0 }}>
| <IconBtn onClick={shareSafeApp} aria-label={`copy ${safeApp.name} Safe App share link to clipboard`}> | ||
| <Icon size="md" type="share" tooltip="copy share link to clipboard" /> |
There was a problem hiding this comment.
| <IconBtn onClick={shareSafeApp} aria-label={`copy ${safeApp.name} Safe App share link to clipboard`}> | |
| <Icon size="md" type="share" tooltip="copy share link to clipboard" /> | |
| <IconBtn onClick={shareSafeApp} aria-label={`Copy shareable ${safeApp.name} Safe App link to clipboard`}> | |
| <Icon size="md" type="share" tooltip="Copy shareable link to clipboard" /> |
There was a problem hiding this comment.
Hi, @liliiaorlenko what is the best label for these tooltips?
There was a problem hiding this comment.
“Copy share link”
“Unpin from the Safe Apps”
“Remove Custom Safe App”
Lets keep it short
* Add analytics for the Share App feature * Improve landing tracking
…ct into new-safe-app-card-design
ESLint Summary View Full Report
Report generated by eslint-plus-action |
What it solves
Resolves #413
Figma
Screenshots