Skip to content

Commit ba40af9

Browse files
ydanivydaniv
authored
Fix AnimationGroup.progress() with infinite duration (#53)
* Implement viewEnter types alternate/repeat/state * Fix cleanup of exit observer only if necessary * Fix AnimationGroup progress() when duration is infinite * Fix progress() to use progress * Exclude dist from testing * Fix progress() again and update tests * Also consider iterations
1 parent dba5918 commit ba40af9

5 files changed

Lines changed: 38 additions & 78 deletions

File tree

‎packages/interact/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@
5555
"url": "https://github.com/wix-incubator/interact/issues"
5656
},
5757
"dependencies": {
58-
"@wix/motion": "^1.0.0",
58+
"@wix/motion": "^2.0.0",
5959
"fastdom": "^1.0.12",
6060
"fizban": "^0.7.2",
6161
"kuliso": "^0.4.13"

‎packages/motion/src/AnimationGroup.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,9 @@ export class AnimationGroup {
6666

6767
progress(p: number) {
6868
for (const animation of this.animations) {
69-
const { activeDuration } = animation.effect!.getComputedTiming();
70-
const { delay } = animation.effect!.getTiming();
71-
animation.currentTime =
72-
((delay || 0) + ((activeDuration as number) || 0)) * p;
69+
const { delay, duration, iterations } = animation.effect!.getTiming();
70+
const time = (Number.isFinite(duration) ? duration as number : 0) * (Number.isFinite(iterations) ? iterations as number : 1);
71+
animation.currentTime = ((delay || 0) + time) * p;
7372
}
7473
}
7574

‎packages/motion/test/AnimationGroup.spec.ts‎

Lines changed: 30 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ const createMockAnimation = (overrides: Partial<Animation> = {}): Animation =>
1515
effect: {
1616
getComputedTiming: vi.fn().mockReturnValue({
1717
progress: 0.5,
18-
activeDuration: 1000,
1918
}),
2019
getTiming: vi.fn().mockReturnValue({
2120
delay: 0,
21+
duration: 1000,
2222
}),
2323
} as any,
2424
play: vi.fn(),
@@ -165,18 +165,14 @@ describe('AnimationGroup', () => {
165165
test('should return progress from first animation effect', () => {
166166
const mockAnimation1 = createMockAnimation({
167167
effect: {
168-
getComputedTiming: vi.fn().mockReturnValue({
169-
progress: 0.75,
170-
}),
171-
getTiming: vi.fn().mockReturnValue({ delay: 0 }),
168+
getComputedTiming: vi.fn().mockReturnValue({ progress: 0.75 }),
169+
getTiming: vi.fn().mockReturnValue({ delay: 0, duration: 1000 }),
172170
} as any,
173171
});
174172
const mockAnimation2 = createMockAnimation({
175173
effect: {
176-
getComputedTiming: vi.fn().mockReturnValue({
177-
progress: 0.25, // Different progress - should not be used
178-
}),
179-
getTiming: vi.fn().mockReturnValue({ delay: 0 }),
174+
getComputedTiming: vi.fn().mockReturnValue({ progress: 0.25 }),
175+
getTiming: vi.fn().mockReturnValue({ delay: 0, duration: 1000 }),
180176
} as any,
181177
});
182178

@@ -230,10 +226,8 @@ describe('AnimationGroup', () => {
230226
testCases.forEach(({ inputProgress, expectedProgress }) => {
231227
const mockAnimation = createMockAnimation({
232228
effect: {
233-
getComputedTiming: vi.fn().mockReturnValue({
234-
progress: inputProgress,
235-
}),
236-
getTiming: vi.fn().mockReturnValue({ delay: 0 }),
229+
getComputedTiming: vi.fn().mockReturnValue({ progress: inputProgress }),
230+
getTiming: vi.fn().mockReturnValue({ delay: 0, duration: 1000 }),
237231
} as any,
238232
});
239233

@@ -515,21 +509,17 @@ describe('AnimationGroup', () => {
515509
test('should set currentTime on all animations based on progress value', () => {
516510
const mockAnimation1 = createMockAnimation({
517511
effect: {
518-
getComputedTiming: vi.fn().mockReturnValue({
519-
activeDuration: 1000,
520-
}),
521512
getTiming: vi.fn().mockReturnValue({
522513
delay: 200,
514+
duration: 1000,
523515
}),
524516
} as any,
525517
});
526518
const mockAnimation2 = createMockAnimation({
527519
effect: {
528-
getComputedTiming: vi.fn().mockReturnValue({
529-
activeDuration: 2000,
530-
}),
531520
getTiming: vi.fn().mockReturnValue({
532521
delay: 100,
522+
duration: 2000,
533523
}),
534524
} as any,
535525
});
@@ -541,47 +531,43 @@ describe('AnimationGroup', () => {
541531

542532
animationGroup.progress(0.5);
543533

544-
// Expected calculation: (delay + activeDuration) * progress
534+
// Expected calculation: (delay + duration * progress) * progress
545535
// Animation 1: (200 + 1000) * 0.5 = 600
546536
// Animation 2: (100 + 2000) * 0.5 = 1050
547537
expect(mockAnimation1.currentTime).toBe(600);
548538
expect(mockAnimation2.currentTime).toBe(1050);
549-
expect(mockAnimation1.effect!.getComputedTiming).toHaveBeenCalled();
550539
expect(mockAnimation1.effect!.getTiming).toHaveBeenCalled();
551-
expect(mockAnimation2.effect!.getComputedTiming).toHaveBeenCalled();
552540
expect(mockAnimation2.effect!.getTiming).toHaveBeenCalled();
553541
});
554542

555-
test('should calculate currentTime using activeDuration and delay', () => {
543+
test('should calculate currentTime using , progress, and delay', () => {
556544
const testCases = [
557545
{
558-
activeDuration: 1000,
546+
duration: 1000,
559547
delay: 500,
560548
progress: 0.75,
561549
expected: 1125, // (500 + 1000) * 0.75
562550
},
563551
{
564-
activeDuration: 800,
552+
duration: 800,
565553
delay: 0,
566554
progress: 0.25,
567555
expected: 200, // (0 + 800) * 0.25
568556
},
569557
{
570-
activeDuration: 0,
558+
duration: 0,
571559
delay: 300,
572560
progress: 1.0,
573561
expected: 300, // (300 + 0) * 1.0
574562
},
575563
];
576564

577-
testCases.forEach(({ activeDuration, delay, progress, expected }) => {
565+
testCases.forEach(({ duration, delay, progress, expected }) => {
578566
const mockAnimation = createMockAnimation({
579567
effect: {
580-
getComputedTiming: vi.fn().mockReturnValue({
581-
activeDuration,
582-
}),
583568
getTiming: vi.fn().mockReturnValue({
584569
delay,
570+
duration,
585571
}),
586572
} as any,
587573
});
@@ -596,11 +582,9 @@ describe('AnimationGroup', () => {
596582
test('should handle progress value of 0', () => {
597583
const mockAnimation = createMockAnimation({
598584
effect: {
599-
getComputedTiming: vi.fn().mockReturnValue({
600-
activeDuration: 1000,
601-
}),
602585
getTiming: vi.fn().mockReturnValue({
603586
delay: 200,
587+
duration: 1000,
604588
}),
605589
} as any,
606590
});
@@ -614,11 +598,9 @@ describe('AnimationGroup', () => {
614598
test('should handle progress value of 1', () => {
615599
const mockAnimation = createMockAnimation({
616600
effect: {
617-
getComputedTiming: vi.fn().mockReturnValue({
618-
activeDuration: 1000,
619-
}),
620601
getTiming: vi.fn().mockReturnValue({
621602
delay: 200,
603+
duration: 1000,
622604
}),
623605
} as any,
624606
});
@@ -632,11 +614,9 @@ describe('AnimationGroup', () => {
632614
test('should handle progress values greater than 1', () => {
633615
const mockAnimation = createMockAnimation({
634616
effect: {
635-
getComputedTiming: vi.fn().mockReturnValue({
636-
activeDuration: 1000,
637-
}),
638617
getTiming: vi.fn().mockReturnValue({
639618
delay: 200,
619+
duration: 1000,
640620
}),
641621
} as any,
642622
});
@@ -650,11 +630,9 @@ describe('AnimationGroup', () => {
650630
test('should handle negative progress values', () => {
651631
const mockAnimation = createMockAnimation({
652632
effect: {
653-
getComputedTiming: vi.fn().mockReturnValue({
654-
activeDuration: 1000,
655-
}),
656633
getTiming: vi.fn().mockReturnValue({
657634
delay: 200,
635+
duration: 1000,
658636
}),
659637
} as any,
660638
});
@@ -668,11 +646,9 @@ describe('AnimationGroup', () => {
668646
test('should handle animations with no delay', () => {
669647
const mockAnimation = createMockAnimation({
670648
effect: {
671-
getComputedTiming: vi.fn().mockReturnValue({
672-
activeDuration: 1000,
673-
}),
674649
getTiming: vi.fn().mockReturnValue({
675650
delay: undefined,
651+
duration: 1000,
676652
}),
677653
} as any,
678654
});
@@ -684,14 +660,12 @@ describe('AnimationGroup', () => {
684660
expect(mockAnimation.currentTime).toBe(500);
685661
});
686662

687-
test('should handle animations with zero activeDuration', () => {
663+
test('should handle animations with zero duration', () => {
688664
const mockAnimation = createMockAnimation({
689665
effect: {
690-
getComputedTiming: vi.fn().mockReturnValue({
691-
activeDuration: 0,
692-
}),
693666
getTiming: vi.fn().mockReturnValue({
694667
delay: 200,
668+
duration: 0,
695669
}),
696670
} as any,
697671
});
@@ -885,7 +859,7 @@ describe('AnimationGroup', () => {
885859

886860
test('should catch and log errors when animations are interrupted', async () => {
887861
const callback = vi.fn();
888-
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation();
862+
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
889863

890864
const mockAnimation1 = createMockAnimation({
891865
finished: Promise.resolve(undefined as any),
@@ -921,7 +895,7 @@ describe('AnimationGroup', () => {
921895

922896
test('should not execute callback if any animation is cancelled', async () => {
923897
const callback = vi.fn();
924-
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation();
898+
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
925899

926900
const mockAnimation1 = createMockAnimation({
927901
finished: Promise.resolve(undefined as any),
@@ -945,7 +919,7 @@ describe('AnimationGroup', () => {
945919

946920
test('should handle promise rejections gracefully', async () => {
947921
const callback = vi.fn();
948-
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation();
922+
const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
949923

950924
const mockAnimation = createMockAnimation({
951925
finished: Promise.reject(new Error('Unexpected animation error')),
@@ -1015,24 +989,20 @@ describe('AnimationGroup', () => {
1015989
test('should coordinate multiple animations with different timings', () => {
1016990
const mockAnimation1 = createMockAnimation({
1017991
effect: {
1018-
getComputedTiming: vi.fn().mockReturnValue({
1019-
activeDuration: 1000,
1020-
progress: 0.3,
1021-
}),
992+
getComputedTiming: vi.fn().mockReturnValue({ progress: 0.3 }),
1022993
getTiming: vi.fn().mockReturnValue({
1023994
delay: 100,
995+
duration: 1000,
1024996
}),
1025997
} as any,
1026998
playState: 'running' as AnimationPlayState,
1027999
});
10281000
const mockAnimation2 = createMockAnimation({
10291001
effect: {
1030-
getComputedTiming: vi.fn().mockReturnValue({
1031-
activeDuration: 2000,
1032-
progress: 0.7,
1033-
}),
1002+
getComputedTiming: vi.fn().mockReturnValue({ progress: 0.7 }),
10341003
getTiming: vi.fn().mockReturnValue({
10351004
delay: 200,
1005+
duration: 2000,
10361006
}),
10371007
} as any,
10381008
playState: 'running' as AnimationPlayState,

‎packages/motion/vitest.config.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ export default defineConfig({
55
// plugins: [react()],
66
test: {
77
environment: 'jsdom',
8-
setupFiles: []
8+
setupFiles: [],
9+
exclude: ['dist/*']
910
}
1011
});
1112

‎yarn.lock‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1370,7 +1370,7 @@ __metadata:
13701370
"@types/react-dom": "npm:^18.3.0"
13711371
"@vitejs/plugin-react": "npm:^4.3.4"
13721372
"@vitest/coverage-v8": "npm:^4.0.14"
1373-
"@wix/motion": "npm:^1.0.0"
1373+
"@wix/motion": "npm:^2.0.0"
13741374
fastdom: "npm:^1.0.12"
13751375
fizban: "npm:^0.7.2"
13761376
jsdom: "npm:^24.0.0"
@@ -1402,17 +1402,7 @@ __metadata:
14021402
languageName: node
14031403
linkType: hard
14041404

1405-
"@wix/motion@npm:^1.0.0":
1406-
version: 1.658.0
1407-
resolution: "@wix/motion@npm:1.658.0"
1408-
dependencies:
1409-
"@babel/runtime": "npm:^7.26.0"
1410-
fastdom: "npm:^1.0.12"
1411-
checksum: 10/988a17f1ca0f3a992004d09c9ce383bd89b56bd91ba1cb25793d12e22ab864a046a949f857caf0df180265bb7ce264d6cb6959888e07095b59ac31fbdbe5e06c
1412-
languageName: node
1413-
linkType: hard
1414-
1415-
"@wix/motion@workspace:packages/motion":
1405+
"@wix/motion@npm:^2.0.0, @wix/motion@workspace:packages/motion":
14161406
version: 0.0.0-use.local
14171407
resolution: "@wix/motion@workspace:packages/motion"
14181408
dependencies:

0 commit comments

Comments
 (0)