From ba84d5b3f0a249c70f01054a696eed7211e81087 Mon Sep 17 00:00:00 2001 From: mmalerba Date: Mon, 13 Aug 2018 09:51:18 -0700 Subject: [PATCH] feat(virtual-scroll): change fixed strategy to use min & max buffer (#12557) * feat(virtual-scroll): change fixed strategy to use min & max buffer * address comments --- .../scrolling/auto-size-virtual-scroll.ts | 33 ++++--- src/cdk-experimental/scrolling/scrolling.md | 6 +- .../scrolling/virtual-scroll-viewport.spec.ts | 12 ++- .../scrolling/fixed-size-virtual-scroll.ts | 99 +++++++++++-------- src/cdk/scrolling/scrolling.md | 18 +++- .../scrolling/virtual-scroll-viewport.spec.ts | 58 ++++++++--- src/cdk/scrolling/virtual-scroll-viewport.ts | 15 +-- 7 files changed, 152 insertions(+), 89 deletions(-) diff --git a/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts b/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts index 5dfa2c16ea9..efe0a63f217 100644 --- a/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts +++ b/src/cdk-experimental/scrolling/auto-size-virtual-scroll.ts @@ -83,7 +83,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy { private _minBufferPx: number; /** The number of buffer items to render beyond the edge of the viewport (in pixels). */ - private _addBufferPx: number; + private _maxBufferPx: number; /** The estimator used to estimate the size of unseen items. */ private _averager: ItemSizeAverager; @@ -107,14 +107,14 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy { /** * @param minBufferPx The minimum amount of buffer rendered beyond the viewport (in pixels). * If the amount of buffer dips below this number, more items will be rendered. - * @param addBufferPx The number of pixels worth of buffer to shoot for when rendering new items. + * @param maxBufferPx The number of pixels worth of buffer to shoot for when rendering new items. * If the actual amount turns out to be less it will not necessarily trigger an additional * rendering cycle (as long as the amount of buffer is still greater than `minBufferPx`). * @param averager The averager used to estimate the size of unseen items. */ - constructor(minBufferPx: number, addBufferPx: number, averager = new ItemSizeAverager()) { + constructor(minBufferPx: number, maxBufferPx: number, averager = new ItemSizeAverager()) { this._minBufferPx = minBufferPx; - this._addBufferPx = addBufferPx; + this._maxBufferPx = maxBufferPx; this._averager = averager; } @@ -172,12 +172,15 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy { /** * Update the buffer parameters. * @param minBufferPx The minimum amount of buffer rendered beyond the viewport (in pixels). - * @param addBufferPx The number of buffer items to render beyond the edge of the viewport (in + * @param maxBufferPx The number of buffer items to render beyond the edge of the viewport (in * pixels). */ - updateBufferSize(minBufferPx: number, addBufferPx: number) { + updateBufferSize(minBufferPx: number, maxBufferPx: number) { + if (maxBufferPx < minBufferPx) { + throw('CDK virtual scroll: maxBufferPx must be greater than or equal to minBufferPx'); + } this._minBufferPx = minBufferPx; - this._addBufferPx = addBufferPx; + this._maxBufferPx = maxBufferPx; } /** Update the rendered content after the user scrolls. */ @@ -242,8 +245,8 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy { } else { // The number of new items to render on the side the user is scrolling towards. Rather than // just filling the underscan space, we actually fill enough to have a buffer size of - // `addBufferPx`. This gives us a little wiggle room in case our item size estimate is off. - const addItems = Math.max(0, Math.ceil((underscan - this._minBufferPx + this._addBufferPx) / + // `maxBufferPx`. This gives us a little wiggle room in case our item size estimate is off. + const addItems = Math.max(0, Math.ceil((underscan - this._minBufferPx + this._maxBufferPx) / this._averager.getAverageItemSize())); // The amount of filled space beyond what is necessary on the side the user is scrolling // away from. @@ -361,7 +364,7 @@ export class AutoSizeVirtualScrollStrategy implements VirtualScrollStrategy { const itemSize = this._averager.getAverageItemSize(); const firstVisibleIndex = Math.min(viewport.getDataLength() - 1, Math.floor(scrollOffset / itemSize)); - const bufferSize = Math.ceil(this._addBufferPx / itemSize); + const bufferSize = Math.ceil(this._maxBufferPx / itemSize); const range = this._expandRange( this._getVisibleRangeForIndex(firstVisibleIndex), bufferSize, bufferSize); @@ -456,14 +459,14 @@ export class CdkAutoSizeVirtualScroll implements OnChanges { * Defaults to 200px. */ @Input() - get addBufferPx(): number { return this._addBufferPx; } - set addBufferPx(value: number) { this._addBufferPx = coerceNumberProperty(value); } - _addBufferPx = 200; + get maxBufferPx(): number { return this._maxBufferPx; } + set maxBufferPx(value: number) { this._maxBufferPx = coerceNumberProperty(value); } + _maxBufferPx = 200; /** The scroll strategy used by this directive. */ - _scrollStrategy = new AutoSizeVirtualScrollStrategy(this.minBufferPx, this.addBufferPx); + _scrollStrategy = new AutoSizeVirtualScrollStrategy(this.minBufferPx, this.maxBufferPx); ngOnChanges() { - this._scrollStrategy.updateBufferSize(this.minBufferPx, this.addBufferPx); + this._scrollStrategy.updateBufferSize(this.minBufferPx, this.maxBufferPx); } } diff --git a/src/cdk-experimental/scrolling/scrolling.md b/src/cdk-experimental/scrolling/scrolling.md index 75c6a168b84..fb212541098 100644 --- a/src/cdk-experimental/scrolling/scrolling.md +++ b/src/cdk-experimental/scrolling/scrolling.md @@ -11,19 +11,19 @@ This can be added to your viewport by using the `autosize` directive. ``` -The `autosize` strategy is configured through two inputs: `minBufferPx` and `addBufferPx`. +The `autosize` strategy is configured through two inputs: `minBufferPx` and `maxBufferPx`. **`minBufferPx`** determines the minimum space outside virtual scrolling viewport that will be filled with content. Increasing this will increase the amount of content a user will see before more content must be rendered. However, too large a value will cause more content to be rendered than is necessary. -**`addBufferPx`** determines the amount of content that will be added incrementally as the viewport +**`maxBufferPx`** determines the amount of content that will be added incrementally as the viewport is scrolled. This should be greater than the size of `minBufferPx` so that one "render" is needed at a time. ```html - + ... ``` diff --git a/src/cdk-experimental/scrolling/virtual-scroll-viewport.spec.ts b/src/cdk-experimental/scrolling/virtual-scroll-viewport.spec.ts index cbe7552a170..8bc7284c4f5 100644 --- a/src/cdk-experimental/scrolling/virtual-scroll-viewport.spec.ts +++ b/src/cdk-experimental/scrolling/virtual-scroll-viewport.spec.ts @@ -40,6 +40,12 @@ describe('CdkVirtualScrollViewport', () => { 'should render 4 items to fill 200px space based on 50px estimate from first item'); })); + it('should throw if maxBufferPx is less than minBufferPx', fakeAsync(() => { + testComponent.minBufferPx = 100; + testComponent.maxBufferPx = 99; + expect(() => finishInit(fixture)).toThrow(); + })); + // TODO(mmalerba): Add test that it corrects the initial render if it didn't render enough, // once it actually does that. }); @@ -61,7 +67,7 @@ function finishInit(fixture: ComponentFixture) { @Component({ template: `
0) { + newRange.end = Math.min(dataLength, newRange.end + expandEnd); + newRange.start = Math.max(0, + Math.floor(firstVisibleIndex - this._minBufferPx / this._itemSize)); + } + } } - const start = Math.max(0, range.start - expandStart); - const end = Math.min(this._viewport.getDataLength(), range.end + expandEnd); - return {start, end}; + this._viewport.setRenderedRange(newRange); + this._viewport.setRenderedContentOffset(this._itemSize * newRange.start); + this._scrolledIndexChange.next(Math.floor(firstVisibleIndex)); } } @@ -172,18 +180,27 @@ export class CdkFixedSizeVirtualScroll implements OnChanges { _itemSize = 20; /** - * The number of extra elements to render on either side of the scrolling viewport. - * Defaults to 5 elements. + * The minimum amount of buffer rendered beyond the viewport (in pixels). + * If the amount of buffer dips below this number, more items will be rendered. Defaults to 100px. */ @Input() - get bufferSize(): number { return this._bufferSize; } - set bufferSize(value: number) { this._bufferSize = coerceNumberProperty(value); } - _bufferSize = 5; + get minBufferPx(): number { return this._minBufferPx; } + set minBufferPx(value: number) { this._minBufferPx = coerceNumberProperty(value); } + _minBufferPx = 100; + + /** + * The number of pixels worth of buffer to render for when rendering new items. Defaults to 200px. + */ + @Input() + get maxBufferPx(): number { return this._maxBufferPx; } + set maxBufferPx(value: number) { this._maxBufferPx = coerceNumberProperty(value); } + _maxBufferPx = 200; /** The scroll strategy used by this directive. */ - _scrollStrategy = new FixedSizeVirtualScrollStrategy(this.itemSize, this.bufferSize); + _scrollStrategy = + new FixedSizeVirtualScrollStrategy(this.itemSize, this.minBufferPx, this.maxBufferPx); ngOnChanges() { - this._scrollStrategy.updateItemAndBufferSize(this.itemSize, this.bufferSize); + this._scrollStrategy.updateItemAndBufferSize(this.itemSize, this.minBufferPx, this.maxBufferPx); } } diff --git a/src/cdk/scrolling/scrolling.md b/src/cdk/scrolling/scrolling.md index 281002e54ec..eb875979e2a 100644 --- a/src/cdk/scrolling/scrolling.md +++ b/src/cdk/scrolling/scrolling.md @@ -94,12 +94,22 @@ that it allows for better performance, since items do not need to be measured as ``` -The fixed size strategy also supports setting the buffer size, i.e. the number of items rendered -beyond the edge of the viewport. This can be adjusted by setting the `bufferSize` input. If -`bufferSize` is not specified it defaults to 5 items. +The fixed size strategy also supports setting a couple of buffer parameters that determine how much +extra content is rendered beyond what is visible in the viewport. The first of these parameters is +`minBufferPx`. The `minBufferPx` is the minimum amount of content buffer (in pixels) that the +viewport must render. If the viewport ever detects that there is less buffered content it will +immediately render more. The second buffer parameter is `maxBufferPx`. This tells the viewport how +much buffer space to render back up to when it detects that more buffer is required. + +The interaction of these two buffer parameters can be best illustrated with an example. Supposed +that we have the following parameters: `itemSize = 50`, `minBufferPx = 100`, `maxBufferPx = 250`. As +the user is scrolling through the content the viewport detects that there is only `90px` of buffer +remaining. Since this is below `minBufferPx` the viewport must render more buffer. It must render at +least enough buffer to get back to `maxBufferPx`. In this case, it renders 4 items (an additional +`200px`) to bring the total buffer size to `290px`, back above `maxBufferPx`. ```html - + ... ``` diff --git a/src/cdk/scrolling/virtual-scroll-viewport.spec.ts b/src/cdk/scrolling/virtual-scroll-viewport.spec.ts index f4aeb2cc7fb..7f5379db242 100644 --- a/src/cdk/scrolling/virtual-scroll-viewport.spec.ts +++ b/src/cdk/scrolling/virtual-scroll-viewport.spec.ts @@ -226,7 +226,7 @@ describe('CdkVirtualScrollViewport', () => { const maxOffset = testComponent.itemSize * testComponent.items.length - testComponent.viewportSize; - for (let offset = 0; offset <= maxOffset; offset += 10) { + for (let offset = 1; offset <= maxOffset; offset += 10) { triggerScroll(viewport, offset); fixture.detectChanges(); flush(); @@ -252,7 +252,7 @@ describe('CdkVirtualScrollViewport', () => { const maxOffset = testComponent.itemSize * testComponent.items.length - testComponent.viewportSize; - for (let offset = maxOffset; offset >= 0; offset -= 10) { + for (let offset = maxOffset - 1; offset >= 0; offset -= 10) { triggerScroll(viewport, offset); fixture.detectChanges(); flush(); @@ -274,7 +274,8 @@ describe('CdkVirtualScrollViewport', () => { })); it('should render buffer element at the end when scrolled to the top', fakeAsync(() => { - testComponent.bufferSize = 1; + testComponent.minBufferPx = testComponent.itemSize; + testComponent.maxBufferPx = testComponent.itemSize; finishInit(fixture); expect(viewport.getRenderedRange()).toEqual({start: 0, end: 5}, @@ -284,7 +285,8 @@ describe('CdkVirtualScrollViewport', () => { it('should render buffer element at the start and end when scrolled to the middle', fakeAsync(() => { - testComponent.bufferSize = 1; + testComponent.minBufferPx = testComponent.itemSize; + testComponent.maxBufferPx = testComponent.itemSize; finishInit(fixture); triggerScroll(viewport, testComponent.itemSize * 2); fixture.detectChanges(); @@ -296,7 +298,8 @@ describe('CdkVirtualScrollViewport', () => { })); it('should render buffer element at the start when scrolled to the bottom', fakeAsync(() => { - testComponent.bufferSize = 1; + testComponent.minBufferPx = testComponent.itemSize; + testComponent.maxBufferPx = testComponent.itemSize; finishInit(fixture); triggerScroll(viewport, testComponent.itemSize * 6); fixture.detectChanges(); @@ -333,7 +336,8 @@ describe('CdkVirtualScrollViewport', () => { expect(viewport.getRenderedRange()) .toEqual({start: 2, end: 6}, 'should render 4 50px items to fill 200px space'); - testComponent.bufferSize = 1; + testComponent.minBufferPx = testComponent.itemSize; + testComponent.maxBufferPx = testComponent.itemSize; fixture.detectChanges(); flush(); @@ -368,7 +372,7 @@ describe('CdkVirtualScrollViewport', () => { const maxOffset = testComponent.itemSize * testComponent.items.length - testComponent.viewportSize; - for (let offset = 0; offset <= maxOffset; offset += 10) { + for (let offset = 1; offset <= maxOffset; offset += 10) { triggerScroll(viewport, offset); fixture.detectChanges(); flush(); @@ -395,7 +399,7 @@ describe('CdkVirtualScrollViewport', () => { const maxOffset = testComponent.itemSize * testComponent.items.length - testComponent.viewportSize; - for (let offset = maxOffset; offset >= 0; offset -= 10) { + for (let offset = maxOffset - 1; offset >= 0; offset -= 10) { triggerScroll(viewport, offset); fixture.detectChanges(); flush(); @@ -551,6 +555,35 @@ describe('CdkVirtualScrollViewport', () => { // create a new view for each one. expect(testComponent.virtualForViewContainer.createEmbeddedView).toHaveBeenCalledTimes(5); })); + + it('should render up to maxBufferPx when buffer dips below minBufferPx', fakeAsync(() => { + testComponent.minBufferPx = testComponent.itemSize; + testComponent.maxBufferPx = testComponent.itemSize * 2; + finishInit(fixture); + + expect(viewport.getRenderedRange()) + .toEqual({start: 0, end: 6}, 'should have 2 buffer items initially'); + + triggerScroll(viewport, 50); + fixture.detectChanges(); + flush(); + + expect(viewport.getRenderedRange()) + .toEqual({start: 0, end: 6}, 'should not render additional buffer yet'); + + triggerScroll(viewport, 51); + fixture.detectChanges(); + flush(); + + expect(viewport.getRenderedRange()) + .toEqual({start: 0, end: 8}, 'should render 2 more buffer items'); + })); + + it('should throw if maxBufferPx is less than minBufferPx', fakeAsync(() => { + testComponent.minBufferPx = 100; + testComponent.maxBufferPx = 99; + expect(() => finishInit(fixture)).toThrow(); + })); }); }); @@ -583,9 +616,9 @@ function triggerScroll(viewport: CdkVirtualScrollViewport, offset?: number) { @Component({ template: ` + [itemSize]="itemSize" [minBufferPx]="minBufferPx" [maxBufferPx]="maxBufferPx" + [orientation]="orientation" [style.height.px]="viewportHeight" + [style.width.px]="viewportWidth" (scrolledIndexChange)="scrolledToIndex = $event">
i); @Input() trackBy; @Input() templateCacheSize = 20; diff --git a/src/cdk/scrolling/virtual-scroll-viewport.ts b/src/cdk/scrolling/virtual-scroll-viewport.ts index 530831e4bbe..85322b64bca 100644 --- a/src/cdk/scrolling/virtual-scroll-viewport.ts +++ b/src/cdk/scrolling/virtual-scroll-viewport.ts @@ -23,7 +23,7 @@ import { ViewEncapsulation, } from '@angular/core'; import {animationFrameScheduler, fromEvent, Observable, Subject} from 'rxjs'; -import {sample, sampleTime, takeUntil} from 'rxjs/operators'; +import {sampleTime, takeUntil} from 'rxjs/operators'; import {CdkVirtualForOf} from './virtual-for-of'; import {VIRTUAL_SCROLL_STRATEGY, VirtualScrollStrategy} from './virtual-scroll-strategy'; @@ -55,9 +55,6 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy { /** Emits when the rendered range changes. */ private _renderedRangeSubject = new Subject(); - /** Emits when a change detection cycle completes. */ - private _changeDetectionComplete = new Subject(); - /** The direction the viewport scrolls. */ @Input() orientation: 'horizontal' | 'vertical' = 'vertical'; @@ -66,10 +63,9 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy { // depending on how the strategy calculates the scrolled index, it may come at a cost to // performance. /** Emits when the index of the first element visible in the viewport changes. */ - @Output() scrolledIndexChange: Observable = - Observable.create(observer => this._scrollStrategy.scrolledIndexChange - .pipe(sample(this._changeDetectionComplete)) - .subscribe(observer)); + @Output() scrolledIndexChange: Observable = Observable.create(observer => + this._scrollStrategy.scrolledIndexChange.subscribe(index => + Promise.resolve().then(() => this._ngZone.run(() => observer.next(index))))); /** The element that wraps the rendered content. */ @ViewChild('contentWrapper') _contentWrapper: ElementRef; @@ -159,7 +155,6 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy { // Complete all subjects this._renderedRangeSubject.complete(); this._detachedSubject.complete(); - this._changeDetectionComplete.complete(); this._destroyed.complete(); } @@ -386,7 +381,5 @@ export class CdkVirtualScrollViewport implements OnInit, OnDestroy { for (const fn of runAfterChangeDetection) { fn(); } - - this._ngZone.run(() => this._changeDetectionComplete.next()); } }