From 95fa0f122d2ea5e5a8f8c6c9bad73360fa5b04d4 Mon Sep 17 00:00:00 2001 From: Sean Monahan Date: Fri, 15 Nov 2024 17:01:03 -0800 Subject: [PATCH 1/4] fix: handle cases when Animation.persist() does not exist The Animation.persist() API is in our full suport matrix but was added after browsers in our partial support matrix. As no polyfill exists for this feature and it cannot be compiled away this commit explicitly handles the case when the API does not exist. When .persist() is not available we short-circuit the animation, using the existing "is reduced motion" code path to make the animation effectively instant while still firing all animation events, etc. --- .../factories/createMotionComponent.test.tsx | 39 ++++++++++++++++++ .../createPresenceComponent.test.tsx | 40 +++++++++++++++++++ .../library/src/hooks/useAnimateAtoms.ts | 7 +++- 3 files changed, 84 insertions(+), 2 deletions(-) diff --git a/packages/react-components/react-motion/library/src/factories/createMotionComponent.test.tsx b/packages/react-components/react-motion/library/src/factories/createMotionComponent.test.tsx index a2508262d2f209..7aa5ce73c5f768 100644 --- a/packages/react-components/react-motion/library/src/factories/createMotionComponent.test.tsx +++ b/packages/react-components/react-motion/library/src/factories/createMotionComponent.test.tsx @@ -36,6 +36,27 @@ function createElementMock() { } describe('createMotionComponent', () => { + let hasAnimation: boolean; + beforeEach(() => { + if (!global.Animation) { + hasAnimation = false; + global.Animation = { + // @ts-expect-error mock + prototype: { + persist: jest.fn(), + }, + }; + } else { + hasAnimation = true; + } + }); + + afterEach(() => { + if (!hasAnimation) { + // @ts-expect-error mock + delete global.Animation; + } + }); it('creates a motion and plays it', () => { const TestAtom = createMotionComponent(motion); const { animateMock, ElementMock } = createElementMock(); @@ -52,6 +73,24 @@ describe('createMotionComponent', () => { }); }); + it('creates a motion and plays it (without .persist())', () => { + // @ts-expect-error mock + delete global.Animation.prototype.persist; + const TestAtom = createMotionComponent(motion); + const { animateMock, ElementMock } = createElementMock(); + + render( + + + , + ); + + expect(animateMock).toHaveBeenCalledWith(motion.keyframes, { + duration: 1, + fill: 'forwards', + }); + }); + it('supports functions as motion definitions', () => { const fnMotion = jest.fn().mockImplementation(() => motion); const TestAtom = createMotionComponent(fnMotion); diff --git a/packages/react-components/react-motion/library/src/factories/createPresenceComponent.test.tsx b/packages/react-components/react-motion/library/src/factories/createPresenceComponent.test.tsx index ad2cedcacd6314..a1cd9bb2df262a 100644 --- a/packages/react-components/react-motion/library/src/factories/createPresenceComponent.test.tsx +++ b/packages/react-components/react-motion/library/src/factories/createPresenceComponent.test.tsx @@ -41,6 +41,28 @@ function createElementMock() { } describe('createPresenceComponent', () => { + let hasAnimation: boolean; + beforeEach(() => { + if (!global.Animation) { + hasAnimation = false; + global.Animation = { + // @ts-expect-error mock + prototype: { + persist: jest.fn(), + }, + }; + } else { + hasAnimation = true; + } + }); + + afterEach(() => { + if (!hasAnimation) { + // @ts-expect-error mock + delete global.Animation; + } + }); + describe('appear', () => { it('does not animate by default', () => { const TestPresence = createPresenceComponent(motion); @@ -68,6 +90,24 @@ describe('createPresenceComponent', () => { expect(animateMock).toHaveBeenCalledWith(enterKeyframes, options); }); + it('animates when is "true" (without .persist())', () => { + // @ts-expect-error mock + delete window.Animation.prototype.persist; + const TestPresence = createPresenceComponent(motion); + const { animateMock, ElementMock } = createElementMock(); + + render( + + + , + ); + + expect(animateMock).toHaveBeenCalledWith(enterKeyframes, { + ...options, + duration: 1, + }); + }); + it('finishes motion when wrapped in motion behaviour context with skip behaviour', async () => { const TestPresence = createPresenceComponent(motion); const { finishMock, ElementMock } = createElementMock(); diff --git a/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts b/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts index 66ea0c35e1b6be..6fa1294484363f 100644 --- a/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts +++ b/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts @@ -2,6 +2,9 @@ import * as React from 'react'; import type { AnimationHandle, AtomMotion } from '../types'; function useAnimateAtomsInSupportedEnvironment() { + // eslint-disable-next-line @nx/workspace-no-restricted-globals + const SUPPORTS_PERSIST = typeof window !== 'undefined' && typeof window.Animation?.prototype.persist === 'function'; + return React.useCallback( ( element: HTMLElement, @@ -19,10 +22,10 @@ function useAnimateAtomsInSupportedEnvironment() { fill: 'forwards', ...params, - ...(isReducedMotion && { duration: 1 }), + ...((isReducedMotion || !SUPPORTS_PERSIST) && { duration: 1 }), }); - animation.persist(); + SUPPORTS_PERSIST && animation.persist(); return animation; }); From 7ff548635f43a3438af232ef3f91af3bcb057a76 Mon Sep 17 00:00:00 2001 From: Sean Monahan Date: Fri, 15 Nov 2024 17:07:41 -0800 Subject: [PATCH 2/4] change file --- ...-react-motion-f4f8b668-785c-48d9-9d8f-2410472b12f4.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-motion-f4f8b668-785c-48d9-9d8f-2410472b12f4.json diff --git a/change/@fluentui-react-motion-f4f8b668-785c-48d9-9d8f-2410472b12f4.json b/change/@fluentui-react-motion-f4f8b668-785c-48d9-9d8f-2410472b12f4.json new file mode 100644 index 00000000000000..81ed563ea2612c --- /dev/null +++ b/change/@fluentui-react-motion-f4f8b668-785c-48d9-9d8f-2410472b12f4.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: handle case when Animation.persist() does not exist", + "packageName": "@fluentui/react-motion", + "email": "seanmonahan@microsoft.com", + "dependentChangeType": "patch" +} From 080f10cc74d0600f35ccf788460a89ec49f44e1d Mon Sep 17 00:00:00 2001 From: Sean Monahan Date: Tue, 3 Dec 2024 10:19:25 -0800 Subject: [PATCH 3/4] PR feedback --- .../library/src/factories/createMotionComponent.test.tsx | 2 +- .../src/factories/createPresenceComponent.test.tsx | 2 +- .../react-motion/library/src/hooks/useAnimateAtoms.ts | 9 +++++++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/react-components/react-motion/library/src/factories/createMotionComponent.test.tsx b/packages/react-components/react-motion/library/src/factories/createMotionComponent.test.tsx index 7aa5ce73c5f768..0a69a75d2f23fb 100644 --- a/packages/react-components/react-motion/library/src/factories/createMotionComponent.test.tsx +++ b/packages/react-components/react-motion/library/src/factories/createMotionComponent.test.tsx @@ -86,7 +86,7 @@ describe('createMotionComponent', () => { ); expect(animateMock).toHaveBeenCalledWith(motion.keyframes, { - duration: 1, + duration: 500, fill: 'forwards', }); }); diff --git a/packages/react-components/react-motion/library/src/factories/createPresenceComponent.test.tsx b/packages/react-components/react-motion/library/src/factories/createPresenceComponent.test.tsx index a1cd9bb2df262a..c00775a85e6725 100644 --- a/packages/react-components/react-motion/library/src/factories/createPresenceComponent.test.tsx +++ b/packages/react-components/react-motion/library/src/factories/createPresenceComponent.test.tsx @@ -104,7 +104,7 @@ describe('createPresenceComponent', () => { expect(animateMock).toHaveBeenCalledWith(enterKeyframes, { ...options, - duration: 1, + duration: 500, }); }); diff --git a/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts b/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts index 6fa1294484363f..74ebd6fc42441a 100644 --- a/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts +++ b/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts @@ -22,10 +22,15 @@ function useAnimateAtomsInSupportedEnvironment() { fill: 'forwards', ...params, - ...((isReducedMotion || !SUPPORTS_PERSIST) && { duration: 1 }), + ...(isReducedMotion && { duration: 1 }), }); - SUPPORTS_PERSIST && animation.persist(); + if (SUPPORTS_PERSIST) { + animation.persist(); + } else { + const resultKeyframe = keyframes[keyframes.length - 1]; + Object.assign(element.style ?? {}, resultKeyframe); + } return animation; }); From 473fea1cdfdeae0080a7540e942b1af2dd9d84eb Mon Sep 17 00:00:00 2001 From: Sean Monahan Date: Tue, 3 Dec 2024 13:43:38 -0800 Subject: [PATCH 4/4] fix lint --- .../react-motion/library/src/hooks/useAnimateAtoms.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts b/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts index 74ebd6fc42441a..b4250d855f664e 100644 --- a/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts +++ b/packages/react-components/react-motion/library/src/hooks/useAnimateAtoms.ts @@ -81,7 +81,7 @@ function useAnimateAtomsInSupportedEnvironment() { }, }; }, - [], + [SUPPORTS_PERSIST], ); }