-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Refactor HorizontalCard
component to ChakraUI.
#7784
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
Conversation
✅ ethereum-org-website-dev deploy preview ready
|
Hey @ameeetgaikwad, thanks for the PR 😎 @pettinarip will be reviewing our Chakra PRs soon. Could you remove the |
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.
Hey @ameeetgaikwad! thanks for the PR! 💪🏼
- I see that you have created a
package-lock.json
& changed theyarn.lock
. Lets not include that here since it is not in the scope of this PR. - I've left a few change requests about the Chakra implementation.
@@ -1,26 +1,6 @@ | |||
import React, { ReactNode } from "react" | |||
import styled from "@emotion/styled" | |||
import Emoji from "./OldEmoji" |
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 change this as well. We have a new Emoji
component that is using Chakra.
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.
With that change, we will have to remove the support of emojiSize
here. It should be renamed to emojiFontSize
and we would need to update those props on the places where we are using it.
For example, in pages/stablecoins.tsx
// before
<TokenCard
key={idx}
emoji={token.emoji}
description={token.description}
emojiSize={3}
/>
// now
<TokenCard
key={idx}
emoji={token.emoji}
description={token.description}
emojiFontSize="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.
Can you eleborate? What should I import?
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 check that here in the Dependencies section https://github.com/ethereum/ethereum-org-website/blob/dev/docs/chakra-migration-guide.md#update-dependencies
Basically you need to replace OldEmoji
with the new emoji component located here: src/components/Emoji
.
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.
Should I refactor pages/stablecoins.tsx
also? Should I ask to get assigned for it in the issues section?
Hey @ameeetgaikwad I see that you have merged back the files. Do you need help with this? if it is easier for you, you can create a new PR with a clean branch. Feel free to ping me on Discord if you need help with git or anything else. |
yeah i'll work on a new branch |
Description
Migrate
HorizontalCard
component to Chakra.Includes refactor of
p
anddiv
of the component.