diff --git a/packages/core/src/animation/interfaces.ts b/packages/core/src/animation/interfaces.ts index f17a9f0bf3e..3302a6cf192 100644 --- a/packages/core/src/animation/interfaces.ts +++ b/packages/core/src/animation/interfaces.ts @@ -52,7 +52,7 @@ const MAX_ANIMATION_TIMEOUT_DEFAULT = 4000; */ export type AnimationFunction = (event: AnimationCallbackEvent) => void; -export type RunEnterAnimationFn = () => void; +export type RunEnterAnimationFn = VoidFunction; export type RunLeaveAnimationFn = () => {promise: Promise; resolve: VoidFunction}; export interface LongestAnimation { @@ -81,13 +81,11 @@ export interface AnimationLViewData { // We chose to use unknown instead of PromiseSettledResult to avoid requiring the type 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; + // Animation functions that have been queued for this view when the view is detached. + // This is used to later remove them from the global animation queue if the view + // is attached before the animation queue runs. This is used in cases where views are + // moved or swapped during list reconciliation. + detachedLeaveAnimationFns?: VoidFunction[]; } /** diff --git a/packages/core/src/animation/queue.ts b/packages/core/src/animation/queue.ts index c430269f28c..4ce0a50fe1d 100644 --- a/packages/core/src/animation/queue.ts +++ b/packages/core/src/animation/queue.ts @@ -8,7 +8,7 @@ import {afterNextRender} from '../render3/after_render/hooks'; import {InjectionToken, Injector} from '../di'; -import {EnterNodeAnimations} from './interfaces'; +import {AnimationLViewData, EnterNodeAnimations} from './interfaces'; export interface AnimationQueue { queue: Set; @@ -36,18 +36,37 @@ export const ANIMATION_QUEUE = new InjectionToken( export function addToAnimationQueue( injector: Injector, animationFns: VoidFunction | VoidFunction[], + animationData?: AnimationLViewData, ) { const animationQueue = injector.get(ANIMATION_QUEUE); if (Array.isArray(animationFns)) { for (const animateFn of animationFns) { animationQueue.queue.add(animateFn); + // If a node is detached, we need to keep track of the queued animation functions + // so we can later remove them from the global animation queue if the view + // is re-attached before the animation queue runs. + animationData?.detachedLeaveAnimationFns?.push(animateFn); } } else { animationQueue.queue.add(animationFns); + // If a node is detached, we need to keep track of the queued animation functions + // so we can later remove them from the global animation queue if the view + // is re-attached before the animation queue runs. + animationData?.detachedLeaveAnimationFns?.push(animationFns); } animationQueue.scheduler && animationQueue.scheduler(injector); } +export function removeFromAnimationQueue(injector: Injector, animationData: AnimationLViewData) { + const animationQueue = injector.get(ANIMATION_QUEUE); + if (animationData.detachedLeaveAnimationFns) { + for (const animationFn of animationData.detachedLeaveAnimationFns) { + animationQueue.queue.delete(animationFn); + } + animationData.detachedLeaveAnimationFns = undefined; + } +} + export function scheduleAnimationQueue(injector: Injector) { const animationQueue = injector.get(ANIMATION_QUEUE); // We only want to schedule the animation queue if it hasn't already been scheduled. diff --git a/packages/core/src/render3/instructions/control_flow.ts b/packages/core/src/render3/instructions/control_flow.ts index 09ab65cc379..2822ad29ccd 100644 --- a/packages/core/src/render3/instructions/control_flow.ts +++ b/packages/core/src/render3/instructions/control_flow.ts @@ -28,6 +28,7 @@ import { DECLARATION_COMPONENT_VIEW, HEADER_OFFSET, HYDRATION, + INJECTOR, LView, TVIEW, TView, @@ -39,15 +40,17 @@ import {NO_CHANGE} from '../tokens'; import {getConstant, getTNode} from '../util/view_utils'; import {createAndRenderEmbeddedLView, shouldAddViewToDom} from '../view_manipulation'; -import {declareNoDirectiveHostTemplate} from './template'; +import {AnimationLViewData} from '../../animation/interfaces'; +import {removeDehydratedViews} from '../../hydration/cleanup'; import { addLViewToLContainer, detachView, getLViewFromLContainer, removeLViewFromLContainer, } from '../view/container'; -import {removeDehydratedViews} from '../../hydration/cleanup'; -import {AnimationLViewData} from '../../animation/interfaces'; +import {declareNoDirectiveHostTemplate} from './template'; +import {removeFromAnimationQueue} from '../../animation/queue'; +import {allLeavingAnimations} from '../../animation/longest_animation'; /** * Creates an LContainer for an ng-template representing a root node @@ -419,10 +422,11 @@ class LiveCollectionLContainerImpl extends LiveCollection< index, shouldAddViewToDom(this.templateTNode, dehydratedView), ); + clearDetachAnimationList(this.lContainer, index); } - override detach(index: number, skipLeaveAnimations?: boolean): LView> { + override detach(index: number): LView> { this.needsIndexUpdate ||= index !== this.length - 1; - if (skipLeaveAnimations) setSkipLeaveAnimations(this.lContainer, index); + maybeInitDetachAnimationList(this.lContainer, index); return detachExistingView>(this.lContainer, index); } override create(index: number, value: unknown): LView> { @@ -570,13 +574,39 @@ function getLContainer(lView: LView, index: number): LContainer { return lContainer; } -function setSkipLeaveAnimations(lContainer: LContainer, index: number): void { +function clearDetachAnimationList(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; + const animations = viewToDetach + ? (viewToDetach[ANIMATIONS] as AnimationLViewData | undefined) + : undefined; + if ( + viewToDetach && + animations && + animations.detachedLeaveAnimationFns && + animations.detachedLeaveAnimationFns.length > 0 + ) { + const injector = viewToDetach[INJECTOR]; + removeFromAnimationQueue(injector, animations); + allLeavingAnimations.delete(viewToDetach); + animations.detachedLeaveAnimationFns = undefined; + } +} + +// Initialize the detach leave animation list for a view about to be detached, but only +// if it has leave animations. +function maybeInitDetachAnimationList(lContainer: LContainer, index: number): void { + if (lContainer.length <= CONTAINER_HEADER_OFFSET) return; + + const indexInContainer = CONTAINER_HEADER_OFFSET + index; + const viewToDetach = lContainer[indexInContainer]; + const animations = viewToDetach + ? (viewToDetach[ANIMATIONS] as AnimationLViewData | undefined) + : undefined; + if (animations && animations.leave && animations.leave.size > 0) { + animations.detachedLeaveAnimationFns = []; } } diff --git a/packages/core/src/render3/list_reconciliation.ts b/packages/core/src/render3/list_reconciliation.ts index 6cf93e89ddb..e2cfb407dea 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, skipLeaveAnimations?: boolean): T; + abstract detach(index: number): T; abstract create(index: number, value: V): T; destroy(item: T): void { // noop by default @@ -49,7 +49,7 @@ export abstract class LiveCollection { // 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 */)); + this.attach(newIdx, this.detach(prevIndex)); } } diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 255d04b03f0..3548ee6468e 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -84,6 +84,7 @@ import {getLViewParent, getNativeByTNode, unwrapRNode} from './util/view_utils'; import {allLeavingAnimations} from '../animation/longest_animation'; import {Injector} from '../di'; import {addToAnimationQueue, queueEnterAnimations} from '../animation/queue'; +import {RunLeaveAnimationFn} from '../animation/interfaces'; const enum WalkTNodeTreeAction { /** node create in the native environment. Run on initial creation. */ @@ -386,37 +387,35 @@ function runLeaveAnimationsWithCallback( if (animations == null || animations.leave == undefined || !animations.leave.has(tNode.index)) return callback(false); - // this is solely for move operations to prevent leave animations from running - // on the moved nodes, which would have deleted the node. - if (animations.skipLeaveAnimations) { - animations.skipLeaveAnimations = false; - return callback(false); - } - if (lView) allLeavingAnimations.add(lView); - addToAnimationQueue(injector, () => { - // it's possible that in the time between when the leave animation was - // and the time it was executed, the data structure changed. So we need - // to be safe here. - if (animations.leave && animations.leave.has(tNode.index)) { - const leaveAnimationMap = animations.leave; - const leaveAnimations = leaveAnimationMap.get(tNode.index); - const runningAnimations = []; - if (leaveAnimations) { - for (let index = 0; index < leaveAnimations.animateFns.length; index++) { - const animationFn = leaveAnimations.animateFns[index]; - const {promise} = animationFn(); - runningAnimations.push(promise); + addToAnimationQueue( + injector, + () => { + // it's possible that in the time between when the leave animation was + // and the time it was executed, the data structure changed. So we need + // to be safe here. + if (animations.leave && animations.leave.has(tNode.index)) { + const leaveAnimationMap = animations.leave; + const leaveAnimations = leaveAnimationMap.get(tNode.index); + const runningAnimations = []; + if (leaveAnimations) { + for (let index = 0; index < leaveAnimations.animateFns.length; index++) { + const animationFn = leaveAnimations.animateFns[index]; + const {promise} = animationFn() as ReturnType; + runningAnimations.push(promise); + } + animations.detachedLeaveAnimationFns = undefined; } + animations.running = Promise.allSettled(runningAnimations); + runAfterLeaveAnimations(lView!, callback); + } else { + if (lView) allLeavingAnimations.delete(lView); + callback(false); } - animations.running = Promise.allSettled(runningAnimations); - runAfterLeaveAnimations(lView!, callback); - } else { - if (lView) allLeavingAnimations.delete(lView); - callback(false); - } - }); + }, + animations, + ); } function runAfterLeaveAnimations(lView: LView, callback: Function) { diff --git a/packages/core/test/acceptance/animation_spec.ts b/packages/core/test/acceptance/animation_spec.ts index d6592d8c221..dec9fc95260 100644 --- a/packages/core/test/acceptance/animation_spec.ts +++ b/packages/core/test/acceptance/animation_spec.ts @@ -2057,6 +2057,47 @@ describe('Animation', () => { expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(3); })); + it('should not remove elements when swapping or moving nodes', fakeAsync(() => { + const animateSpy = jasmine.createSpy('animateSpy'); + @Component({ + selector: 'test-cmp', + template: ` +
+ @for (item of items; track item.id) { +

{{ item.id }}

+ } +
+ `, + encapsulation: ViewEncapsulation.None, + }) + class TestComponent { + items = [{id: 1}, {id: 2}, {id: 3}]; + private cd = inject(ChangeDetectorRef); + + animate(event: AnimationCallbackEvent) { + animateSpy(); + event.animationComplete(); + } + + shuffle() { + this.items = this.shuffleArray(this.items); + this.cd.markForCheck(); + } + + shuffleArray(array: readonly T[]): T[] { + return [array[1], array[2], array[0]]; + } + } + TestBed.configureTestingModule({animationsEnabled: true}); + + const fixture = TestBed.createComponent(TestComponent); + const cmp = fixture.componentInstance; + cmp.shuffle(); + fixture.detectChanges(); + expect(animateSpy).not.toHaveBeenCalled(); + expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(3); + })); + it('should not remove elements when child element animations finish', fakeAsync(() => { const animateStyles = ` .fade {