-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
SpeedRunEthereum banner on developers page #12891
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.
LGTM
@carletex, thank you for putting this together. I just have a small adjustment for the mobile, to make it centered and aligned with the other sections |
Just pushed a fix: 531e06a / beb146b Thanks for the feedback @nloureiro |
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 @carletex! I'm a power contributor here and also a collaborator/maintainer for Chakra. Figured I take a quick look here at your approach.
Nice work with implementation in the code structure. Just one tiny comment from me for consistency sake, but otherwise looks ready to go! 😁
Co-authored-by: Tyler Pfledderer <tyler.pfledderer@gmail.com>
src/pages/developers/index.tsx
Outdated
position="absolute" | ||
insetInlineStart="8" | ||
maxW={{ base: "lg", xl: "xl" }} | ||
top={{ base: "25", xl: "50" }} |
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 we use chakra units here?
and adjust to better fit on mobile
top={{ base: "2", md:"4" lg: "8" }}
(I think this is the how it's done, we want 8px >> 16px >>32px)
left={{ base: "2", md:"4" lg: "8" }}
(do the same on the left too)
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.
As this is a temporary banner for the moment, I'm just going to merge it as is
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: 2024 Ethereum.org Contributor: Head to gitpoap.io & connect your GitHub account to mint! Learn more about GitPOAPs here. |
Hey all!
This PR results from a conversation between @austintgriffith and @minimalsm, @nloureiro, Jakub, etc (That's why I didn't create an issue first, but happy to do so if needed!).
The idea is to have a SpeedRunEthereum banner on the
/developers
page.Here is a preview video of how it would look like:
ethorg_sre.mp4
I'm not familiar with the codebase, but tried to stick to the project standards. Let me know if there is anything I should change (code or design related), or feel free to push any changes directly.
Thank you!