Skip to content

Commit

Permalink
fix(cdk/scrolling): virtual scroll not picking up trackBy function wh…
Browse files Browse the repository at this point in the history
…en items come in after init (#21335)

The virtual scroll only creates its `IterableDiffer` once, which means that if the
`trackBy` function comes in at a later point (e.g. the input changed or the
initialization order is different), it won't be picked up.

These changes make it so the differ always has a `trackBy` which then delegates
to the user-provided one or falls back to the default.

Fixes #21281.

(cherry picked from commit 43081d9)
  • Loading branch information
crisbeto authored and mmalerba committed Dec 17, 2020
1 parent 9cd2f47 commit 2a6862d
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 23 deletions.
1 change: 1 addition & 0 deletions src/cdk/scrolling/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ ng_test_library(
"//src/cdk/bidi",
"//src/cdk/collections",
"//src/cdk/testing/private",
"@npm//@angular/common",
"@npm//rxjs",
],
)
Expand Down
10 changes: 7 additions & 3 deletions src/cdk/scrolling/virtual-for-of.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,11 @@ export class CdkVirtualForOf<T> implements
}
this._renderedItems = this._data.slice(this._renderedRange.start, this._renderedRange.end);
if (!this._differ) {
this._differ = this._differs.find(this._renderedItems).create(this.cdkVirtualForTrackBy);
// Use a wrapper function for the `trackBy` so any new values are
// picked up automatically without having to recreate the differ.
this._differ = this._differs.find(this._renderedItems).create((index, item) => {
return this.cdkVirtualForTrackBy ? this.cdkVirtualForTrackBy(index, item) : item;
});
}
this._needsUpdate = true;
}
Expand All @@ -310,7 +314,7 @@ export class CdkVirtualForOf<T> implements
const count = this._data.length;
let i = this._viewContainerRef.length;
while (i--) {
let view = this._viewContainerRef.get(i) as EmbeddedViewRef<CdkVirtualForOfContext<T>>;
const view = this._viewContainerRef.get(i) as EmbeddedViewRef<CdkVirtualForOfContext<T>>;
view.context.index = this._renderedRange.start + i;
view.context.count = count;
this._updateComputedContextProperties(view.context);
Expand All @@ -324,7 +328,7 @@ export class CdkVirtualForOf<T> implements
changes,
this._viewContainerRef,
(record: IterableChangeRecord<T>,
adjustedPreviousIndex: number | null,
_adjustedPreviousIndex: number | null,
currentIndex: number | null) => this._getEmbeddedViewArgs(record, currentIndex!),
(record) => record.item);

Expand Down
106 changes: 86 additions & 20 deletions src/cdk/scrolling/virtual-scroll-viewport.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import {
ScrollDispatcher,
ScrollingModule
} from '@angular/cdk/scrolling';
import {CommonModule} from '@angular/common';
import {dispatchFakeEvent} from '@angular/cdk/testing/private';
import {
Component,
Input,
NgZone,
TrackByFunction,
ViewChild,
Expand All @@ -29,7 +29,7 @@ import {animationFrameScheduler, Subject} from 'rxjs';


describe('CdkVirtualScrollViewport', () => {
describe ('with FixedSizeVirtualScrollStrategy', () => {
describe('with FixedSizeVirtualScrollStrategy', () => {
let fixture: ComponentFixture<FixedSizeVirtualScroll>;
let testComponent: FixedSizeVirtualScroll;
let viewport: CdkVirtualScrollViewport;
Expand Down Expand Up @@ -899,6 +899,35 @@ describe('CdkVirtualScrollViewport', () => {
.toEqual(['0', '1', '2', '3', '4', '5', '6', '7']);
}));
});

describe('with delayed initialization', () => {
let fixture: ComponentFixture<DelayedInitializationVirtualScroll>;
let testComponent: DelayedInitializationVirtualScroll;
let viewport: CdkVirtualScrollViewport;

beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
imports: [ScrollingModule, CommonModule],
declarations: [DelayedInitializationVirtualScroll],
}).compileComponents();
fixture = TestBed.createComponent(DelayedInitializationVirtualScroll);
testComponent = fixture.componentInstance;
viewport = testComponent.viewport;
}));

it('should call custom trackBy when virtual for is added after init', fakeAsync(() => {
finishInit(fixture);
expect(testComponent.trackBy).not.toHaveBeenCalled();

testComponent.renderVirtualFor = true;
fixture.detectChanges();
triggerScroll(viewport, testComponent.itemSize * 5);
fixture.detectChanges();
flush();

expect(testComponent.trackBy).toHaveBeenCalled();
}));
});
});


Expand Down Expand Up @@ -973,15 +1002,15 @@ class FixedSizeVirtualScroll {
// Casting virtualForOf as any so we can spy on private methods
@ViewChild(CdkVirtualForOf, {static: true}) virtualForOf: any;

@Input() orientation = 'vertical';
@Input() viewportSize = 200;
@Input() viewportCrossSize = 100;
@Input() itemSize = 50;
@Input() minBufferPx = 0;
@Input() maxBufferPx = 0;
@Input() items = Array(10).fill(0).map((_, i) => i);
@Input() trackBy: TrackByFunction<number>;
@Input() templateCacheSize = 20;
orientation = 'vertical';
viewportSize = 200;
viewportCrossSize = 100;
itemSize = 50;
minBufferPx = 0;
maxBufferPx = 0;
items = Array(10).fill(0).map((_, i) => i);
trackBy: TrackByFunction<number>;
templateCacheSize = 20;

scrolledToIndex = 0;
hasMargin = false;
Expand Down Expand Up @@ -1033,15 +1062,15 @@ class FixedSizeVirtualScroll {
class FixedSizeVirtualScrollWithRtlDirection {
@ViewChild(CdkVirtualScrollViewport, {static: true}) viewport: CdkVirtualScrollViewport;

@Input() orientation = 'vertical';
@Input() viewportSize = 200;
@Input() viewportCrossSize = 100;
@Input() itemSize = 50;
@Input() minBufferPx = 0;
@Input() maxBufferPx = 0;
@Input() items = Array(10).fill(0).map((_, i) => i);
@Input() trackBy: TrackByFunction<number>;
@Input() templateCacheSize = 20;
orientation = 'vertical';
viewportSize = 200;
viewportCrossSize = 100;
itemSize = 50;
minBufferPx = 0;
maxBufferPx = 0;
items = Array(10).fill(0).map((_, i) => i);
trackBy: TrackByFunction<number>;
templateCacheSize = 20;

scrolledToIndex = 0;

Expand Down Expand Up @@ -1116,3 +1145,40 @@ class VirtualScrollWithItemInjectingViewContainer {
items = Array(20000).fill(0).map((_, i) => i);
}


@Component({
template: `
<cdk-virtual-scroll-viewport [itemSize]="itemSize">
<ng-container *ngIf="renderVirtualFor">
<div class="item" *cdkVirtualFor="let item of items; trackBy: trackBy">{{item}}</div>
</ng-container>
</cdk-virtual-scroll-viewport>
`,
styles: [`
.cdk-virtual-scroll-content-wrapper {
display: flex;
flex-direction: column;
}
.cdk-virtual-scroll-viewport {
width: 200px;
height: 200px;
background-color: #f5f5f5;
}
.item {
width: 100%;
height: 50px;
box-sizing: border-box;
border: 1px dashed #ccc;
}
`],
encapsulation: ViewEncapsulation.None,
})
class DelayedInitializationVirtualScroll {
@ViewChild(CdkVirtualScrollViewport, {static: true}) viewport: CdkVirtualScrollViewport;
itemSize = 50;
items = Array(20000).fill(0).map((_, i) => i);
trackBy = jasmine.createSpy('trackBy').and.callFake((item: unknown) => item);
renderVirtualFor = false;
}

0 comments on commit 2a6862d

Please sign in to comment.