Skip to content

virtual-scroll: add onContentRendered hook to VirtualScrollStrategy #10290

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 2 commits into from
Mar 7, 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
37 changes: 28 additions & 9 deletions src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,10 @@ export class ItemSizeAverager {
* @param size The measured size of the given range in pixels.
*/
addSample(range: ListRange, size: number) {
const weight = range.end - range.start;
const newTotalWeight = this._totalWeight + weight;
const newTotalWeight = this._totalWeight + range.end - range.start;
if (newTotalWeight) {
const newAverageItemSize =
(size * weight + this._averageItemSize * this._totalWeight) / newTotalWeight;
(size + this._averageItemSize * this._totalWeight) / newTotalWeight;
if (newAverageItemSize) {
this._averageItemSize = newAverageItemSize;
this._totalWeight = newTotalWeight;
Expand Down Expand Up @@ -87,7 +86,6 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
*/
attach(viewport: CdkVirtualScrollViewport) {
this._viewport = viewport;
this._updateTotalContentSize();
this._renderContentForOffset(this._viewport.measureScrollOffset());
}

Expand All @@ -96,21 +94,27 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
this._viewport = null;
}

/** Called when the viewport is scrolled. */
/** Implemented as part of VirtualScrollStrategy. */
onContentScrolled() {
if (this._viewport) {
this._renderContentForOffset(this._viewport.measureScrollOffset());
}
}

/** Called when the length of the data changes. */
/** Implemented as part of VirtualScrollStrategy. */
onDataLengthChanged() {
if (this._viewport) {
this._updateTotalContentSize();
this._renderContentForOffset(this._viewport.measureScrollOffset());
}
}

/** Implemented as part of VirtualScrollStrategy. */
onContentRendered() {
if (this._viewport) {
this._checkRenderedContentSize();
}
}

/**
* Update the buffer parameters.
* @param minBufferPx The minimum amount of buffer rendered beyond the viewport (in pixels).
Expand All @@ -122,6 +126,17 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
this._addBufferPx = addBufferPx;
}

/**
* Checks the size of the currently rendered content and uses it to update the estimated item size
* and estimated total content size.
*/
private _checkRenderedContentSize() {
const viewport = this._viewport!;
const renderedContentSize = viewport.measureRenderedContentSize();
this._averager.addSample(viewport.getRenderedRange(), renderedContentSize);
this._updateTotalContentSize(renderedContentSize);
}

/**
* Render the content that we estimate should be shown for the given scroll offset.
* Note: must not be called if `this._viewport` is null
Expand Down Expand Up @@ -179,9 +194,13 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy {
}

/** Update the viewport's total content size. */
private _updateTotalContentSize() {
private _updateTotalContentSize(renderedContentSize: number) {
const viewport = this._viewport!;
viewport.setTotalContentSize(viewport.getDataLength() * this._averager.getAverageItemSize());
const renderedRange = viewport.getRenderedRange();
const totalSize = renderedContentSize +
(viewport.getDataLength() - (renderedRange.end - renderedRange.start)) *
this._averager.getAverageItemSize();
viewport.setTotalContentSize(totalSize);
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/cdk-experimental/scrolling/fixed-size-virtual-scroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ export class FixedSizeVirtualScrollStrategy implements VirtualScrollStrategy {
this._updateRenderedRange();
}

/** Called when the range of items rendered in the DOM has changed. */
onContentRendered() { /* no-op */ }

/** Update the viewport's total content size. */
private _updateTotalContentSize() {
if (!this._viewport) {
Expand Down
3 changes: 3 additions & 0 deletions src/cdk-experimental/scrolling/virtual-scroll-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,7 @@ export interface VirtualScrollStrategy {

/** Called when the length of the data changes. */
onDataLengthChanged();

/** Called when the range of items rendered in the DOM has changed. */
onContentRendered();
}
15 changes: 15 additions & 0 deletions src/cdk-experimental/scrolling/virtual-scroll-viewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
import {Observable} from 'rxjs/Observable';
import {fromEvent} from 'rxjs/observable/fromEvent';
import {sampleTime} from 'rxjs/operators/sampleTime';
import {take} from 'rxjs/operators/take';
import {takeUntil} from 'rxjs/operators/takeUntil';
import {animationFrame} from 'rxjs/scheduler/animationFrame';
import {Subject} from 'rxjs/Subject';
Expand Down Expand Up @@ -99,6 +100,11 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
return this._viewportSize;
}

/** Get the current rendered range of items. */
getRenderedRange(): ListRange {
return this._renderedRange;
}

// TODO(mmalebra): Consider calling `detectChanges()` directly rather than the methods below.

/**
Expand All @@ -122,6 +128,9 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
this._ngZone.run(() => {
this._renderedRangeSubject.next(this._renderedRange = range);
this._changeDetectorRef.markForCheck();
this._ngZone.runOutsideAngular(() => this._ngZone.onStable.pipe(take(1)).subscribe(() => {
Copy link
Member

Choose a reason for hiding this comment

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

It seems a little weird to run outside Angular and then waiting for the zone to stabilize. What about flipping them around like:

ngZone.onStable.pipe(take(1)).subscribe(() => {
  ngZone.runOutsideAngular(() => onContentCentered());
});

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't subscribing within the zone cause another change detection when the zone stabilizes?

Copy link
Member

Choose a reason for hiding this comment

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

Does NgZone patch into the observable subscriptions? AFAIK it only does browser APIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jelbourn I was worried about that too, but I tested by creating a component that did:

ngAfterContentChecked() {
  this.ngZone.onStable.subscribe(() => {})
}

It didn't go into a death spiral, so it looks like @crisbeto's suggestion would work.

To me it seems clearer to have it the way it is though. It makes it obvious that I don't want any change detection as a result of this subscription.

this._scrollStrategy.onContentRendered();
}));
});
}
}
Expand Down Expand Up @@ -169,6 +178,12 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy {
this.elementRef.nativeElement.scrollLeft : this.elementRef.nativeElement.scrollTop;
}

/** Measure the combined size of all of the rendered items. */
measureRenderedContentSize() {
const contentEl = this._contentWrapper.nativeElement;
return this.orientation === 'horizontal' ? contentEl.offsetWidth : contentEl.offsetHeight;
}

ngOnInit() {
Promise.resolve().then(() => {
this._viewportSize = this.orientation === 'horizontal' ?
Expand Down