Skip to content

Conversation

ameeetgaikwad
Copy link
Contributor

@ameeetgaikwad ameeetgaikwad commented Sep 10, 2022

Description

Migrate HorizontalCard component to Chakra.

Includes refactor of p and div of the component.

@gatsby-cloud
Copy link

gatsby-cloud bot commented Sep 10, 2022

✅ ethereum-org-website-dev deploy preview ready

@minimalsm
Copy link
Contributor

Hey @ameeetgaikwad, thanks for the PR 😎

@pettinarip will be reviewing our Chakra PRs soon. Could you remove the yarn.lock/package.json changes from this PR please?

Copy link
Member

@pettinarip pettinarip left a 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 the yarn.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"
Copy link
Member

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.

Copy link
Member

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"
/>

Copy link
Contributor Author

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?

Copy link
Member

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.

Copy link
Contributor Author

@ameeetgaikwad ameeetgaikwad Sep 17, 2022

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?

@pettinarip
Copy link
Member

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.

@ameeetgaikwad
Copy link
Contributor Author

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

@github-actions github-actions bot added the abandoned This has been abandoned or will not be implemented label Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

abandoned This has been abandoned or will not be implemented archived: chakra-migration dependencies 📦 Changes related to project dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants