Skip to content

Commit 869baa2

Browse files
fix(core): skip leave animations on view swaps
We accounted for skipping leave animations during moves, but not swaps. This accounts for the swap cases. Essentially any time a detach is fired by liveCollection during a swap or move, we want to skip leave animations. The only case where we don't want to skip is a destroy action. fixes: angular#64818 fixes: angular#64730
1 parent a0fe177 commit 869baa2

File tree

2 files changed

+50
-3
lines changed

2 files changed

+50
-3
lines changed

packages/core/src/render3/list_reconciliation.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ export abstract class LiveCollection<T, V> {
3636
swap(index1: number, index2: number): void {
3737
const startIdx = Math.min(index1, index2);
3838
const endIdx = Math.max(index1, index2);
39-
const endItem = this.detach(endIdx);
39+
const endItem = this.detach(endIdx, true /* skipLeaveAnimations */);
4040
if (endIdx - startIdx > 1) {
41-
const startItem = this.detach(startIdx);
41+
const startItem = this.detach(startIdx, true /* skipLeaveAnimations */);
4242
this.attach(startIdx, endItem);
4343
this.attach(endIdx, startItem);
4444
} else {
@@ -223,7 +223,13 @@ export function reconcile<T, V>(
223223
// We know that the new item exists later on in old collection but we don't know its index
224224
// and as the consequence can't move it (don't know where to find it). Detach the old item,
225225
// hoping that it unlocks the fast path again.
226-
detachedItems.set(liveStartKey, liveCollection.detach(liveStartIdx));
226+
227+
// we don't always know if we should skip leave animations here. We need to actually check
228+
// to see if the item is actually new in the new collection to know if it should skip leave
229+
// animations.
230+
const skipLeaveAnimations =
231+
newCollection.find((itm) => liveStartValue === itm) !== undefined;
232+
detachedItems.set(liveStartKey, liveCollection.detach(liveStartIdx, skipLeaveAnimations));
227233
liveEndIdx--;
228234
}
229235
}

packages/core/test/acceptance/animation_spec.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2057,6 +2057,47 @@ describe('Animation', () => {
20572057
expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(3);
20582058
}));
20592059

2060+
it('should not remove elements when swapping or moving nodes', fakeAsync(() => {
2061+
const animateSpy = jasmine.createSpy('animateSpy');
2062+
@Component({
2063+
selector: 'test-cmp',
2064+
template: `
2065+
<div>
2066+
@for (item of items; track item.id) {
2067+
<p (animate.leave)="animate($event)" #el>{{ item.id }}</p>
2068+
}
2069+
</div>
2070+
`,
2071+
encapsulation: ViewEncapsulation.None,
2072+
})
2073+
class TestComponent {
2074+
items = [{id: 1}, {id: 2}, {id: 3}];
2075+
private cd = inject(ChangeDetectorRef);
2076+
2077+
animate(event: AnimationCallbackEvent) {
2078+
animateSpy();
2079+
event.animationComplete();
2080+
}
2081+
2082+
shuffle() {
2083+
this.items = this.shuffleArray(this.items);
2084+
this.cd.markForCheck();
2085+
}
2086+
2087+
shuffleArray<T>(array: readonly T[]): T[] {
2088+
return [array[1], array[2], array[0]];
2089+
}
2090+
}
2091+
TestBed.configureTestingModule({animationsEnabled: true});
2092+
2093+
const fixture = TestBed.createComponent(TestComponent);
2094+
const cmp = fixture.componentInstance;
2095+
cmp.shuffle();
2096+
fixture.detectChanges();
2097+
expect(animateSpy).not.toHaveBeenCalled();
2098+
expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(3);
2099+
}));
2100+
20602101
it('should not remove elements when child element animations finish', fakeAsync(() => {
20612102
const animateStyles = `
20622103
.fade {

0 commit comments

Comments
 (0)