-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: useAnimate respects MotionConfig.skipAnimations #3680
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import "@testing-library/jest-dom" | ||
| import { render } from "@testing-library/react" | ||
| import { useEffect } from "react" | ||
| import { MotionConfig } from "../../../components/MotionConfig" | ||
| import { useAnimate } from "../use-animate" | ||
|
|
||
| describe("useAnimate", () => { | ||
|
|
@@ -115,4 +116,39 @@ describe("useAnimate", () => { | |
|
|
||
| expect(frameCount).toEqual(3) | ||
| }) | ||
|
|
||
| test("Skips animations when MotionConfig skipAnimations is true", () => { | ||
| return new Promise<void>((resolve) => { | ||
| const Component = () => { | ||
| const [scope, animate] = useAnimate() | ||
|
|
||
| useEffect(() => { | ||
| const animation = animate( | ||
| scope.current, | ||
| { opacity: 0.5 }, | ||
| { duration: 1 } | ||
| ) | ||
|
|
||
| // Animation should not be tracked in scope | ||
| expect(scope.animations.length).toBe(0) | ||
|
|
||
| animation.then(() => { | ||
| // Element style should not be changed | ||
| expect(scope.current).not.toHaveStyle( | ||
| "opacity: 0.5;" | ||
| ) | ||
|
Comment on lines
+137
to
+139
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This assertion encodes the inconsistency flagged in the implementation: the test expects the element NOT to have |
||
| resolve() | ||
| }) | ||
| }) | ||
|
|
||
| return <div ref={scope} /> | ||
| } | ||
|
|
||
| render( | ||
| <MotionConfig skipAnimations> | ||
| <Component /> | ||
| </MotionConfig> | ||
| ) | ||
| }) | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,10 +1,11 @@ | ||||||||||||||||||||||
| "use client" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| import { useMemo } from "react" | ||||||||||||||||||||||
| import { AnimationScope } from "motion-dom" | ||||||||||||||||||||||
| import { useContext, useMemo } from "react" | ||||||||||||||||||||||
| import { AnimationScope, GroupAnimationWithThen } from "motion-dom" | ||||||||||||||||||||||
| import { useConstant } from "../../utils/use-constant" | ||||||||||||||||||||||
| import { useUnmountEffect } from "../../utils/use-unmount-effect" | ||||||||||||||||||||||
| import { useReducedMotionConfig } from "../../utils/reduced-motion/use-reduced-motion-config" | ||||||||||||||||||||||
| import { MotionConfigContext } from "../../context/MotionConfigContext" | ||||||||||||||||||||||
| import { createScopedAnimate } from "../animate" | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export function useAnimate<T extends Element = any>() { | ||||||||||||||||||||||
|
|
@@ -13,11 +14,15 @@ export function useAnimate<T extends Element = any>() { | |||||||||||||||||||||
| animations: [], | ||||||||||||||||||||||
| })) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const { skipAnimations } = useContext(MotionConfigContext) | ||||||||||||||||||||||
| const reduceMotion = useReducedMotionConfig() ?? undefined | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const animate = useMemo( | ||||||||||||||||||||||
| () => createScopedAnimate({ scope, reduceMotion }), | ||||||||||||||||||||||
| [scope, reduceMotion] | ||||||||||||||||||||||
| () => | ||||||||||||||||||||||
| skipAnimations | ||||||||||||||||||||||
| ? createNoopAnimate(scope) | ||||||||||||||||||||||
| : createScopedAnimate({ scope, reduceMotion }), | ||||||||||||||||||||||
| [scope, reduceMotion, skipAnimations] | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| useUnmountEffect(() => { | ||||||||||||||||||||||
|
|
@@ -27,3 +32,16 @@ export function useAnimate<T extends Element = any>() { | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| return [scope, animate] as [AnimationScope<T>, typeof animate] | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * When skipAnimations is true, return an animate function that resolves | ||||||||||||||||||||||
| * immediately without creating any WAAPI animations. This prevents | ||||||||||||||||||||||
| * browsers (particularly WebKit) from reporting running animations via | ||||||||||||||||||||||
| * element.getAnimations(), which can break tools like Playwright that | ||||||||||||||||||||||
| * check element stability. | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| function createNoopAnimate<T extends Element>(scope: AnimationScope<T>) { | ||||||||||||||||||||||
| return ((..._args: any[]) => { | ||||||||||||||||||||||
| return new GroupAnimationWithThen([]) | ||||||||||||||||||||||
| }) as ReturnType<typeof createScopedAnimate> | ||||||||||||||||||||||
|
Comment on lines
+43
to
+46
|
||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+43
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Concrete failure: calling A correct implementation should call
Comment on lines
+43
to
+47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
|
||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test asserts that when
skipAnimationsis true the element style is not updated. However,skipAnimationsis documented/covered elsewhere as "values will be set instantly" (seeMotionConfigContextandcomponents/MotionConfig/__tests__/index.test.tsx). ForuseAnimate, it would be more consistent to apply the final target state synchronously (without creating WAAPI animations) and still resolve immediately, and update this expectation accordingly.