-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
fix: use WAAPI to control timing of JS-based animations #13018
Conversation
🦋 Changeset detectedLatest commit: 51e3342 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
This could use some manual testing, but it feels quite a bit simpler than what we had before |
Ok, this is way better than the status quo. Svelte 4 — discontinuities all over the place when you start spamming the toggle button: Screen.Recording.2024-08-26.at.10.03.14.PM.movSvelte 5 is somehow worse: Screen.Recording.2024-08-26.at.10.04.27.PM.movThis PR — smooth as butter, and of course you can use devtools to slow the animation down if you need to: Screen.Recording.2024-08-26.at.10.05.36.PM.mov |
This is great! |
packages/svelte/src/internal/client/dom/elements/transitions.js
Outdated
Show resolved
Hide resolved
packages/svelte/src/internal/client/dom/elements/transitions.js
Outdated
Show resolved
Hide resolved
This still needs a changeset and I had one question why a comment was removed, but code-wise it looks great |
Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Follow-up to #12883.
Fixes #12730
Fixes #13019
Currently we assume that a transition can have a
css
method or atick
method but not both. #12883 allows both, but insists on at least one. In fact, we need to support all four cases —css
,tick
,css
+tick
and neither.In #12883 (comment) I proposed always creating a
task
and putting the completion logic in there, but that's no good — we don't want to create atask
if there's notick
method, because that would create extra power-consuming work.This PR flips it on its head. Here, we always create a Web Animation, but we only provide it with keyframes if
css
is provided. If there's atick
method, the loop gets the current time from the animation, rather than from therequestAnimationFrame
callback argument, and when the animation completes we calltick?.(t2, 1 - t2)
.This gives us something I've wanted for a long time:
By using a primitive that the browser deeply understands, we can do stuff like slow the transition down using devtools, and without causing callbacks to be called prematurely — even when using JS transitions:
Screen.Recording.2024-08-24.at.9.46.11.PM.mov
Right now a bunch of tests fail. Since our test suite uses a bootleg
Animation
shim it's hard to tell which failures are legit (if any) and which just need some finagling — will look into it more tomorrow.Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint