Skip to content

Commit

Permalink
fix: make animations more robust to quick shuffling
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dummdidumm committed Jul 18, 2024
1 parent ea22840 commit 9c6f2a9
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changeset/eleven-donuts-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: make animations more robust to quick shuffling
22 changes: 16 additions & 6 deletions packages/svelte/src/internal/client/dom/elements/transitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ??= {
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
}
};
Expand Down
12 changes: 8 additions & 4 deletions packages/svelte/tests/animation-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ function tick(time) {
}

class Animation {
#target;
#keyframes;
#duration;
#delay;
Expand All @@ -42,6 +41,7 @@ class Animation {
#finished = () => {};
#cancelled = () => {};

target;
currentTime = 0;
startTime = 0;

Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
}
}
Expand Down Expand Up @@ -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);
};

0 comments on commit 9c6f2a9

Please sign in to comment.