From 32a18a38dd9e92da7e4fad48573d62086144821d Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Tue, 17 Mar 2026 07:42:11 +0100 Subject: [PATCH 1/4] Remove persisted styles when calling cancel() cancel() now removes inline styles that were persisted by Motion's style persistence mechanism, making the element revert to its pre-animation state. Previously, cancel() only cancelled the WAAPI animation but left the persisted inline style in place. Changes: - NativeAnimation.cancel(): removes persisted inline style via removeStyle() - NativeAnimation.stop(): calls native WAAPI cancel directly to preserve committed styles (stop should keep the current value) - JSAnimation.cancel(): resets state to "running" before tick(0) so the animation properly resets to its initial value when cancelled after finish - Added removeStyle() utility alongside existing setStyle() Fixes #2948 Co-Authored-By: Claude Opus 4.6 --- .../tests/animate-cancel-removes-styles.tsx | 40 +++++++ .../animate-cancel-removes-styles.ts | 11 ++ .../motion-dom/src/animation/JSAnimation.ts | 1 + .../src/animation/NativeAnimation.ts | 13 ++- .../__tests__/NativeAnimation.test.ts | 102 ++++++++++++++++++ .../motion-dom/src/render/dom/style-set.ts | 9 ++ tests/animate/animate.spec.ts | 8 +- 7 files changed, 178 insertions(+), 6 deletions(-) create mode 100644 dev/react/src/tests/animate-cancel-removes-styles.tsx create mode 100644 packages/framer-motion/cypress/integration/animate-cancel-removes-styles.ts diff --git a/dev/react/src/tests/animate-cancel-removes-styles.tsx b/dev/react/src/tests/animate-cancel-removes-styles.tsx new file mode 100644 index 0000000000..d2b1ecb0bd --- /dev/null +++ b/dev/react/src/tests/animate-cancel-removes-styles.tsx @@ -0,0 +1,40 @@ +import { useEffect, useRef, useState } from "react" +import { animate } from "framer-motion" + +export const App = () => { + const ref = useRef(null) + const [result, setResult] = useState("running") + + useEffect(() => { + if (!ref.current) return + + const animation = animate( + ref.current, + { opacity: 1 }, + { duration: 0.1 } + ) + + // Wait for animation to finish, then cancel + const timeout = setTimeout(() => { + const before = ref.current!.style.opacity + + animation.cancel() + + const after = ref.current!.style.opacity + + if (before === "1" && after === "") { + setResult("success") + } else { + setResult(`fail:before=${before},after=${after}`) + } + }, 500) + + return () => clearTimeout(timeout) + }, []) + + return ( +
+ {result} +
+ ) +} diff --git a/packages/framer-motion/cypress/integration/animate-cancel-removes-styles.ts b/packages/framer-motion/cypress/integration/animate-cancel-removes-styles.ts new file mode 100644 index 0000000000..552c22ee2d --- /dev/null +++ b/packages/framer-motion/cypress/integration/animate-cancel-removes-styles.ts @@ -0,0 +1,11 @@ +describe("animation.cancel() removes persisted styles", () => { + it("Removes persisted inline style when cancel is called after completion", () => { + cy.visit("?test=animate-cancel-removes-styles") + .wait(3000) + .get("#box") + .then(($el) => { + const text = $el.text() + expect(text).to.equal("success") + }) + }) +}) diff --git a/packages/motion-dom/src/animation/JSAnimation.ts b/packages/motion-dom/src/animation/JSAnimation.ts index b0a7a1e96a..c382a7496d 100644 --- a/packages/motion-dom/src/animation/JSAnimation.ts +++ b/packages/motion-dom/src/animation/JSAnimation.ts @@ -517,6 +517,7 @@ export class JSAnimation cancel() { this.holdTime = null this.startTime = 0 + this.state = "running" this.tick(0) this.teardown() this.options.onCancel?.() diff --git a/packages/motion-dom/src/animation/NativeAnimation.ts b/packages/motion-dom/src/animation/NativeAnimation.ts index 93f8201918..6671330b17 100644 --- a/packages/motion-dom/src/animation/NativeAnimation.ts +++ b/packages/motion-dom/src/animation/NativeAnimation.ts @@ -4,7 +4,7 @@ import { noop, secondsToMilliseconds, } from "motion-utils" -import { setStyle } from "../render/dom/style-set" +import { removeStyle, setStyle } from "../render/dom/style-set" import { supportsScrollTimeline } from "../utils/supports/scroll-timeline" import { getFinalKeyframe } from "./keyframes/get-final" import { @@ -148,6 +148,11 @@ export class NativeAnimation try { this.animation.cancel() } catch (e) {} + + const { element, name } = this.options || {} + if (element && name && !this.isPseudoElement) { + removeStyle(element, name) + } } stop() { @@ -165,7 +170,11 @@ export class NativeAnimation this.commitStyles() } - if (!this.isPseudoElement) this.cancel() + if (!this.isPseudoElement) { + try { + this.animation.cancel() + } catch (e) {} + } } /** diff --git a/packages/motion-dom/src/animation/__tests__/NativeAnimation.test.ts b/packages/motion-dom/src/animation/__tests__/NativeAnimation.test.ts index 517e464bd7..2ea2103153 100644 --- a/packages/motion-dom/src/animation/__tests__/NativeAnimation.test.ts +++ b/packages/motion-dom/src/animation/__tests__/NativeAnimation.test.ts @@ -1,4 +1,5 @@ import { motionValue } from "../../value" +import { NativeAnimation } from "../NativeAnimation" import { NativeAnimationExtended } from "../NativeAnimationExtended" /** @@ -9,6 +10,107 @@ import { NativeAnimationExtended } from "../NativeAnimationExtended" * the scheduled render could apply the correct value, causing a visual flash * back to the initial value. */ +describe("NativeAnimation - cancel removes persisted styles", () => { + let mockAnimation: any + + beforeEach(() => { + mockAnimation = { + cancel: jest.fn(), + onfinish: null, + playbackRate: 1, + currentTime: 300, + playState: "running", + effect: { + getComputedTiming: () => ({ duration: 300 }), + updateTiming: jest.fn(), + }, + } + + Element.prototype.animate = jest + .fn() + .mockImplementation(() => mockAnimation) + }) + + afterEach(() => { + ;(Element.prototype as any).animate = undefined + jest.restoreAllMocks() + }) + + test("cancel() removes persisted inline style after animation finishes", () => { + const element = document.createElement("div") + const mv = motionValue(0) + + const anim = new NativeAnimationExtended({ + element, + name: "opacity", + keyframes: [0, 1], + motionValue: mv, + finalKeyframe: 1, + onComplete: jest.fn(), + duration: 300, + ease: "easeOut", + } as any) + + // Simulate the WAAPI onfinish event firing + mockAnimation.onfinish?.() + + // After finish, inline style should be persisted + expect(element.style.opacity).toBe("1") + + // Now cancel - should remove the persisted style + anim.cancel() + expect(element.style.opacity).toBe("") + }) + + test("cancel() removes persisted inline style for CSS custom properties", () => { + const element = document.createElement("div") + const mv = motionValue(0) + + const anim = new NativeAnimationExtended({ + element, + name: "--my-color", + keyframes: ["red", "blue"], + motionValue: mv, + finalKeyframe: "blue", + onComplete: jest.fn(), + duration: 300, + ease: "easeOut", + } as any) + + // Simulate finish + mockAnimation.onfinish?.() + expect(element.style.getPropertyValue("--my-color")).toBe("blue") + + // Cancel should remove + anim.cancel() + expect(element.style.getPropertyValue("--my-color")).toBe("") + }) + + test("stop() preserves committed inline styles", () => { + const element = document.createElement("div") + document.body.appendChild(element) + + const anim = new NativeAnimation({ + element, + name: "opacity", + keyframes: [0, 1], + finalKeyframe: 1, + onComplete: jest.fn(), + duration: 300, + ease: "easeOut", + } as any) + + // Mock commitStyles to set inline style (simulating WAAPI behavior) + mockAnimation.commitStyles = jest.fn(() => { + element.style.opacity = "0.5" + }) + + // stop() should preserve the committed style + anim.stop() + expect(element.style.opacity).toBe("0.5") + }) +}) + describe("NativeAnimation - onfinish style commit", () => { let mockAnimation: any diff --git a/packages/motion-dom/src/render/dom/style-set.ts b/packages/motion-dom/src/render/dom/style-set.ts index b81b164d6f..ac17634772 100644 --- a/packages/motion-dom/src/render/dom/style-set.ts +++ b/packages/motion-dom/src/render/dom/style-set.ts @@ -10,3 +10,12 @@ export function setStyle( ? element.style.setProperty(name, value as string) : (element.style[name as any] = value as string) } + +export function removeStyle( + element: HTMLElement | SVGElement, + name: string +) { + isCSSVar(name) + ? element.style.removeProperty(name) + : (element.style[name as any] = "") +} diff --git a/tests/animate/animate.spec.ts b/tests/animate/animate.spec.ts index db2391424e..15e8b91d06 100644 --- a/tests/animate/animate.spec.ts +++ b/tests/animate/animate.spec.ts @@ -112,19 +112,19 @@ test.describe("animate() methods", () => { }) }) - test("cancel() after finish is a no-op", async ({ page }) => { + test("cancel() after finish removes persisted styles", async ({ page }) => { await waitForAnimation( "animate/animate-cancel-after-finish.html", page ) await eachBox(page, async (box) => { const id = await box.getAttribute("id") - // cancel() after finish should not revert — the final value is committed + // cancel() after finish should revert — removing persisted inline styles const boundingBox = await box.boundingBox() expect( boundingBox?.x, - `${id} should remain at final position after cancel` - ).toBeCloseTo(100) + `${id} should revert to original position after cancel` + ).toBeCloseTo(0) }) }) From 702c6ba05a6ac3f36694a9b3286ccfda565ff941 Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Tue, 5 May 2026 15:18:19 +0200 Subject: [PATCH 2/4] Revert to first keyframe on cancel instead of stripping styles Previous approach (32a18a38d) used removeStyle() to strip the inline property entirely on cancel. That throws away any value persisted by an earlier animation: // anim A: 0 -> 100 finishes, inline style = "100" // anim B: animates from 100, cancelled mid-flight // removeStyle clears inline -> falls back to CSS (0) // ^ wrong: should remain "100" Match JSAnimation.cancel()'s tick(0) semantics by writing the animation's first keyframe back to the element via updateMotionValue + setStyle. If the first keyframe is null (unresolved placeholder), leave the inline style alone. Adds a unit test that captures the multi-animation case the old approach broke. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../tests/animate-cancel-removes-styles.tsx | 40 ---------- .../animate-cancel-reverts-to-keyframe.tsx | 41 ++++++++++ .../animate-cancel-removes-styles.ts | 11 --- .../animate-cancel-reverts-to-keyframe.ts | 11 +++ .../src/animation/NativeAnimation.ts | 23 +++++- .../__tests__/NativeAnimation.test.ts | 76 ++++++++++++++----- .../motion-dom/src/render/dom/style-set.ts | 9 --- 7 files changed, 129 insertions(+), 82 deletions(-) delete mode 100644 dev/react/src/tests/animate-cancel-removes-styles.tsx create mode 100644 dev/react/src/tests/animate-cancel-reverts-to-keyframe.tsx delete mode 100644 packages/framer-motion/cypress/integration/animate-cancel-removes-styles.ts create mode 100644 packages/framer-motion/cypress/integration/animate-cancel-reverts-to-keyframe.ts diff --git a/dev/react/src/tests/animate-cancel-removes-styles.tsx b/dev/react/src/tests/animate-cancel-removes-styles.tsx deleted file mode 100644 index d2b1ecb0bd..0000000000 --- a/dev/react/src/tests/animate-cancel-removes-styles.tsx +++ /dev/null @@ -1,40 +0,0 @@ -import { useEffect, useRef, useState } from "react" -import { animate } from "framer-motion" - -export const App = () => { - const ref = useRef(null) - const [result, setResult] = useState("running") - - useEffect(() => { - if (!ref.current) return - - const animation = animate( - ref.current, - { opacity: 1 }, - { duration: 0.1 } - ) - - // Wait for animation to finish, then cancel - const timeout = setTimeout(() => { - const before = ref.current!.style.opacity - - animation.cancel() - - const after = ref.current!.style.opacity - - if (before === "1" && after === "") { - setResult("success") - } else { - setResult(`fail:before=${before},after=${after}`) - } - }, 500) - - return () => clearTimeout(timeout) - }, []) - - return ( -
- {result} -
- ) -} diff --git a/dev/react/src/tests/animate-cancel-reverts-to-keyframe.tsx b/dev/react/src/tests/animate-cancel-reverts-to-keyframe.tsx new file mode 100644 index 0000000000..2c2b308ba1 --- /dev/null +++ b/dev/react/src/tests/animate-cancel-reverts-to-keyframe.tsx @@ -0,0 +1,41 @@ +import { useEffect, useRef, useState } from "react" +import { animate } from "framer-motion" + +const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)) + +export const App = () => { + const ref = useRef(null) + const [result, setResult] = useState("running") + + useEffect(() => { + const el = ref.current + if (!el) return + ;(async () => { + await animate(el, { opacity: 1 }, { duration: 0.1 }).finished + const before = el.style.opacity + + await animate(el, { opacity: 0 }, { duration: 0.1 }).finished + const persisted = el.style.opacity + + // Cancel mid-flight must revert to this animation's start value + // (0, persisted by the previous animation), not strip the inline + // style entirely. + const c = animate(el, { opacity: 1 }, { duration: 0.5 }) + await wait(50) + c.cancel() + const after = el.style.opacity + + setResult( + before === "1" && persisted === "0" && after === "0" + ? "success" + : `fail:before=${before},persisted=${persisted},after=${after}` + ) + })() + }, []) + + return ( +
+ {result} +
+ ) +} diff --git a/packages/framer-motion/cypress/integration/animate-cancel-removes-styles.ts b/packages/framer-motion/cypress/integration/animate-cancel-removes-styles.ts deleted file mode 100644 index 552c22ee2d..0000000000 --- a/packages/framer-motion/cypress/integration/animate-cancel-removes-styles.ts +++ /dev/null @@ -1,11 +0,0 @@ -describe("animation.cancel() removes persisted styles", () => { - it("Removes persisted inline style when cancel is called after completion", () => { - cy.visit("?test=animate-cancel-removes-styles") - .wait(3000) - .get("#box") - .then(($el) => { - const text = $el.text() - expect(text).to.equal("success") - }) - }) -}) diff --git a/packages/framer-motion/cypress/integration/animate-cancel-reverts-to-keyframe.ts b/packages/framer-motion/cypress/integration/animate-cancel-reverts-to-keyframe.ts new file mode 100644 index 0000000000..335b5f621b --- /dev/null +++ b/packages/framer-motion/cypress/integration/animate-cancel-reverts-to-keyframe.ts @@ -0,0 +1,11 @@ +describe("animation.cancel() reverts to first keyframe", () => { + it("Reverts to value at start of animation, preserving prior persisted state", () => { + cy.visit("?test=animate-cancel-reverts-to-keyframe") + .wait(3000) + .get("#box") + .then(($el) => { + const text = $el.text() + expect(text).to.equal("success") + }) + }) +}) diff --git a/packages/motion-dom/src/animation/NativeAnimation.ts b/packages/motion-dom/src/animation/NativeAnimation.ts index 6671330b17..51706d9dcf 100644 --- a/packages/motion-dom/src/animation/NativeAnimation.ts +++ b/packages/motion-dom/src/animation/NativeAnimation.ts @@ -4,7 +4,7 @@ import { noop, secondsToMilliseconds, } from "motion-utils" -import { removeStyle, setStyle } from "../render/dom/style-set" +import { setStyle } from "../render/dom/style-set" import { supportsScrollTimeline } from "../utils/supports/scroll-timeline" import { getFinalKeyframe } from "./keyframes/get-final" import { @@ -149,10 +149,25 @@ export class NativeAnimation this.animation.cancel() } catch (e) {} - const { element, name } = this.options || {} - if (element && name && !this.isPseudoElement) { - removeStyle(element, name) + if (this.isPseudoElement) return + + const { element, name, keyframes } = this.options || {} + if (!element || !name || !keyframes) return + + /** + * Revert to the value at animation start so cancel matches + * JSAnimation's tick(0) behaviour. If the first keyframe is a + * null/undefined placeholder we don't know the resolved start + * value — leave the inline style alone rather than risk + * stripping a value persisted by an earlier animation. + */ + const initialValue = (keyframes as AnyResolvedKeyframe[])[0] + if (initialValue === null || initialValue === undefined) return + + if (this.updateMotionValue) { + this.updateMotionValue(initialValue as T) } + setStyle(element, name, initialValue) } stop() { diff --git a/packages/motion-dom/src/animation/__tests__/NativeAnimation.test.ts b/packages/motion-dom/src/animation/__tests__/NativeAnimation.test.ts index 2ea2103153..834363c66e 100644 --- a/packages/motion-dom/src/animation/__tests__/NativeAnimation.test.ts +++ b/packages/motion-dom/src/animation/__tests__/NativeAnimation.test.ts @@ -3,14 +3,12 @@ import { NativeAnimation } from "../NativeAnimation" import { NativeAnimationExtended } from "../NativeAnimationExtended" /** - * Tests for the Firefox opacity bug (issue #3552) where the WAAPI animation's - * onfinish handler needed to commit the final style to the element's inline - * styles before cancelling the animation. Without this, there was a timing - * window in Firefox where the WAAPI fill was removed (via cancel()) before - * the scheduled render could apply the correct value, causing a visual flash - * back to the initial value. + * cancel() must revert the element to the value at animation start so that + * its behaviour matches JSAnimation.cancel() (which calls tick(0) to render + * the first keyframe). Stripping the inline style entirely would lose any + * value that was persisted by an earlier animation. */ -describe("NativeAnimation - cancel removes persisted styles", () => { +describe("NativeAnimation - cancel reverts to first keyframe", () => { let mockAnimation: any beforeEach(() => { @@ -36,14 +34,14 @@ describe("NativeAnimation - cancel removes persisted styles", () => { jest.restoreAllMocks() }) - test("cancel() removes persisted inline style after animation finishes", () => { + test("cancel() reverts inline style to first keyframe after finish", () => { const element = document.createElement("div") const mv = motionValue(0) const anim = new NativeAnimationExtended({ element, name: "opacity", - keyframes: [0, 1], + keyframes: [0.25, 1], motionValue: mv, finalKeyframe: 1, onComplete: jest.fn(), @@ -51,18 +49,14 @@ describe("NativeAnimation - cancel removes persisted styles", () => { ease: "easeOut", } as any) - // Simulate the WAAPI onfinish event firing mockAnimation.onfinish?.() - - // After finish, inline style should be persisted expect(element.style.opacity).toBe("1") - // Now cancel - should remove the persisted style anim.cancel() - expect(element.style.opacity).toBe("") + expect(element.style.opacity).toBe("0.25") }) - test("cancel() removes persisted inline style for CSS custom properties", () => { + test("cancel() reverts CSS custom property to first keyframe", () => { const element = document.createElement("div") const mv = motionValue(0) @@ -77,13 +71,59 @@ describe("NativeAnimation - cancel removes persisted styles", () => { ease: "easeOut", } as any) - // Simulate finish mockAnimation.onfinish?.() expect(element.style.getPropertyValue("--my-color")).toBe("blue") - // Cancel should remove anim.cancel() - expect(element.style.getPropertyValue("--my-color")).toBe("") + expect(element.style.getPropertyValue("--my-color")).toBe("red") + }) + + test("cancel() preserves a value previously persisted by another animation", () => { + const element = document.createElement("div") + + // Earlier animation persisted opacity = "0.5". + element.style.opacity = "0.5" + + const mv = motionValue(0.5) + + const anim = new NativeAnimationExtended({ + element, + name: "opacity", + keyframes: [0.5, 1], + motionValue: mv, + finalKeyframe: 1, + onComplete: jest.fn(), + duration: 300, + ease: "easeOut", + } as any) + + anim.cancel() + // Must revert to 0.5 — the value before this animation ran — + // not strip the inline style. + expect(element.style.opacity).toBe("0.5") + }) + + test("cancel() leaves inline style alone when first keyframe is unresolved", () => { + const element = document.createElement("div") + element.style.opacity = "0.5" + + const mv = motionValue(0.5) + + const anim = new NativeAnimationExtended({ + element, + name: "opacity", + keyframes: [null, 1], + motionValue: mv, + finalKeyframe: 1, + onComplete: jest.fn(), + duration: 300, + ease: "easeOut", + } as any) + + anim.cancel() + // First keyframe is null — we don't know the start value, so the + // pre-existing inline style is preserved rather than overwritten. + expect(element.style.opacity).toBe("0.5") }) test("stop() preserves committed inline styles", () => { diff --git a/packages/motion-dom/src/render/dom/style-set.ts b/packages/motion-dom/src/render/dom/style-set.ts index ac17634772..b81b164d6f 100644 --- a/packages/motion-dom/src/render/dom/style-set.ts +++ b/packages/motion-dom/src/render/dom/style-set.ts @@ -10,12 +10,3 @@ export function setStyle( ? element.style.setProperty(name, value as string) : (element.style[name as any] = value as string) } - -export function removeStyle( - element: HTMLElement | SVGElement, - name: string -) { - isCSSVar(name) - ? element.style.removeProperty(name) - : (element.style[name as any] = "") -} From 018af8764cdadb7e92ea766725f419b65ee8471b Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Wed, 6 May 2026 10:08:09 +0200 Subject: [PATCH 3/4] Use holdTime=0 to revert JSAnimation on cancel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous cancel set state to "running" before calling tick(0) to bypass the "finished" clamp at JSAnimation.ts:245 (which would force currentTime to totalDuration when state==="finished"). That worked but read as a state-machine lie — the animation isn't running. Both the clamp at line 245 and the isAnimationFinished override at line 331 are gated on holdTime===null. Setting holdTime=0 bypasses both directly, so tick(0) renders the first keyframe without temporarily flipping state. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/motion-dom/src/animation/JSAnimation.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/motion-dom/src/animation/JSAnimation.ts b/packages/motion-dom/src/animation/JSAnimation.ts index c382a7496d..fa6aae997d 100644 --- a/packages/motion-dom/src/animation/JSAnimation.ts +++ b/packages/motion-dom/src/animation/JSAnimation.ts @@ -515,9 +515,15 @@ export class JSAnimation } cancel() { - this.holdTime = null + /** + * Hold currentTime at 0 so tick() renders the first keyframe. + * A non-null holdTime bypasses both the "finished" clamp (which + * would force currentTime to totalDuration) and the + * isAnimationFinished override (which would replace the value + * with getFinalKeyframe). + */ + this.holdTime = 0 this.startTime = 0 - this.state = "running" this.tick(0) this.teardown() this.options.onCancel?.() From c5b34f1533797d61bfa0eef5d92c9898d78ff4be Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Wed, 6 May 2026 10:24:57 +0200 Subject: [PATCH 4/4] Condense JSAnimation.cancel comment Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/motion-dom/src/animation/JSAnimation.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/motion-dom/src/animation/JSAnimation.ts b/packages/motion-dom/src/animation/JSAnimation.ts index fa6aae997d..2eed5c9996 100644 --- a/packages/motion-dom/src/animation/JSAnimation.ts +++ b/packages/motion-dom/src/animation/JSAnimation.ts @@ -515,13 +515,7 @@ export class JSAnimation } cancel() { - /** - * Hold currentTime at 0 so tick() renders the first keyframe. - * A non-null holdTime bypasses both the "finished" clamp (which - * would force currentTime to totalDuration) and the - * isAnimationFinished override (which would replace the value - * with getFinalKeyframe). - */ + // holdTime=0 (not null) so tick() doesn't clamp to totalDuration when state is "finished". this.holdTime = 0 this.startTime = 0 this.tick(0)