Skip to content

Commit 4db1cfe

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: #64818 fixes: #64730
1 parent a0fe177 commit 4db1cfe

File tree

2 files changed

+51
-5
lines changed

2 files changed

+51
-5
lines changed

packages/core/src/render3/list_reconciliation.ts

Lines changed: 10 additions & 5 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 {
@@ -180,8 +180,9 @@ export function reconcile<T, V>(
180180
const liveStartKey = trackByFn(liveStartIdx, liveStartValue);
181181
const liveEndKey = trackByFn(liveEndIdx, liveEndValue);
182182
const newStartKey = trackByFn(liveStartIdx, newStartValue);
183-
if (Object.is(newStartKey, liveEndKey)) {
184-
const newEndKey = trackByFn(newEndIdx, newEndValue);
183+
const newEndKey = trackByFn(liveEndIdx, newEndValue);
184+
185+
if (Object.is(newStartKey, liveEndKey) || Object.is(newEndKey, liveStartKey)) {
185186
// detect swap on both ends;
186187
if (Object.is(newEndKey, liveStartKey)) {
187188
liveCollection.swap(liveStartIdx, liveEndIdx);
@@ -223,6 +224,7 @@ export function reconcile<T, V>(
223224
// We know that the new item exists later on in old collection but we don't know its index
224225
// and as the consequence can't move it (don't know where to find it). Detach the old item,
225226
// hoping that it unlocks the fast path again.
227+
226228
detachedItems.set(liveStartKey, liveCollection.detach(liveStartIdx));
227229
liveEndIdx--;
228230
}
@@ -292,7 +294,10 @@ export function reconcile<T, V>(
292294
} else {
293295
// it is a move forward - detach the current item without advancing in collections
294296
const liveKey = trackByFn(liveStartIdx, liveValue);
295-
detachedItems.set(liveKey, liveCollection.detach(liveStartIdx));
297+
detachedItems.set(
298+
liveKey,
299+
liveCollection.detach(liveStartIdx, false /* skipLeaveAnimations */),
300+
);
296301
liveEndIdx--;
297302
}
298303
}

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)