Skip to content

virtual-scroll: fix updating when data changes and add trackBy demos #11085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/cdk-experimental/scrolling/virtual-for-of.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,15 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
private _differs: IterableDiffers,
/** The virtual scrolling viewport that these items are being rendered in. */
@SkipSelf() private _viewport: CdkVirtualScrollViewport) {
this.dataStream.subscribe(data => this._data = data);
this._viewport.renderedRangeStream.subscribe(range => this._onRenderedRangeChange(range));
this.dataStream.subscribe(data => {
this._data = data;
Copy link
Member

@crisbeto crisbeto May 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saving the value like this doesn't feel very rxjs-ey. Perhaps the dataStream and the renderedRangeStream below should be converted into BehaviorSubject and the _data would just become a proxy for dataStream.value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dataStream is created by piping together a bunch of observable operators, is there a way to turn that back into something like a BehaviorSubject?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ben's advice is that if you're creating a BehaviorSubject, there's probably a better way. In this case, wouldn't you want to combineLatest?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least in the case of _data, I'm still going to need to save it, because the latest data value is used in other places besides just _onRenderedDataChange. I could do a combineLatest for the renderedRamgeStream if you guys feel strongly about it. I'd prefer to just leave it as is so it looks the same as the way I'm doing the _data one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel strongly either way

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel too strongly about it either, it just felt weird to have to save the state from inside a subscription.

this._onRenderedDataChange();
});
this._viewport.renderedRangeStream.subscribe(range => {
this._renderedRange = range;
this.viewChange.next(this._renderedRange);
this._onRenderedDataChange();
});
this._viewport.attach(this);
}

Expand Down Expand Up @@ -213,9 +220,10 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
}

