Skip to content

Commit ebfc92f

Browse files
rkaraivanovrkaraivanov
authored
refactor(animation): Animation controller cleanups (#1939)
* Added API documentation * Renamed 'stopAll' to 'cancelAll' and simplified cancelation logic * General code cleanup in the controller * Use 'playExclusive' in relevant components * Fixed some issues with banner tests not waiting for the its update complete logic
1 parent 4233821 commit ebfc92f

File tree

9 files changed

+106
-97
lines changed

9 files changed

+106
-97
lines changed

‎src/animations/player.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ describe('Animations Player', () => {
6363
it('should cancel running animations', async () => {
6464
const [playbackEvent] = (await Promise.all([
6565
el.player.play(fade),
66-
el.player.stopAll(),
66+
el.player.cancelAll(),
6767
])) as AnimationPlaybackEvent[];
6868

6969
expect(playbackEvent.type).to.equal('cancel');

‎src/animations/player.ts‎

Lines changed: 81 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,86 +1,113 @@
11
import type { ReactiveController, ReactiveControllerHost } from 'lit';
22
import type { Ref } from 'lit/directives/ref.js';
3-
43
import { isElement } from '../components/common/util.js';
54
import type { AnimationReferenceMetadata } from './types.js';
65

7-
const listenerOptions = { once: true };
6+
/**
7+
* Defines the result of an optional View Transition start.
8+
*/
9+
type ViewTransitionResult = {
10+
transition?: ViewTransition;
11+
};
12+
13+
const LISTENER_OPTIONS = { once: true } as const;
814

9-
function getPrefersReducedMotion() {
15+
/**
16+
* Checks the user's preference for reduced motion.
17+
*/
18+
function getPrefersReducedMotion(): boolean {
1019
return globalThis?.matchMedia('(prefers-reduced-motion: reduce)').matches;
1120
}
1221

22+
/**
23+
* A ReactiveController for managing Web Animation API (WAAPI) playback
24+
* on a host element or a specified target element.
25+
*
26+
* It provides methods to play, stop, and coordinate animations, including
27+
* support for 'height: auto' transitions and reduced motion preference.
28+
*/
1329
class AnimationController implements ReactiveController {
14-
protected get target() {
15-
if (isElement(this._target)) {
16-
return this._target;
30+
private readonly _host: ReactiveControllerHost & HTMLElement;
31+
private readonly _ref?: Ref<HTMLElement> | HTMLElement;
32+
33+
/**
34+
* The actual HTMLElement target for the animations.
35+
* Prioritizes a passed-in Ref value, then a direct HTMLElement, falling back to the host.
36+
*/
37+
protected get _target(): HTMLElement {
38+
if (isElement(this._ref)) {
39+
return this._ref;
1740
}
18-
return this._target?.value ?? this.host;
41+
42+
return this._ref?.value ?? this._host;
1943
}
2044

2145
constructor(
22-
private readonly host: ReactiveControllerHost & HTMLElement,
23-
private _target?: Ref<HTMLElement> | HTMLElement
46+
host: ReactiveControllerHost & HTMLElement,
47+
ref?: Ref<HTMLElement> | HTMLElement
2448
) {
25-
this.host.addController(this);
49+
this._host = host;
50+
this._ref = ref;
51+
this._host.addController(this);
2652
}
2753

28-
private parseKeyframes(keyframes: Keyframe[]) {
29-
return keyframes.map((keyframe) => {
30-
if (!keyframe.height) return keyframe;
31-
32-
return {
33-
...keyframe,
34-
height:
35-
keyframe.height === 'auto'
36-
? `${this.target.scrollHeight}px`
37-
: keyframe.height,
38-
};
54+
/** Pre-processes keyframes, specifically resolving 'auto' height to the element's scrollHeight. */
55+
private _parseKeyframes(keyframes: Keyframe[]): Keyframe[] {
56+
const target = this._target;
57+
58+
return keyframes.map((frame) => {
59+
return frame.height === 'auto'
60+
? { ...frame, height: `${target.scrollHeight}px` }
61+
: frame;
3962
});
4063
}
4164

42-
public async play(animation: AnimationReferenceMetadata) {
65+
/** @internal */
66+
public hostConnected(): void {}
67+
68+
/** Plays a sequence of keyframes, first cancelling all existing animations on the target. */
69+
public async playExclusive(
70+
animation: AnimationReferenceMetadata
71+
): Promise<boolean> {
72+
await this.cancelAll();
73+
74+
const event = await this.play(animation);
75+
return event.type === 'finish';
76+
}
77+
78+
/**
79+
* Plays a sequence of keyframes using WAAPI.
80+
* Automatically sets duration to 0 if 'prefers-reduced-motion' is set.
81+
*/
82+
public async play(
83+
animation: AnimationReferenceMetadata
84+
): Promise<AnimationPlaybackEvent> {
4385
const { steps, options } = animation;
86+
const duration = getPrefersReducedMotion() ? 0 : (options?.duration ?? 0);
4487

45-
if (options?.duration === Number.POSITIVE_INFINITY) {
88+
if (!Number.isFinite(duration)) {
4689
throw new Error('Promise-based animations must be finite.');
4790
}
4891

4992
return new Promise<AnimationPlaybackEvent>((resolve) => {
50-
const animation = this.target.animate(this.parseKeyframes(steps), {
93+
const animation = this._target.animate(this._parseKeyframes(steps), {
5194
...options,
52-
duration: getPrefersReducedMotion() ? 0 : options!.duration,
95+
duration,
5396
});
5497

55-
animation.addEventListener('cancel', resolve, listenerOptions);
56-
animation.addEventListener('finish', resolve, listenerOptions);
98+
animation.addEventListener('cancel', resolve, LISTENER_OPTIONS);
99+
animation.addEventListener('finish', resolve, LISTENER_OPTIONS);
57100
});
58101
}
59102

60-
public stopAll() {
61-
return Promise.all(
62-
this.target.getAnimations().map((animation) => {
63-
return new Promise((resolve) => {
64-
const resolver = () => requestAnimationFrame(resolve);
65-
animation.addEventListener('cancel', resolver, listenerOptions);
66-
animation.addEventListener('finish', resolver, listenerOptions);
67-
68-
animation.cancel();
69-
});
70-
})
71-
);
72-
}
73-
74-
public async playExclusive(animation: AnimationReferenceMetadata) {
75-
const [_, event] = await Promise.all([
76-
this.stopAll(),
77-
this.play(animation),
78-
]);
103+
/** Cancels all active animations on the target element. */
104+
public cancelAll(): Promise<void> {
105+
for (const animation of this._target.getAnimations()) {
106+
animation.cancel();
107+
}
79108

80-
return event.type === 'finish';
109+
return Promise.resolve();
81110
}
82-
83-
public hostConnected() {}
84111
}
85112

86113
/**
@@ -91,14 +118,14 @@ class AnimationController implements ReactiveController {
91118
export function addAnimationController(
92119
host: ReactiveControllerHost & HTMLElement,
93120
target?: Ref<HTMLElement> | HTMLElement
94-
) {
121+
): AnimationController {
95122
return new AnimationController(host, target);
96123
}
97124

98-
type ViewTransitionResult = {
99-
transition?: ViewTransition;
100-
};
101-
125+
/**
126+
* Initiates a View Transition if supported by the browser and not suppressed by
127+
* the 'prefers-reduced-motion' setting.
128+
*/
102129
export function startViewTransition(
103130
callback?: ViewTransitionUpdateCallback
104131
): ViewTransitionResult {

‎src/components/banner/banner.spec.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
import { spy } from 'sinon';
99

1010
import { defineComponents } from '../common/definitions/defineComponents.js';
11-
import { finishAnimationsFor } from '../common/utils.spec.js';
11+
import { finishAnimationsFor, simulateClick } from '../common/utils.spec.js';
1212
import IgcIconComponent from '../icon/icon.js';
1313
import IgcBannerComponent from './banner.js';
1414

@@ -231,36 +231,36 @@ describe('Banner', () => {
231231

232232
describe('Action Tests', () => {
233233
it('should close the banner when clicking the default button', async () => {
234+
const button = banner.renderRoot.querySelector('igc-button')!;
235+
234236
expect(banner.open).to.be.false;
235237

236238
await banner.show();
237-
238239
expect(banner.open).to.be.true;
239240

240-
const button = banner.shadowRoot!.querySelector('igc-button');
241-
242-
button?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
241+
simulateClick(button);
242+
await elementUpdated(banner);
243243
await clickHideComplete();
244244

245245
expect(banner.open).to.be.false;
246246
});
247247

248248
it('should emit correct event sequence for the default action button', async () => {
249249
const eventSpy = spy(banner, 'emitEvent');
250+
const button = banner.renderRoot.querySelector('igc-button')!;
250251

251252
expect(banner.open).to.be.false;
252253

253254
await banner.show();
254-
255255
expect(banner.open).to.be.true;
256256

257-
const button = banner.shadowRoot!.querySelector('igc-button');
258-
button?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
257+
simulateClick(button);
259258

260259
expect(eventSpy.callCount).to.equal(1);
261260
expect(eventSpy).calledWith('igcClosing', { cancelable: true });
262261

263262
eventSpy.resetHistory();
263+
await elementUpdated(banner);
264264
await clickHideComplete();
265265

266266
expect(eventSpy).calledWith('igcClosed');
@@ -269,17 +269,17 @@ describe('Banner', () => {
269269

270270
it('can cancel `igcClosing` event', async () => {
271271
const eventSpy = spy(banner, 'emitEvent');
272-
const button = banner.shadowRoot!.querySelector('igc-button');
272+
const button = banner.renderRoot.querySelector('igc-button')!;
273273

274274
banner.addEventListener('igcClosing', (event) => {
275275
event.preventDefault();
276276
});
277277

278278
await banner.show();
279-
280279
expect(banner.open).to.be.true;
281280

282-
button?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
281+
simulateClick(button);
282+
await elementUpdated(banner);
283283
await clickHideComplete();
284284

285285
expect(eventSpy).calledWith('igcClosing');

‎src/components/banner/banner.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,7 @@ export default class IgcBannerComponent extends EventEmitterMixin<
104104

105105
private async toggleAnimation(dir: 'open' | 'close') {
106106
const animation = dir === 'open' ? growVerIn : growVerOut;
107-
108-
const [_, event] = await Promise.all([
109-
this._animationPlayer.stopAll(),
110-
this._animationPlayer.play(animation()),
111-
]);
112-
113-
return event.type === 'finish';
107+
return this._animationPlayer.playExclusive(animation());
114108
}
115109

116110
private async handleClick() {

‎src/components/expansion-panel/expansion-panel.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -125,13 +125,7 @@ export default class IgcExpansionPanelComponent extends EventEmitterMixin<
125125

126126
private async _toggleAnimation(dir: 'open' | 'close'): Promise<boolean> {
127127
const animation = dir === 'open' ? growVerIn : growVerOut;
128-
129-
const [_, event] = await Promise.all([
130-
this._player.stopAll(),
131-
this._player.play(animation()),
132-
]);
133-
134-
return event.type === 'finish';
128+
return this._player.playExclusive(animation());
135129
}
136130

137131
private async _openWithEvent(): Promise<void> {

‎src/components/stepper/step.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -184,16 +184,16 @@ export default class IgcStepComponent extends LitElement {
184184
height: bodyHeight,
185185
};
186186

187-
const [_, event] = await Promise.all([
188-
this.bodyAnimationPlayer.stopAll(),
189-
this.bodyAnimationPlayer.play(bodyAnimation({ keyframe: options, step })),
190-
this.contentAnimationPlayer.stopAll(),
191-
this.contentAnimationPlayer.play(
187+
const result = await Promise.all([
188+
this.bodyAnimationPlayer.playExclusive(
189+
bodyAnimation({ keyframe: options, step })
190+
),
191+
this.contentAnimationPlayer.playExclusive(
192192
contentAnimation({ keyframe: options, step })
193193
),
194194
]);
195195

196-
return event.type;
196+
return result.every(Boolean);
197197
}
198198

199199
@watch('active', { waitUntilFirstUpdate: true })

‎src/components/tooltip/tooltip.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ export default class IgcTooltipComponent extends EventEmitterMixin<
410410

411411
private _stopTimeoutAndAnimation(): void {
412412
clearTimeout(this._timeoutId);
413-
this._player.stopAll();
413+
this._player.cancelAll();
414414
}
415415

416416
private _setAutoHide(): void {

‎src/components/tree/tree-item.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,7 @@ export default class IgcTreeItemComponent extends LitElement {
152152

153153
private async toggleAnimation(dir: 'open' | 'close') {
154154
const animation = dir === 'open' ? growVerIn : growVerOut;
155-
156-
const [_, event] = await Promise.all([
157-
this.animationPlayer.stopAll(),
158-
this.animationPlayer.play(animation()),
159-
]);
160-
161-
return event.type === 'finish';
155+
return this.animationPlayer.playExclusive(animation());
162156
}
163157

164158
@watch('expanded', { waitUntilFirstUpdate: true })

‎stories/textarea.stories.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,9 @@ const metadata: Meta<IgcTextareaComponent> = {
9595
rows: {
9696
type: 'number',
9797
description:
98-
'The number of visible text lines for the control. If it is specified, it must be a positive integer.\nIf it is not specified, the default value is 2.',
98+
'The number of visible text lines for the control. If it is specified, it must be a positive integer.\nIf it is not specified, the default value is 3.',
9999
control: 'number',
100-
table: { defaultValue: { summary: '2' } },
100+
table: { defaultValue: { summary: '3' } },
101101
},
102102
value: {
103103
type: 'string',
@@ -209,7 +209,7 @@ interface IgcTextareaArgs {
209209
resize: 'vertical' | 'auto' | 'none';
210210
/**
211211
* The number of visible text lines for the control. If it is specified, it must be a positive integer.
212-
* If it is not specified, the default value is 2.
212+
* If it is not specified, the default value is 3.
213213
*/
214214
rows: number;
215215
/** The value of the component */

0 commit comments

Comments
 (0)