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

ChakraUI: Migrate FileContributors.tsx [#8636] #9878

Merged
merged 9 commits into from
May 2, 2023

Conversation

SyedAhkam
Copy link
Contributor

@SyedAhkam SyedAhkam commented Apr 4, 2023

Migrated FileContributors.tsx to Chakra UI

Description

Contributing to the UI Library Epic: #6374

I have migrated many components that made up the FileContributors Component.

  • Migrated Modal component in Modal.tsx
  • Built Contributor and ContributorList components using chakra primitives.
  • Re-implemented the Skeleton components using components from chakra.
  • Removed unnecessary components like Container, SkeletonButtonContainer, and ButtonContent and replaced them with simple Flex widgets.
  • Migrated from old Icon component to new chakra based Icon
  • Centered Modal vertically, unlike the previous implementation.

The results wielded might seem slightly inconsistent, but I left them intentionally because it seemed better that way.

image
image

Related Issue

Solves #8636

@SyedAhkam SyedAhkam changed the title migrate: FileContributors.tsx to ChakraUI ChakraUI: Migrate FileContributors.tsx Apr 4, 2023
@gatsby-cloud
Copy link

gatsby-cloud bot commented Apr 4, 2023

✅ ethereum-org-website-dev deploy preview ready

@SyedAhkam SyedAhkam changed the title ChakraUI: Migrate FileContributors.tsx ChakraUI: Migrate FileContributors.tsx [#8636] Apr 4, 2023
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.

Thanks @SyedAhkam great work here. I liked the direction you took on adjusting the Modal to use the chakra modal.

I've only added a few changes:

  • conventions like p="2" => p={2} or p="1rem" => p={4}
  • removed some abstractions that were not totally necessary
  • cleanup some styles that were not necessary in a few components

I'll finish testing it soon but in general I think this is ready to be approved.

@github-actions github-actions bot added the event 📅 This issue or pull request is related to an event listing label Apr 25, 2023
@SyedAhkam
Copy link
Contributor Author

Thanks!

Ahh I'm not that familiar with ChakraUI so I'm not aware of all the prop conventions yet. I'll surely look into it.

I totally understand how you feel about unnecessary abstractions, I got a bit carried away trying to provide a 1:1 migration to the old codebase and that left me with components like ContributorButton and LeftContent. I'll keep this in mind while contributing back again.

I'm glad that you liked my implementation :)

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.

:shipit:

@pettinarip pettinarip merged commit 36752e0 into ethereum:dev May 2, 2023
@gitpoap-bot
Copy link

gitpoap-bot bot commented May 2, 2023

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

Be sure to join the Ethereum.org discord if you are interested in contributing further to the project or have any questions for the team.

GitPOAP: 2023 Ethereum.org Contributor:

GitPOAP: 2023 Ethereum.org Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

@corwintines corwintines mentioned this pull request May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archived: chakra-migration event 📅 This issue or pull request is related to an event listing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants