diff --git a/packages/core/src/animation/interfaces.ts b/packages/core/src/animation/interfaces.ts index aac45817dd0..65bee63448a 100644 --- a/packages/core/src/animation/interfaces.ts +++ b/packages/core/src/animation/interfaces.ts @@ -85,4 +85,12 @@ export interface AnimationLViewData { // Leave animations that apply to nodes in this view running?: Promise[]>; + + // Skip leave animations + // This flag is solely used when move operations occur. DOM Node move + // operations occur in lists, like `@for` loops, and use the same code + // path during move that detaching or removing does. So to prevent + // unexpected disappearing of moving nodes, we use this flag to skip + // the animations in that case. + skipLeaveAnimations?: boolean; } diff --git a/packages/core/src/render3/instructions/control_flow.ts b/packages/core/src/render3/instructions/control_flow.ts index 8ea286e8573..09ab65cc379 100644 --- a/packages/core/src/render3/instructions/control_flow.ts +++ b/packages/core/src/render3/instructions/control_flow.ts @@ -23,6 +23,7 @@ import {CONTAINER_HEADER_OFFSET, LContainer} from '../interfaces/container'; import {ComponentTemplate} from '../interfaces/definition'; import {LocalRefExtractor, TAttributes, TNode, TNodeFlags} from '../interfaces/node'; import { + ANIMATIONS, CONTEXT, DECLARATION_COMPONENT_VIEW, HEADER_OFFSET, @@ -46,6 +47,7 @@ import { removeLViewFromLContainer, } from '../view/container'; import {removeDehydratedViews} from '../../hydration/cleanup'; +import {AnimationLViewData} from '../../animation/interfaces'; /** * Creates an LContainer for an ng-template representing a root node @@ -418,8 +420,9 @@ class LiveCollectionLContainerImpl extends LiveCollection< shouldAddViewToDom(this.templateTNode, dehydratedView), ); } - override detach(index: number): LView> { + override detach(index: number, skipLeaveAnimations?: boolean): LView> { this.needsIndexUpdate ||= index !== this.length - 1; + if (skipLeaveAnimations) setSkipLeaveAnimations(this.lContainer, index); return detachExistingView>(this.lContainer, index); } override create(index: number, value: unknown): LView> { @@ -567,6 +570,16 @@ function getLContainer(lView: LView, index: number): LContainer { return lContainer; } +function setSkipLeaveAnimations(lContainer: LContainer, index: number): void { + if (lContainer.length <= CONTAINER_HEADER_OFFSET) return; + + const indexInContainer = CONTAINER_HEADER_OFFSET + index; + const viewToDetach = lContainer[indexInContainer]; + if (viewToDetach && viewToDetach[ANIMATIONS]) { + (viewToDetach[ANIMATIONS] as AnimationLViewData).skipLeaveAnimations = true; + } +} + function detachExistingView(lContainer: LContainer, index: number): LView { const existingLView = detachView(lContainer, index); ngDevMode && assertLView(existingLView); diff --git a/packages/core/src/render3/list_reconciliation.ts b/packages/core/src/render3/list_reconciliation.ts index bfc4bb64de3..6cf93e89ddb 100644 --- a/packages/core/src/render3/list_reconciliation.ts +++ b/packages/core/src/render3/list_reconciliation.ts @@ -20,7 +20,7 @@ export abstract class LiveCollection { abstract get length(): number; abstract at(index: number): V; abstract attach(index: number, item: T): void; - abstract detach(index: number): T; + abstract detach(index: number, skipLeaveAnimations?: boolean): T; abstract create(index: number, value: V): T; destroy(item: T): void { // noop by default @@ -45,7 +45,11 @@ export abstract class LiveCollection { } } move(prevIndex: number, newIdx: number): void { - this.attach(newIdx, this.detach(prevIndex)); + // For move operations, the detach code path is the same one used for removing + // DOM nodes, which would trigger `animate.leave` bindings. We need to skip + // those animations in the case of a move operation so the moving elements don't + // unexpectedly disappear. + this.attach(newIdx, this.detach(prevIndex, true /* skipLeaveAnimations */)); } } diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index e9bf0178a76..ee9fcee3657 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -352,12 +352,18 @@ function cleanUpView(tView: TView, lView: LView): void { function runLeaveAnimationsWithCallback(lView: LView | undefined, callback: Function) { if (lView && lView[ANIMATIONS] && lView[ANIMATIONS].leave) { - const runningAnimations = []; - for (let animateFn of lView[ANIMATIONS].leave) { - runningAnimations.push(animateFn()); + if (lView[ANIMATIONS].skipLeaveAnimations) { + lView[ANIMATIONS].skipLeaveAnimations = false; + } else { + const leaveAnimations = lView[ANIMATIONS].leave; + const runningAnimations = []; + for (let index = 0; index < leaveAnimations.length; index++) { + const animateFn = leaveAnimations[index]; + runningAnimations.push(animateFn()); + } + lView[ANIMATIONS].running = Promise.allSettled(runningAnimations); + lView[ANIMATIONS].leave = undefined; } - lView[ANIMATIONS].running = Promise.allSettled(runningAnimations); - lView[ANIMATIONS].leave = undefined; } runAfterLeaveAnimations(lView, callback); } diff --git a/packages/core/test/acceptance/animation_spec.ts b/packages/core/test/acceptance/animation_spec.ts index e7b3c129608..618dd2c1961 100644 --- a/packages/core/test/acceptance/animation_spec.ts +++ b/packages/core/test/acceptance/animation_spec.ts @@ -11,6 +11,7 @@ import {ViewEncapsulation} from '@angular/compiler'; import { AnimationCallbackEvent, Component, + computed, Directive, ElementRef, NgModule, @@ -1539,5 +1540,66 @@ describe('Animation', () => { expect(fixture.debugElement.queryAll(By.css('p.slide-in')).length).toBe(1); expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(4); })); + + it('should only remove one element in reactive `@for` loops when removing the second to last item', fakeAsync(() => { + const animateStyles = ` + .fade { + animation: fade-out 500ms; + } + @keyframes fade-out { + from { + opacity: 1; + } + to { + opacity: 0; + } + } + `; + + @Component({ + selector: 'test-cmp', + styles: animateStyles, + template: ` +
+ @for (item of shown(); track item) { +

I should slide in {{item}}.

+ } +
+ `, + encapsulation: ViewEncapsulation.None, + }) + class TestComponent { + items = signal([1, 2, 3, 4, 5, 6]); + shown = computed(() => this.items().slice(0, 3)); + @ViewChild('el', {read: ElementRef}) el!: ElementRef; + max = 6; + + removeSecondToLast() { + this.items.update((old) => { + const newList = [...old]; + newList.splice(1, 1); + return newList; + }); + } + } + TestBed.configureTestingModule({animationsEnabled: true}); + + const fixture = TestBed.createComponent(TestComponent); + const cmp = fixture.componentInstance; + fixture.detectChanges(); + cmp.removeSecondToLast(); + fixture.detectChanges(); + tickAnimationFrames(1); + + expect(fixture.debugElement.queryAll(By.css('p.fade')).length).toBe(1); + expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(4); + fixture.debugElement + .query(By.css('p.fade')) + .nativeElement.dispatchEvent( + new AnimationEvent('animationend', {animationName: 'fade-out'}), + ); + tick(); + expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(3); + })); }); });