-
Notifications
You must be signed in to change notification settings - Fork 366
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
feat: landing - animate hero #197
Conversation
e6f7b34
to
da3d15c
Compare
Awesome work @olarclara! Pretty smooth, and the FPS is not dropping below 30. Thanks for all the effort 🎉👏👏 I have two comments that are important to fix:
|
03f5982
to
a894b3f
Compare
@zehfernandes thank you! the fouc shold be fixed now. I'll take a shot at fixing the latter issue tomorrow, probably in another PR. hero_fouc_fix.mov |
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.
Very impressive work @olarclara!
@@ -27,10 +27,56 @@ export const globalStyles = globalCss({ | |||
boxSizing: "border-box", | |||
}, | |||
|
|||
body: { | |||
// Remove when we replace scrollbar styling in the sandpack library |
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.
It should have been fixed in another PR, so feel free to remove it
id="container" | ||
style={ | ||
{ | ||
"--progress": isMounted ? progress : 0, |
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.
Love this approach
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.
it was @marcovincit's idea to define all the variables in a single place, it looks super neat and improves perceived performance 🙌🏽
This pull request is being automatically deployed with Vercel (learn more). sandpack-theme – ./website/theme🔍 Inspect: https://vercel.com/codesandbox/sandpack-theme/FhC1Fr6H51sgM435mm1mmVAd33V2 |
86ffb49
to
44f496e
Compare
Closes CSB-1862.
What kind of change does this pull request introduce?
Animates the landing page hero on desktop.
What is the current behavior?
What is the new behavior?
What steps did you take to test this? This is required before we can merge, make sure to test the flow you've updated.
Verified animations on the following browsers:
Performance verification on Chrome.
Checklist