/** React to scroll state changes in the viewport. */
private _onRenderedRangeChange(renderedRange: ListRange) {
this._renderedRange = renderedRange;
this.viewChange.next(this._renderedRange);
private _onRenderedDataChange() {
if (!this._renderedRange) {
return;
}
this._renderedItems = this._data.slice(this._renderedRange.start, this._renderedRange.end);
if (!this._differ) {
this._differ = this._differs.find(this._renderedItems).create(this.cdkVirtualForTrackBy);
Expand Down
38 changes: 37 additions & 1 deletion src/demo-app/virtual-scroll/virtual-scroll-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,44 @@ <h2>Fixed size</h2>
<h2>Observable data</h2>

<cdk-virtual-scroll-viewport class="demo-viewport" [itemSize]="50">
<div *cdkVirtualFor="let size of observableData; let i = index" class="demo-item"
<div *cdkVirtualFor="let size of observableData | async; let i = index" class="demo-item"
[style.height.px]="size">
Item #{{i}} - ({{size}}px)
</div>
</cdk-virtual-scroll-viewport>

<h2>No trackBy</h2>

<button (click)="sortBy('name')">Sort by state name</button>
<button (click)="sortBy('capital')">Sort by state capital</button>
<cdk-virtual-scroll-viewport class="demo-viewport" [itemSize]="60">
<div *cdkVirtualFor="let state of statesObservable | async"
class="demo-state-item">
<div class="demo-state">{{state.name}}</div>
<div class="demo-capital">{{state.capital}}</div>
</div>
</cdk-virtual-scroll-viewport>

<h2>trackBy index</h2>

<button (click)="sortBy('name')">Sort by state name</button>
<button (click)="sortBy('capital')">Sort by state capital</button>
<cdk-virtual-scroll-viewport class="demo-viewport" [itemSize]="60">
<div *cdkVirtualFor="let state of statesObservable | async; trackBy: indexTrackFn"
class="demo-state-item">
<div class="demo-state">{{state.name}}</div>
<div class="demo-capital">{{state.capital}}</div>
</div>
</cdk-virtual-scroll-viewport>

<h2>trackBy state name</h2>

<button (click)="sortBy('name')">Sort by state name</button>
<button (click)="sortBy('capital')">Sort by state capital</button>
<cdk-virtual-scroll-viewport class="demo-viewport" [itemSize]="60">
<div *cdkVirtualFor="let state of statesObservable | async; trackBy: nameTrackFn"
class="demo-state-item">
<div class="demo-state">{{state.name}}</div>
<div class="demo-capital">{{state.capital}}</div>
</div>
</cdk-virtual-scroll-viewport>
16 changes: 16 additions & 0 deletions src/demo-app/virtual-scroll/virtual-scroll-demo.scss
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,19 @@
writing-mode: vertical-lr;
}
}

.demo-state-item {
height: 60px;
display: flex;
flex-direction: column;
justify-content: center;
}

.demo-state {
font-size: 20px;
font-weight: 500;
}

.demo-capital {
font-size: 14px;
}
69 changes: 69 additions & 0 deletions src/demo-app/virtual-scroll/virtual-scroll-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
import {Component, ViewEncapsulation} from '@angular/core';
import {BehaviorSubject} from 'rxjs/index';


type State = {
name: string,
capital: string
};


@Component({
moduleId: module.id,
selector: 'virtual-scroll-demo',
Expand All @@ -23,6 +30,61 @@ export class VirtualScrollDemo {
.map((_, i) => (1 + Math.floor((10000 - i) / 1000)) * 20);
randomData = Array(10000).fill(0).map(() => Math.round(Math.random() * 100));
observableData = new BehaviorSubject<number[]>([]);
states = [
{name: 'Alabama', capital: 'Montgomery'},
{name: 'Alaska', capital: 'Juneau'},
{name: 'Arizona', capital: 'Phoenix'},
{name: 'Arkansas', capital: 'Little Rock'},
{name: 'California', capital: 'Sacramento'},
{name: 'Colorado', capital: 'Denver'},
{name: 'Connecticut', capital: 'Hartford'},
{name: 'Delaware', capital: 'Dover'},
{name: 'Florida', capital: 'Tallahassee'},
{name: 'Georgia', capital: 'Atlanta'},
{name: 'Hawaii', capital: 'Honolulu'},
{name: 'Idaho', capital: 'Boise'},
{name: 'Illinois', capital: 'Springfield'},
{name: 'Indiana', capital: 'Indianapolis'},
{name: 'Iowa', capital: 'Des Moines'},
{name: 'Kansas', capital: 'Topeka'},
{name: 'Kentucky', capital: 'Frankfort'},
{name: 'Louisiana', capital: 'Baton Rouge'},
{name: 'Maine', capital: 'Augusta'},
{name: 'Maryland', capital: 'Annapolis'},
{name: 'Massachusetts', capital: 'Boston'},
{name: 'Michigan', capital: 'Lansing'},
{name: 'Minnesota', capital: 'St. Paul'},
{name: 'Mississippi', capital: 'Jackson'},
{name: 'Missouri', capital: 'Jefferson City'},
{name: 'Montana', capital: 'Helena'},
{name: 'Nebraska', capital: 'Lincoln'},
{name: 'Nevada', capital: 'Carson City'},
{name: 'New Hampshire', capital: 'Concord'},
{name: 'New Jersey', capital: 'Trenton'},
{name: 'New Mexico', capital: 'Santa Fe'},
{name: 'New York', capital: 'Albany'},
{name: 'North Carolina', capital: 'Raleigh'},
{name: 'North Dakota', capital: 'Bismarck'},
{name: 'Ohio', capital: 'Columbus'},
{name: 'Oklahoma', capital: 'Oklahoma City'},
{name: 'Oregon', capital: 'Salem'},
{name: 'Pennsylvania', capital: 'Harrisburg'},
{name: 'Rhode Island', capital: 'Providence'},
{name: 'South Carolina', capital: 'Columbia'},
{name: 'South Dakota', capital: 'Pierre'},
{name: 'Tennessee', capital: 'Nashville'},
{name: 'Texas', capital: 'Austin'},
{name: 'Utah', capital: 'Salt Lake City'},
{name: 'Vermont', capital: 'Montpelier'},
{name: 'Virginia', capital: 'Richmond'},
{name: 'Washington', capital: 'Olympia'},
{name: 'West Virginia', capital: 'Charleston'},
{name: 'Wisconsin', capital: 'Madison'},
{name: 'Wyoming', capital: 'Cheyenne'},
];
statesObservable = new BehaviorSubject(this.states);
indexTrackFn = (index: number) => index;
nameTrackFn = (_: number, item: State) => item.name;

constructor() {
this.emitData();
Expand All @@ -35,4 +97,11 @@ export class VirtualScrollDemo {
setTimeout(() => this.emitData(), 1000);
}
}

sortBy(prop: 'name' | 'capital') {
this.statesObservable.next(this.states.map(s => ({...s})).sort((a, b) => {
const aProp = a[prop], bProp = b[prop];
return aProp < bProp ? -1 : (aProp > bProp ? 1 : 0);
}));
}
}