Skip to content

Commit e279f30

Browse files
thePunderWomanmmalerba
thePunderWoman
authored and
mmalerba
committed
fix(core): prevent early exit from leave animations when multiple transitions are present (#64225)
Our code ensuring host binding composition for animations was causing the early exit and removal of elements when multiple transitions were present on the same element. This commit fixes the issue by ensuring that we properly keep track of all the promise resolvers on the LView and then only call them once we've properly waited for the longest animation to finish. fixes: #64209 PR Close #64225
1 parent 3b60eb4 commit e279f30

6 files changed

Lines changed: 120 additions & 37 deletions

File tree

‎packages/core/src/animation/interfaces.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,17 @@ export interface LongestAnimation {
9797
duration: number;
9898
}
9999

100+
export interface NodeAnimations {
101+
animateFns: Function[];
102+
resolvers?: VoidFunction[];
103+
}
104+
100105
export interface AnimationLViewData {
101106
// Enter animations that apply to nodes in this view
102-
enter?: Map<number, Function[]>;
107+
enter?: Map<number, NodeAnimations>;
103108

104109
// Leave animations that apply to nodes in this view
105-
leave?: Map<number, Function[]>;
110+
leave?: Map<number, NodeAnimations>;
106111

107112
// Leave animations that apply to nodes in this view
108113
// We chose to use unknown instead of PromiseSettledResult<void> to avoid requiring the type

‎packages/core/src/animation/utils.ts‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {stringify} from '../util/stringify'; // Adjust imports as per actual location
10-
import {ANIMATIONS_DISABLED, LongestAnimation} from './interfaces';
10+
import {ANIMATIONS_DISABLED, LongestAnimation, NodeAnimations} from './interfaces';
1111
import {INJECTOR, LView, DECLARATION_LCONTAINER, ANIMATIONS} from '../render3/interfaces/view';
1212
import {RuntimeError, RuntimeErrorCode} from '../errors';
1313
import {Renderer} from '../render3/interfaces/renderer';
@@ -162,17 +162,17 @@ export function trackLeavingNodes(tNode: TNode, el: HTMLElement): void {
162162
/**
163163
* Retrieves the list of specified enter animations from the lView
164164
*/
165-
export function getLViewEnterAnimations(lView: LView): Map<number, Function[]> {
165+
export function getLViewEnterAnimations(lView: LView): Map<number, NodeAnimations> {
166166
const animationData = (lView[ANIMATIONS] ??= {});
167-
return (animationData.enter ??= new Map<number, Function[]>());
167+
return (animationData.enter ??= new Map<number, NodeAnimations>());
168168
}
169169

170170
/**
171171
* Retrieves the list of specified leave animations from the lView
172172
*/
173-
export function getLViewLeaveAnimations(lView: LView): Map<number, Function[]> {
173+
export function getLViewLeaveAnimations(lView: LView): Map<number, NodeAnimations> {
174174
const animationData = (lView[ANIMATIONS] ??= {});
175-
return (animationData.leave ??= new Map<number, Function[]>());
175+
return (animationData.leave ??= new Map<number, NodeAnimations>());
176176
}
177177

178178
/**
@@ -253,11 +253,30 @@ export function isLongestAnimation(
253253
* @param fn The animation function to be called later
254254
*/
255255
export function addAnimationToLView(
256-
animations: Map<number, Function[]>,
256+
animations: Map<number, NodeAnimations>,
257257
tNode: TNode,
258258
fn: Function,
259259
) {
260-
const animationFns = animations.get(tNode.index) ?? [];
261-
animationFns.push(fn);
262-
animations.set(tNode.index, animationFns);
260+
const nodeAnimations = animations.get(tNode.index) ?? {animateFns: []};
261+
nodeAnimations.animateFns.push(fn);
262+
animations.set(tNode.index, nodeAnimations);
263+
}
264+
265+
export function cleanupAfterLeaveAnimations(
266+
resolvers: VoidFunction[] | undefined,
267+
cleanupFns: Function[],
268+
): void {
269+
if (resolvers) {
270+
for (const fn of resolvers) {
271+
fn();
272+
}
273+
}
274+
for (const fn of cleanupFns) {
275+
fn();
276+
}
277+
}
278+
279+
export function clearLViewNodeAnimationResolvers(lView: LView, tNode: TNode) {
280+
const nodeAnimations = getLViewLeaveAnimations(lView).get(tNode.index);
281+
if (nodeAnimations) nodeAnimations.resolvers = undefined;
263282
}

‎packages/core/src/render3/instructions/animation.ts‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@ import {
3232
assertElementNodes,
3333
cancelAnimationsIfRunning,
3434
cancelLeavingNodes,
35+
cleanupAfterLeaveAnimations,
3536
cleanupEnterClassData,
3637
clearLeavingNodes,
38+
clearLViewNodeAnimationResolvers,
3739
enterClassMap,
3840
getClassListFromValue,
3941
getLViewEnterAnimations,
@@ -246,7 +248,11 @@ export function ɵɵanimateLeave(value: string | Function): typeof ɵɵanimateLe
246248
return ɵɵanimateLeave; // For chaining
247249
}
248250

249-
function runLeaveAnimations(lView: LView, tNode: TNode, value: string | Function): Promise<void> {
251+
function runLeaveAnimations(
252+
lView: LView,
253+
tNode: TNode,
254+
value: string | Function,
255+
): {promise: Promise<void>; resolve: VoidFunction} {
250256
const {promise, resolve} = promiseWithResolvers<void>();
251257
const nativeElement = getNativeByTNode(tNode, lView) as Element;
252258

@@ -255,22 +261,23 @@ function runLeaveAnimations(lView: LView, tNode: TNode, value: string | Function
255261
const renderer = lView[RENDERER];
256262
const ngZone = lView[INJECTOR].get(NgZone);
257263
allLeavingAnimations.add(lView);
264+
(getLViewLeaveAnimations(lView).get(tNode.index)!.resolvers ??= []).push(resolve);
258265

259266
const activeClasses = getClassListFromValue(value);
260267
if (activeClasses && activeClasses.length > 0) {
261268
animateLeaveClassRunner(
262269
nativeElement as HTMLElement,
263270
tNode,
271+
lView,
264272
activeClasses,
265273
renderer,
266274
ngZone,
267-
resolve,
268275
);
269276
} else {
270277
resolve();
271278
}
272279

273-
return promise;
280+
return {promise, resolve};
274281
}
275282

276283
/**
@@ -280,13 +287,14 @@ function runLeaveAnimations(lView: LView, tNode: TNode, value: string | Function
280287
function animateLeaveClassRunner(
281288
el: HTMLElement,
282289
tNode: TNode,
290+
lView: LView,
283291
classList: string[],
284292
renderer: Renderer,
285293
ngZone: NgZone,
286-
resolver: VoidFunction,
287294
) {
288295
cancelAnimationsIfRunning(el, renderer);
289296
const cleanupFns: Function[] = [];
297+
const resolvers = getLViewLeaveAnimations(lView).get(tNode.index)?.resolvers;
290298

291299
const handleOutAnimationEnd = (event: AnimationEvent | TransitionEvent | CustomEvent) => {
292300
// this early exit case is to prevent issues with bubbling events that are from child element animations
@@ -308,36 +316,29 @@ function animateLeaveClassRunner(
308316
renderer.removeClass(el, item);
309317
}
310318
}
311-
}
312-
resolver();
313-
for (const fn of cleanupFns) {
314-
fn();
319+
cleanupAfterLeaveAnimations(resolvers, cleanupFns);
320+
clearLViewNodeAnimationResolvers(lView, tNode);
315321
}
316322
};
317323

318324
ngZone.runOutsideAngular(() => {
319325
cleanupFns.push(renderer.listen(el, 'animationend', handleOutAnimationEnd));
320326
cleanupFns.push(renderer.listen(el, 'transitionend', handleOutAnimationEnd));
321327
});
322-
323328
trackLeavingNodes(tNode, el);
324-
325329
for (const item of classList) {
326330
renderer.addClass(el, item);
327331
}
328-
332+
// In the case that the classes added have no animations, we need to remove
333+
// the element right away. This could happen because someone is intentionally
334+
// preventing an animation via selector specificity.
329335
ngZone.runOutsideAngular(() => {
330336
requestAnimationFrame(() => {
331-
// In the case that the classes added have no animations, we need to remove
332-
// the element right away. This could happen because someone is intentionally
333-
// preventing an animation via selector specificity.
334337
determineLongestAnimation(el, longestAnimations, areAnimationSupported);
335338
if (!longestAnimations.has(el)) {
336339
clearLeavingNodes(tNode, el);
337-
resolver();
338-
for (const fn of cleanupFns) {
339-
fn();
340-
}
340+
cleanupAfterLeaveAnimations(resolvers, cleanupFns);
341+
clearLViewNodeAnimationResolvers(lView, tNode);
341342
}
342343
});
343344
});
@@ -438,8 +439,8 @@ function queueEnterAnimations(lView: LView) {
438439
const enterAnimations = lView[ANIMATIONS]?.enter;
439440
if (enterAnimations) {
440441
const animationQueue = lView[INJECTOR].get(ANIMATION_QUEUE);
441-
for (const [_, animateFns] of enterAnimations) {
442-
for (const animateFn of animateFns) {
442+
for (const [_, nodeAnimations] of enterAnimations) {
443+
for (const animateFn of nodeAnimations.animateFns) {
443444
animationQueue.queue.add(animateFn);
444445
}
445446
}

‎packages/core/src/render3/node_manipulation.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ function maybeQueueEnterAnimation(
111111
const enterAnimations = parentLView?.[ANIMATIONS]?.enter;
112112
if (parent !== null && enterAnimations && enterAnimations.has(tNode.index)) {
113113
const animationQueue = injector.get(ANIMATION_QUEUE);
114-
for (const animateFn of enterAnimations.get(tNode.index)!) {
114+
for (const animateFn of enterAnimations.get(tNode.index)!.animateFns) {
115115
animationQueue.queue.add(animateFn);
116116
}
117117
}
@@ -416,9 +416,10 @@ function runLeaveAnimationsWithCallback(
416416
const leaveAnimations = leaveAnimationMap.get(tNode.index);
417417
const runningAnimations = [];
418418
if (leaveAnimations) {
419-
for (let index = 0; index < leaveAnimations.length; index++) {
420-
const animationFn = leaveAnimations[index];
421-
runningAnimations.push(animationFn());
419+
for (let index = 0; index < leaveAnimations.animateFns.length; index++) {
420+
const animationFn = leaveAnimations.animateFns[index];
421+
const {promise} = animationFn();
422+
runningAnimations.push(promise);
422423
}
423424
}
424425
animations.running = Promise.allSettled(runningAnimations);

‎packages/core/test/acceptance/animation_spec.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,63 @@ describe('Animation', () => {
714714
expect(fixture.debugElement.query(By.css('child-cmp'))).toBeNull();
715715
}));
716716

717+
it('should await the longest animation when multiple transitions are present', fakeAsync(() => {
718+
const multiple = `
719+
.slide-out {
720+
grid-template-rows: 0fr;
721+
overflow: hidden;
722+
opacity: 0;
723+
translate: 0 -60px;
724+
margin-top: -16px;
725+
transition: grid-template-rows 1s ease 400ms, margin-top 1s ease 400ms,
726+
opacity 400ms ease, translate 400ms ease;
727+
}
728+
`;
729+
@Component({
730+
selector: 'test-cmp',
731+
styles: multiple,
732+
template: '@if (show()) { <div animate.leave="slide-out"><p>Element with text</p></div> }',
733+
encapsulation: ViewEncapsulation.None,
734+
})
735+
class TestComponent {
736+
show = signal(true);
737+
}
738+
TestBed.configureTestingModule({animationsEnabled: true});
739+
740+
const fixture = TestBed.createComponent(TestComponent);
741+
const cmp = fixture.componentInstance;
742+
fixture.detectChanges();
743+
const div = fixture.debugElement.query(By.css('div'));
744+
745+
expect(div.nativeElement.className).not.toContain('slide-out');
746+
cmp.show.set(false);
747+
fixture.detectChanges();
748+
tickAnimationFrames(1);
749+
expect(cmp.show()).toBeFalsy();
750+
fixture.detectChanges();
751+
expect(div.nativeElement.className).toContain('slide-out');
752+
fixture.detectChanges();
753+
754+
div.nativeElement.dispatchEvent(
755+
new TransitionEvent('transitionend', {propertyName: 'opacity'}),
756+
);
757+
div.nativeElement.dispatchEvent(
758+
new TransitionEvent('transitionend', {propertyName: 'translate'}),
759+
);
760+
761+
expect(fixture.nativeElement.outerHTML).toContain('slide-out');
762+
763+
div.nativeElement.dispatchEvent(
764+
new TransitionEvent('transitionend', {propertyName: 'margin-top'}),
765+
);
766+
div.nativeElement.dispatchEvent(
767+
new TransitionEvent('transitionend', {propertyName: 'grid-template-rows'}),
768+
);
769+
tick();
770+
expect(fixture.nativeElement.outerHTML).not.toContain('slide-out');
771+
expect(fixture.debugElement.query(By.css('div'))).toBeNull();
772+
}));
773+
717774
describe('legacy animations compatibility', () => {
718775
beforeAll(() => {
719776
TestBed.resetTestEnvironment();

‎packages/core/test/bundling/defer/bundle.golden_symbols.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
"shimStylesContent"
5959
],
6060
"lazy": [
61+
"DeferComponent",
6162
"AFTER_RENDER_SEQUENCES_TO_ADD",
6263
"ANIMATIONS",
6364
"ANIMATION_QUEUE",
@@ -746,8 +747,7 @@
746747
"wasLastNodeCreated",
747748
"writeDirectClass",
748749
"writeDirectStyle",
749-
"writeToDirectiveInput",
750-
"DeferComponent"
750+
"writeToDirectiveInput"
751751
]
752752
}
753753
}

0 commit comments

Comments
 (0)