From e02c90209b81d047f98f930456264053fa9b33f5 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Fri, 19 Jul 2024 13:28:25 +0200 Subject: [PATCH] fix: make animations more robust to quick shuffling (#12496) Previously, if transitions/animations were playing in quick succession, overlapping each other, it could have disastrous outcomes, leading to elements jumping all over the place. This PR gets that into much better state (not completely fixed, but close) by applying a few fixes: - destructure style object from `getComputedStyles`, because it's a live object with getters and we're interested in the fixed values at the beginning - `unfix` for animations didn't reset the transition styles - don't apply `fix` when we detect already-running animations on the element. That means it's already away from its original position, and doesn't need fixing. Worse, applying an absolute position can lead to the element jumping to the top left if the running animation also applies a transition style - those take precedence over the one we would apply fixes #10252 --- .changeset/eleven-donuts-sit.md | 5 +++++ .../client/dom/elements/transitions.js | 22 ++++++++++++++----- packages/svelte/tests/animation-helpers.js | 12 ++++++---- 3 files changed, 29 insertions(+), 10 deletions(-) create mode 100644 .changeset/eleven-donuts-sit.md diff --git a/.changeset/eleven-donuts-sit.md b/.changeset/eleven-donuts-sit.md new file mode 100644 index 0000000000..b372cc1983 --- /dev/null +++ b/.changeset/eleven-donuts-sit.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: make animations more robust to quick shuffling diff --git a/packages/svelte/src/internal/client/dom/elements/transitions.js b/packages/svelte/src/internal/client/dom/elements/transitions.js index a0aabd92dc..3369e263f3 100644 --- a/packages/svelte/src/internal/client/dom/elements/transitions.js +++ b/packages/svelte/src/internal/client/dom/elements/transitions.js @@ -75,7 +75,7 @@ export function animation(element, get_fn, get_params) { /** @type {import('#client').Animation | undefined} */ var animation; - /** @type {null | { position: string, width: string, height: string }} */ + /** @type {null | { position: string, width: string, height: string, transform: string }} */ var original_styles = null; item.a ??= { @@ -110,20 +110,29 @@ export function animation(element, get_fn, get_params) { } }, fix() { - var computed_style = getComputedStyle(element); + // If an animation is already running, transforming the element is likely to fail, + // because the styles applied by the animation take precedence. In the case of crossfade, + // that means the `translate(...)` of the crossfade transition overrules the `translate(...)` + // we would apply below, leading to the element jumping somewhere to the top left. + if (element.getAnimations().length) return; - if (computed_style.position !== 'absolute' && computed_style.position !== 'fixed') { + // It's important to destructure these to get fixed values - the object itself has getters, + // and changing the style to 'absolute' can for example influence the width. + var { position, width, height } = getComputedStyle(element); + + if (position !== 'absolute' && position !== 'fixed') { var style = /** @type {HTMLElement | SVGElement} */ (element).style; original_styles = { position: style.position, width: style.width, - height: style.height + height: style.height, + transform: style.transform }; style.position = 'absolute'; - style.width = computed_style.width; - style.height = computed_style.height; + style.width = width; + style.height = height; var to = element.getBoundingClientRect(); if (from.left !== to.left || from.top !== to.top) { @@ -139,6 +148,7 @@ export function animation(element, get_fn, get_params) { style.position = original_styles.position; style.width = original_styles.width; style.height = original_styles.height; + style.transform = original_styles.transform; } } }; diff --git a/packages/svelte/tests/animation-helpers.js b/packages/svelte/tests/animation-helpers.js index e7c27721ef..809d3e9487 100644 --- a/packages/svelte/tests/animation-helpers.js +++ b/packages/svelte/tests/animation-helpers.js @@ -32,7 +32,6 @@ function tick(time) { } class Animation { - #target; #keyframes; #duration; #delay; @@ -42,6 +41,7 @@ class Animation { #finished = () => {}; #cancelled = () => {}; + target; currentTime = 0; startTime = 0; @@ -51,7 +51,7 @@ class Animation { * @param {{ duration: number, delay: number }} options */ constructor(target, keyframes, { duration, delay }) { - this.#target = target; + this.target = target; this.#keyframes = keyframes; this.#duration = duration; this.#delay = delay ?? 0; @@ -111,14 +111,14 @@ class Animation { for (let prop in frame) { // @ts-ignore - this.#target.style[prop] = frame[prop]; + this.target.style[prop] = frame[prop]; } if (this.currentTime >= this.#duration) { this.currentTime = this.#duration; for (let prop in frame) { // @ts-ignore - this.#target.style[prop] = null; + this.target.style[prop] = null; } } } @@ -181,3 +181,7 @@ HTMLElement.prototype.animate = function (keyframes, options) { // @ts-ignore return animation; }; + +HTMLElement.prototype.getAnimations = function () { + return Array.from(raf.animations).filter((animation) => animation.target === this); +};