From 224c90891652ff790f3e1070fa5243499e3acac5 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Tue, 9 Jun 2020 16:08:22 +0200 Subject: [PATCH 1/2] fix(cdk/scrolling): fixed-size-virtual-scroll wrong range and position when items length changes and current scroll is greater than new data length --- .../scrolling/fixed-size-virtual-scroll.ts | 9 +++- .../scrolling/virtual-scroll-viewport.spec.ts | 53 +++++++++++++++++-- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/src/cdk/scrolling/fixed-size-virtual-scroll.ts b/src/cdk/scrolling/fixed-size-virtual-scroll.ts index 4e7e8811a4be..9c7c61174537 100644 --- a/src/cdk/scrolling/fixed-size-virtual-scroll.ts +++ b/src/cdk/scrolling/fixed-size-virtual-scroll.ts @@ -121,11 +121,18 @@ export class FixedSizeVirtualScrollStrategy implements VirtualScrollStrategy { } const scrollOffset = this._viewport.measureScrollOffset(); - const firstVisibleIndex = scrollOffset / this._itemSize; const renderedRange = this._viewport.getRenderedRange(); const newRange = {start: renderedRange.start, end: renderedRange.end}; const viewportSize = this._viewport.getViewportSize(); const dataLength = this._viewport.getDataLength(); + let firstVisibleIndex = scrollOffset / this._itemSize; + + if (newRange.end >= dataLength) { + const maxVisibleItems = Math.ceil(viewportSize / this._itemSize); + firstVisibleIndex = Math.max(0, Math.min(firstVisibleIndex, dataLength - maxVisibleItems)); + newRange.start = Math.floor(firstVisibleIndex); + newRange.end = Math.max(0, Math.min(dataLength - 1, newRange.start + maxVisibleItems)); + } const startBuffer = scrollOffset - newRange.start * this._itemSize; if (startBuffer < this._minBufferPx && newRange.start != 0) { diff --git a/src/cdk/scrolling/virtual-scroll-viewport.spec.ts b/src/cdk/scrolling/virtual-scroll-viewport.spec.ts index 27f6e854c202..37e78ec0cb71 100644 --- a/src/cdk/scrolling/virtual-scroll-viewport.spec.ts +++ b/src/cdk/scrolling/virtual-scroll-viewport.spec.ts @@ -412,12 +412,26 @@ describe('CdkVirtualScrollViewport', () => { fixture.detectChanges(); flush(); - triggerScroll(viewport); + expect(viewport.getOffsetToRenderedContentStart()) + .toBe(testComponent.itemSize, 'should be scrolled to bottom of 5 item list'); + })); + + it('should handle dynamic item array keeping position when possibile', fakeAsync(() => { + testComponent.items = Array(100).fill(0); + finishInit(fixture); + triggerScroll(viewport, testComponent.itemSize * 50); fixture.detectChanges(); flush(); expect(viewport.getOffsetToRenderedContentStart()) - .toBe(testComponent.itemSize, 'should be scrolled to bottom of 5 item list'); + .toBe(testComponent.itemSize * 50, 'should be scrolled to index 50 item list'); + + testComponent.items = Array(54).fill(0); + fixture.detectChanges(); + flush(); + + expect(viewport.getOffsetToRenderedContentStart()) + .toBe(testComponent.itemSize * 50, 'should be kept the scroll position'); })); it('should update viewport as user scrolls right in horizontal mode', fakeAsync(() => { @@ -900,6 +914,15 @@ function triggerScroll(viewport: CdkVirtualScrollViewport, offset?: number) { .cdk-virtual-scroll-orientation-horizontal .cdk-virtual-scroll-content-wrapper { flex-direction: row; } + + .cdk-virtual-scroll-viewport { + background-color: #f5f5f5; + } + + .item { + box-sizing: border-box; + border: 1px dashed #ccc; + } `], encapsulation: ViewEncapsulation.None, }) @@ -952,6 +975,15 @@ class FixedSizeVirtualScroll { .cdk-virtual-scroll-orientation-horizontal .cdk-virtual-scroll-content-wrapper { flex-direction: row; } + + .cdk-virtual-scroll-viewport { + background-color: #f5f5f5; + } + + .item { + box-sizing: border-box; + border: 1px dashed #ccc; + } `], encapsulation: ViewEncapsulation.None, }) @@ -982,9 +1014,19 @@ class FixedSizeVirtualScrollWithRtlDirection { @Component({ template: ` -
{{item}}
+
{{item}}
 - ` + `, + styles: [` + .cdk-virtual-scroll-viewport { + background-color: #f5f5f5; + } + + .item { + box-sizing: border-box; + border: 1px dashed #ccc; + } + `] }) class VirtualScrollWithNoStrategy { items = []; @@ -1013,11 +1055,14 @@ class InjectsViewContainer { .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 From ce0ba06a264389b7824c3baf41c8439e5b95b85f Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Fri, 19 Jun 2020 15:57:46 +0200 Subject: [PATCH 2/2] Added comments and fixed behavior when new dataLength matches current range end --- .../scrolling/fixed-size-virtual-scroll.ts | 21 ++++++++++++++----- .../scrolling/virtual-scroll-viewport.spec.ts | 20 ++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/cdk/scrolling/fixed-size-virtual-scroll.ts b/src/cdk/scrolling/fixed-size-virtual-scroll.ts index 9c7c61174537..c7a66eab765e 100644 --- a/src/cdk/scrolling/fixed-size-virtual-scroll.ts +++ b/src/cdk/scrolling/fixed-size-virtual-scroll.ts @@ -120,18 +120,29 @@ export class FixedSizeVirtualScrollStrategy implements VirtualScrollStrategy { return; } - const scrollOffset = this._viewport.measureScrollOffset(); const renderedRange = this._viewport.getRenderedRange(); const newRange = {start: renderedRange.start, end: renderedRange.end}; const viewportSize = this._viewport.getViewportSize(); const dataLength = this._viewport.getDataLength(); + let scrollOffset = this._viewport.measureScrollOffset(); let firstVisibleIndex = scrollOffset / this._itemSize; - if (newRange.end >= dataLength) { + // If user scrolls to the bottom of the list and data changes to a smaller list + if (newRange.end > dataLength) { + // We have to recalculate the first visible index based on new data length and viewport size. const maxVisibleItems = Math.ceil(viewportSize / this._itemSize); - firstVisibleIndex = Math.max(0, Math.min(firstVisibleIndex, dataLength - maxVisibleItems)); - newRange.start = Math.floor(firstVisibleIndex); - newRange.end = Math.max(0, Math.min(dataLength - 1, newRange.start + maxVisibleItems)); + const newVisibleIndex = Math.max(0, + Math.min(firstVisibleIndex, dataLength - maxVisibleItems)); + + // If first visible index changed we must update scroll offset to handle start/end buffers + // Current range must also be adjusted to cover the new position (bottom of new list). + if (firstVisibleIndex != newVisibleIndex) { + firstVisibleIndex = newVisibleIndex; + scrollOffset = newVisibleIndex * this._itemSize; + newRange.start = Math.floor(firstVisibleIndex); + } + + newRange.end = Math.max(0, Math.min(dataLength, newRange.start + maxVisibleItems)); } const startBuffer = scrollOffset - newRange.start * this._itemSize; diff --git a/src/cdk/scrolling/virtual-scroll-viewport.spec.ts b/src/cdk/scrolling/virtual-scroll-viewport.spec.ts index 37e78ec0cb71..17edffd99cd4 100644 --- a/src/cdk/scrolling/virtual-scroll-viewport.spec.ts +++ b/src/cdk/scrolling/virtual-scroll-viewport.spec.ts @@ -416,6 +416,26 @@ describe('CdkVirtualScrollViewport', () => { .toBe(testComponent.itemSize, 'should be scrolled to bottom of 5 item list'); })); + it('should handle dynamic item array with dynamic buffer', fakeAsync(() => { + finishInit(fixture); + triggerScroll(viewport, testComponent.itemSize * 6); + fixture.detectChanges(); + flush(); + + expect(viewport.getOffsetToRenderedContentStart()) + .toBe(testComponent.itemSize * 6, 'should be scrolled to bottom of 10 item list'); + + testComponent.items = Array(5).fill(0); + testComponent.minBufferPx = testComponent.itemSize; + testComponent.maxBufferPx = testComponent.itemSize; + + fixture.detectChanges(); + flush(); + + expect(viewport.getOffsetToRenderedContentStart()) + .toBe(0, 'should render from first item'); + })); + it('should handle dynamic item array keeping position when possibile', fakeAsync(() => { testComponent.items = Array(100).fill(0); finishInit(fixture);