Skip to content
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

Refactor CallToContribute component #8372

Merged
merged 3 commits into from Oct 29, 2022

Conversation

TylerAPfledderer
Copy link
Contributor

@TylerAPfledderer TylerAPfledderer commented Oct 25, 2022

Description

Refactor CallToContribute post Chakra migration.

  • Create ContentColumn reusable component for the image and the content.
  • Use the Show component to only show the image above the lg breakpoint (see additional information below).
  • Create DescriptionParagraph reusable component for the content paragraphs.

Related Issue

N/A

Additional Information

As of this PR, the Chakra UI migration is not complete. If I were to do the following with Show:

<Show above="lg">

The token value would not be recognized. This is due to the under the hood functionality of Show using the useTheme hook from Chakra. useTheme looks through the emotion package to retrieve theme values from the Context, and I believe it is receiving the old theme config instead of the Chakra config. Indeed there is no issue when applying Chakra's token values in the style props (I believe it has to do with how the theme config is acquired different from useTheme)

useToken solves this issue temporarily until the complete merge occurs.

@github-actions github-actions bot added the needs review 👀 Review is needed for this issue or pull request label Oct 25, 2022
@gatsby-cloud
Copy link

gatsby-cloud bot commented Oct 25, 2022

✅ ethereum-org-website-dev deploy preview ready

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.

Nice! surprising to know that Show doesn't read the values directly from the Chakra theme 🤔 however it kind of makes sense if it is using useTheme under the hood. Thanks for sharing that information!

@pettinarip pettinarip merged commit 28fbbd6 into ethereum:dev Oct 29, 2022
@TylerAPfledderer
Copy link
Contributor Author

TylerAPfledderer commented Oct 29, 2022

@pettinarip certainly! I have not found anything yet that leads me to believe otherwise without deleting that old config file

@TylerAPfledderer TylerAPfledderer deleted the refactor/call-to-contribute branch October 29, 2022 16:31
@corwintines corwintines mentioned this pull request Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chakra-migration needs review 👀 Review is needed for this issue or pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants