-
Notifications
You must be signed in to change notification settings - Fork 21
fix(components): prevent animation functions from being executed on server side in post-header and post-collapsible
#6696
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
base: main
Are you sure you want to change the base?
Conversation
…e finish function of the burgerMenuAnimation in the post-header component
…ards in functions which are not ssr compatible
🦋 Changeset detectedLatest commit: 3fc72b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
0adf67f to
c56572b
Compare
…favor of SSR compatibility
…rown-by-post-main-navigation
packages/components/src/components/post-collapsible/post-collapsible.tsx
Outdated
Show resolved
Hide resolved
| if (!this.isLoaded || isMotionReduced()) animation.finish(); | ||
|
|
||
| await animation.finished; | ||
| if (animation && animation.finished) { |
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.
animation is now always defined
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.
Sadly it's not during server side rendering.
Stencils hydrate app is mocking a lot of browser specific things (e.g. window, etc.), so it is able to pre-render the components on the server.
But it it not mocking the HTMLElement Animation API.
And adding a IS_SERVER guard did not help at all.
I'm still trying to figure out how we can detect that, because const IS_SERVER = typeof window === 'undefined'; does not work, since we updated some packages to their latest versions.
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.
Instead of checking the animation, I fixed the IS_BROWSER and IS_SERVER utility flags (it only took me 2 days 😢) and used them in the component. This way it's clearer for devs, why this check is necessary.
…rown-by-post-main-navigation
c3f03fb to
0330c02
Compare
0330c02 to
96b0c66
Compare
…rown-by-post-main-navigation
…rown-by-post-main-navigation # Conflicts: # packages/components/src/components.d.ts
|



📄 Description
post-header, so we don't call accidentally thefinishfunction on a not defind animation object.post-collapsiblecomponent, so we can't accidentally execute the following logics:post-collapsible-triggerattributes frompost-collapsibleis impossible.post-collapsibletoggle function, by improving the behavior when user perfers reduced motion.🚀 Demo
If applicable, please add a screenshot or video to illustrate the changes.
🔮 Design review
📝 Checklist