Skip to content

Commit e02c902

Browse files
authored
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
1 parent 20e6508 commit e02c902

File tree

3 files changed

+29
-10
lines changed

3 files changed

+29
-10
lines changed

.changeset/eleven-donuts-sit.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: make animations more robust to quick shuffling

packages/svelte/src/internal/client/dom/elements/transitions.js

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export function animation(element, get_fn, get_params) {
7575
/** @type {import('#client').Animation | undefined} */
7676
var animation;
7777

78-
/** @type {null | { position: string, width: string, height: string }} */
78+
/** @type {null | { position: string, width: string, height: string, transform: string }} */
7979
var original_styles = null;
8080

8181
item.a ??= {
@@ -110,20 +110,29 @@ export function animation(element, get_fn, get_params) {
110110
}
111111
},
112112
fix() {
113-
var computed_style = getComputedStyle(element);
113+
// If an animation is already running, transforming the element is likely to fail,
114+
// because the styles applied by the animation take precedence. In the case of crossfade,
115+
// that means the `translate(...)` of the crossfade transition overrules the `translate(...)`
116+
// we would apply below, leading to the element jumping somewhere to the top left.
117+
if (element.getAnimations().length) return;
114118

115-
if (computed_style.position !== 'absolute' && computed_style.position !== 'fixed') {
119+
// It's important to destructure these to get fixed values - the object itself has getters,
120+
// and changing the style to 'absolute' can for example influence the width.
121+
var { position, width, height } = getComputedStyle(element);
122+
123+
if (position !== 'absolute' && position !== 'fixed') {
116124
var style = /** @type {HTMLElement | SVGElement} */ (element).style;
117125

118126
original_styles = {
119127
position: style.position,
120128
width: style.width,
121-
height: style.height
129+
height: style.height,
130+
transform: style.transform
122131
};
123132

124133
style.position = 'absolute';
125-
style.width = computed_style.width;
126-
style.height = computed_style.height;
134+
style.width = width;
135+
style.height = height;
127136
var to = element.getBoundingClientRect();
128137

129138
if (from.left !== to.left || from.top !== to.top) {
@@ -139,6 +148,7 @@ export function animation(element, get_fn, get_params) {
139148
style.position = original_styles.position;
140149
style.width = original_styles.width;
141150
style.height = original_styles.height;
151+
style.transform = original_styles.transform;
142152
}
143153
}
144154
};

packages/svelte/tests/animation-helpers.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ function tick(time) {
3232
}
3333

3434
class Animation {
35-
#target;
3635
#keyframes;
3736
#duration;
3837
#delay;
@@ -42,6 +41,7 @@ class Animation {
4241
#finished = () => {};
4342
#cancelled = () => {};
4443

44+
target;
4545
currentTime = 0;
4646
startTime = 0;
4747

@@ -51,7 +51,7 @@ class Animation {
5151
* @param {{ duration: number, delay: number }} options
5252
*/
5353
constructor(target, keyframes, { duration, delay }) {
54-
this.#target = target;
54+
this.target = target;
5555
this.#keyframes = keyframes;
5656
this.#duration = duration;
5757
this.#delay = delay ?? 0;
@@ -111,14 +111,14 @@ class Animation {
111111

112112
for (let prop in frame) {
113113
// @ts-ignore
114-
this.#target.style[prop] = frame[prop];
114+
this.target.style[prop] = frame[prop];
115115
}
116116

117117
if (this.currentTime >= this.#duration) {
118118
this.currentTime = this.#duration;
119119
for (let prop in frame) {
120120
// @ts-ignore
121-
this.#target.style[prop] = null;
121+
this.target.style[prop] = null;
122122
}
123123
}
124124
}
@@ -181,3 +181,7 @@ HTMLElement.prototype.animate = function (keyframes, options) {
181181
// @ts-ignore
182182
return animation;
183183
};
184+
185+
HTMLElement.prototype.getAnimations = function () {
186+
return Array.from(raf.animations).filter((animation) => animation.target === this);
187+
};

0 commit comments

Comments
 (0)