-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
feat: migrate HorizontalCard to tailwind #14226
base: dev
Are you sure you want to change the base?
feat: migrate HorizontalCard to tailwind #14226
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.
@kushagrasarathe looking good, left a few comments.
<div className="text-lg">{title}</div> | ||
<div className="text-base">{description}</div> |
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.
I think we could still use p
tags here to keep the semantics
<div className="text-lg">{title}</div> | |
<div className="text-base">{description}</div> | |
<p className="text-lg">{title}</p> | |
<p className="text-base">{description}</p> |
@@ -303,8 +303,8 @@ const GasPage = () => { | |||
key={benefit.emoji} | |||
emoji={benefit.emoji} | |||
description={benefit.description} | |||
className="text-5xl" | |||
align="center" | |||
emojiClassName="text-5xl" |
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.
5xl is the default size applied in HorizontalCard
. We can remove, its just redundant.
emojiClassName="text-5xl" |
@@ -458,7 +458,7 @@ const StablecoinsPage = ({ markets, marketsHasError }) => { | |||
<HorizontalCard | |||
emoji={token.emoji} | |||
description={token.description} | |||
className="text-5xl" | |||
emojiClassName="text-5xl" |
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.
emojiClassName="text-5xl" |
className="text-[2.5rem]" | ||
alignItems="center" | ||
className="my-0.5 w-[100%] items-center" | ||
emojiClassName={"text-[2.5rem]"} |
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.
emojiClassName={"text-[2.5rem]"} | |
emojiClassName="text-[2.5rem]" |
@@ -212,7 +212,7 @@ const CardContainer = (props: FlexProps) => ( | |||
) | |||
|
|||
const TokenCard = (props: ComponentProps<typeof HorizontalCard>) => ( | |||
<HorizontalCard minW="full" my={2} mx={0} borderRadius={0} {...props} /> | |||
<HorizontalCard {...props} className="mx-0 my-2 min-w-full rounded-none" /> |
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 need to use the cn
function here to merge the classNames. Otherwise, we will always override the className passed to TokenCard
.
<HorizontalCard {...props} className="mx-0 my-2 min-w-full rounded-none" /> | |
<HorizontalCard className={cn("mx-0 my-2 min-w-full rounded-none", className)} {...props} /> |
className="text-5xl" | ||
align="center" | ||
emojiClassName="text-5xl" | ||
className="flex items-center py-2" |
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.
className="flex items-center py-2" | |
className="flex items-center" |
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.
@kushagrasarathe looking good, left a few comments.
Migrates
HorizontalCard
component to tailwind from ChakraDescription
This PR updates the HorizontalCard to use tailwindcss instead of Chakra ui. Styles have been updated for both light and dark version.
Related Issue
#13946