Skip to content

Commit f15cfa4

Browse files
thePunderWomanAndrewKushnir
thePunderWoman
authored and
AndrewKushnir
committed
fix(core): fixes regression in animate.leave function bindings (#64413)
When adding and removing items in a `@for` loop, the `animate.leave` event binding instruction was not updated to use the same logic as the class function when the animation queue was added. We were not returning the correct signature for the `animate.leave` function, which caused the animation to not trigger correctly. This updates the event binding instruction to use the same logic as the class function when adding the animation to the queue. fixes: #64336 PR Close #64413
1 parent 285dad4 commit f15cfa4

3 files changed

Lines changed: 233 additions & 16 deletions

File tree

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,3 +280,15 @@ export function clearLViewNodeAnimationResolvers(lView: LView, tNode: TNode) {
280280
const nodeAnimations = getLViewLeaveAnimations(lView).get(tNode.index);
281281
if (nodeAnimations) nodeAnimations.resolvers = undefined;
282282
}
283+
284+
export function leaveAnimationFunctionCleanup(
285+
lView: LView,
286+
tNode: TNode,
287+
nativeElement: HTMLElement,
288+
resolvers: VoidFunction[] | undefined,
289+
cleanupFns: Function[],
290+
) {
291+
clearLeavingNodes(tNode, nativeElement as HTMLElement);
292+
cleanupAfterLeaveAnimations(resolvers, cleanupFns);
293+
clearLViewNodeAnimationResolvers(lView, tNode);
294+
}

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

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import {
4141
getLViewEnterAnimations,
4242
getLViewLeaveAnimations,
4343
isLongestAnimation,
44+
leaveAnimationFunctionCleanup,
4445
longestAnimations,
4546
noOpAnimationComplete,
4647
trackEnterClasses,
@@ -388,50 +389,81 @@ function runLeaveAnimationFunction(
388389
lView: LView,
389390
tNode: TNode,
390391
value: AnimationFunction,
391-
): Promise<void> {
392+
): {promise: Promise<void>; resolve: VoidFunction} {
392393
const {promise, resolve} = promiseWithResolvers<void>();
393394
const nativeElement = getNativeByTNode(tNode, lView) as Element;
394395

395396
ngDevMode && assertElementNodes(nativeElement, 'animate.leave');
396397

398+
const cleanupFns: Function[] = [];
397399
const renderer = lView[RENDERER];
398400
const animationsDisabled = areAnimationsDisabled(lView);
399401
const ngZone = lView[INJECTOR]!.get(NgZone);
400402
const maxAnimationTimeout = lView[INJECTOR]!.get(MAX_ANIMATION_TIMEOUT);
401403

404+
(getLViewLeaveAnimations(lView).get(tNode.index)!.resolvers ??= []).push(resolve);
405+
const resolvers = getLViewLeaveAnimations(lView).get(tNode.index)?.resolvers;
406+
402407
if (animationsDisabled) {
403-
resolve();
408+
leaveAnimationFunctionCleanup(
409+
lView,
410+
tNode,
411+
nativeElement as HTMLElement,
412+
resolvers,
413+
cleanupFns,
414+
);
404415
} else {
405-
const timeoutId = setTimeout(() => {
406-
clearLeavingNodes(tNode, nativeElement as HTMLElement);
407-
resolve();
408-
}, maxAnimationTimeout);
416+
const timeoutId = setTimeout(
417+
() =>
418+
leaveAnimationFunctionCleanup(
419+
lView,
420+
tNode,
421+
nativeElement as HTMLElement,
422+
resolvers,
423+
cleanupFns,
424+
),
425+
maxAnimationTimeout,
426+
);
409427

410428
const event: AnimationCallbackEvent = {
411429
target: nativeElement,
412430
animationComplete: () => {
413-
clearLeavingNodes(tNode, nativeElement as HTMLElement);
431+
leaveAnimationFunctionCleanup(
432+
lView,
433+
tNode,
434+
nativeElement as HTMLElement,
435+
resolvers,
436+
cleanupFns,
437+
);
414438
clearTimeout(timeoutId);
415-
resolve();
416439
},
417440
};
418441
trackLeavingNodes(tNode, nativeElement as HTMLElement);
419442

420443
ngZone.runOutsideAngular(() => {
421-
renderer.listen(
422-
nativeElement,
423-
'animationend',
424-
() => {
425-
resolve();
426-
},
427-
{once: true},
444+
cleanupFns.push(
445+
renderer.listen(
446+
nativeElement,
447+
'animationend',
448+
() => {
449+
leaveAnimationFunctionCleanup(
450+
lView,
451+
tNode,
452+
nativeElement as HTMLElement,
453+
resolvers,
454+
cleanupFns,
455+
);
456+
clearTimeout(timeoutId);
457+
},
458+
{once: true},
459+
),
428460
);
429461
});
430462
value.call(lView[CONTEXT], event);
431463
}
432464

433465
// Ensure cleanup if the LView is destroyed before the animation runs.
434-
return promise;
466+
return {promise, resolve};
435467
}
436468

437469
function queueEnterAnimations(lView: LView) {

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

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,6 +1641,179 @@ describe('Animation', () => {
16411641
expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(4);
16421642
}));
16431643

1644+
it('should run leave and enter animations for `@for` loops when adding / removing simultaneously', fakeAsync(() => {
1645+
const animateStyles = `
1646+
.slide-in {
1647+
animation: slide-in 500ms;
1648+
}
1649+
.fade {
1650+
animation: fade-out 500ms;
1651+
}
1652+
@keyframes slide-in {
1653+
from {
1654+
transform: translateX(-10px);
1655+
}
1656+
to {
1657+
transform: translateX(0);
1658+
}
1659+
}
1660+
@keyframes fade-out {
1661+
from {
1662+
opacity: 1;
1663+
}
1664+
to {
1665+
opacity: 0;
1666+
}
1667+
}
1668+
`;
1669+
1670+
@Component({
1671+
selector: 'test-cmp',
1672+
styles: animateStyles,
1673+
template: `
1674+
<div>
1675+
@for (item of items(); track item) {
1676+
<p id="item-{{item}}" animate.enter="slide-in" animate.leave="fade">I should slide in {{item}}.</p>
1677+
}
1678+
</div>
1679+
`,
1680+
encapsulation: ViewEncapsulation.None,
1681+
})
1682+
class TestComponent {
1683+
items = signal([1, 2, 3]);
1684+
1685+
addremove() {
1686+
this.items.update((l) => l.slice(1).concat([l.at(-1)! + 1]));
1687+
}
1688+
}
1689+
TestBed.configureTestingModule({animationsEnabled: true});
1690+
1691+
const fixture = TestBed.createComponent(TestComponent);
1692+
const cmp = fixture.componentInstance;
1693+
fixture.detectChanges();
1694+
tickAnimationFrames(1);
1695+
const paragraphs = fixture.debugElement.queryAll(By.css('p'));
1696+
paragraphs.forEach((p) => {
1697+
p.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
1698+
p.nativeElement.dispatchEvent(
1699+
new AnimationEvent('animationend', {animationName: 'slide-in'}),
1700+
);
1701+
});
1702+
cmp.addremove();
1703+
fixture.detectChanges();
1704+
tickAnimationFrames(1);
1705+
1706+
const first = fixture.debugElement.query(By.css('p#item-1'));
1707+
const last = fixture.debugElement.query(By.css('p#item-4'));
1708+
first.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
1709+
last.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
1710+
expect(fixture.debugElement.queryAll(By.css('p.fade')).length).toBe(1);
1711+
expect(fixture.debugElement.queryAll(By.css('p.slide-in')).length).toBe(1);
1712+
1713+
last.nativeElement.dispatchEvent(
1714+
new AnimationEvent('animationend', {animationName: 'slide-in'}),
1715+
);
1716+
first.nativeElement.dispatchEvent(
1717+
new AnimationEvent('animationend', {animationName: 'fade-out'}),
1718+
);
1719+
fixture.detectChanges();
1720+
tickAnimationFrames(1);
1721+
tick();
1722+
1723+
expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(3);
1724+
expect(fixture.debugElement.queryAll(By.css('p.slide-in')).length).toBe(0);
1725+
expect(fixture.debugElement.queryAll(By.css('p.fade')).length).toBe(0);
1726+
}));
1727+
1728+
it('should run leave and enter animations for `@for` loops when adding / removing simultaneously with leave function', fakeAsync(() => {
1729+
const animateStyles = `
1730+
.slide-in {
1731+
animation: slide-in 500ms;
1732+
}
1733+
.fade {
1734+
animation: fade-out 500ms;
1735+
}
1736+
@keyframes slide-in {
1737+
from {
1738+
transform: translateX(-10px);
1739+
}
1740+
to {
1741+
transform: translateX(0);
1742+
}
1743+
}
1744+
@keyframes fade-out {
1745+
from {
1746+
opacity: 1;
1747+
}
1748+
to {
1749+
opacity: 0;
1750+
}
1751+
}
1752+
`;
1753+
1754+
@Component({
1755+
selector: 'test-cmp',
1756+
styles: animateStyles,
1757+
template: `
1758+
<div>
1759+
@for (item of items(); track item) {
1760+
<p id="item-{{item}}" animate.enter="slide-in" (animate.leave)="fadeOut($event)">I should slide in {{item}}.</p>
1761+
}
1762+
</div>
1763+
`,
1764+
encapsulation: ViewEncapsulation.None,
1765+
})
1766+
class TestComponent {
1767+
items = signal([1, 2, 3]);
1768+
1769+
addremove() {
1770+
this.items.update((l) => l.slice(1).concat([l.at(-1)! + 1]));
1771+
}
1772+
1773+
fadeOut(event: AnimationCallbackEvent) {
1774+
event.target.classList.add('fade');
1775+
setTimeout(() => event.animationComplete(), 500);
1776+
}
1777+
}
1778+
TestBed.configureTestingModule({animationsEnabled: true});
1779+
1780+
const fixture = TestBed.createComponent(TestComponent);
1781+
const cmp = fixture.componentInstance;
1782+
fixture.detectChanges();
1783+
tickAnimationFrames(1);
1784+
const paragraphs = fixture.debugElement.queryAll(By.css('p'));
1785+
paragraphs.forEach((p) => {
1786+
p.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
1787+
p.nativeElement.dispatchEvent(
1788+
new AnimationEvent('animationend', {animationName: 'slide-in'}),
1789+
);
1790+
});
1791+
cmp.addremove();
1792+
fixture.detectChanges();
1793+
tickAnimationFrames(1);
1794+
1795+
const first = fixture.debugElement.query(By.css('p#item-1'));
1796+
const last = fixture.debugElement.query(By.css('p#item-4'));
1797+
first.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
1798+
last.nativeElement.dispatchEvent(new AnimationEvent('animationstart'));
1799+
expect(fixture.debugElement.queryAll(By.css('p.fade')).length).toBe(1);
1800+
expect(fixture.debugElement.queryAll(By.css('p.slide-in')).length).toBe(1);
1801+
1802+
last.nativeElement.dispatchEvent(
1803+
new AnimationEvent('animationend', {animationName: 'slide-in'}),
1804+
);
1805+
first.nativeElement.dispatchEvent(
1806+
new AnimationEvent('animationend', {animationName: 'fade-out'}),
1807+
);
1808+
fixture.detectChanges();
1809+
tickAnimationFrames(1);
1810+
tick();
1811+
1812+
expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(3);
1813+
expect(fixture.debugElement.queryAll(By.css('p.slide-in')).length).toBe(0);
1814+
expect(fixture.debugElement.queryAll(By.css('p.fade')).length).toBe(0);
1815+
}));
1816+
16441817
it('should always run animations for custom repeater loops when adding and removing quickly', fakeAsync(() => {
16451818
const animateStyles = `
16461819
.slide-in {

0 commit comments

Comments
 (0)