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

Split JS and CSS loading of the polyfill #217

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bramus
Copy link
Collaborator

@bramus bramus commented Jan 28, 2024

Fixes #160.

Not sure if we do want to have this though, as it will leave Firefox Nightly somewhat useless state for CSS-based SDA’s.

With the changes in this PR, the polyfill won’t kick for the CSS part but will for the JS part in Firefox Nightly. However, because Firefox Nightly supports an older CSS syntax (e.g. using vertical instead of y, or requiring an time-based animation-duration) this will leave CSS Scroll-Driven Animations in a broken state.

One thing I do like about this PR is the code restructure that splits off the JS plugging into scroll-timeline-js.js.

@bramus bramus marked this pull request as draft January 28, 2024 13:58
@bramus
Copy link
Collaborator Author

bramus commented Jan 28, 2024

Converting to “draft” state to prevent accidental merges. Would love to get feedback on the situation described above though.

@bramus
Copy link
Collaborator Author

bramus commented Jan 28, 2024

RE Firefox Nightly's half-broken state: if the browser claims support then the polyfill should accept this as true. If their implementation is buggy, then they should fix that.

@bramus bramus marked this pull request as ready for review February 5, 2024 11:04
@@ -5,7 +5,7 @@ import { ScrollTimeline, ViewTimeline, getScrollParent, calculateRange,

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until we can get rid of the ScrollTimeline, ViewTimeline and ProxyAnimation imports I think the css polyfill will only work if the js polyfill was loaded. E.g. we use whether the returned animation is a ProxyAnimation to determine whether we need to set up a css scroll driven animation: https://github.com/flackr/scroll-timeline/blob/master/src/scroll-timeline-css.js#L157

We're similarly dependent on getAnimations being overridden to return the ProxyAnimation instances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Polyfill doesn't define ScrollTimeline at all.
2 participants