Skip to content

fix(cdk/scrolling): fixed-size-virtual-scroll wrong range and position when items length changes and current scroll is greater than new data length #19578

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
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
22 changes: 20 additions & 2 deletions src/cdk/scrolling/fixed-size-virtual-scroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,30 @@ export class FixedSizeVirtualScrollStrategy implements VirtualScrollStrategy {
return;
}

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 scrollOffset = this._viewport.measureScrollOffset();
let firstVisibleIndex = scrollOffset / this._itemSize;

// 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);
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;
if (startBuffer < this._minBufferPx && newRange.start != 0) {
Expand Down
73 changes: 69 additions & 4 deletions src/cdk/scrolling/virtual-scroll-viewport.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,12 +412,46 @@ 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 with dynamic buffer', fakeAsync(() => {
finishInit(fixture);
triggerScroll(viewport, testComponent.itemSize * 6);
fixture.detectChanges();
flush();

expect(viewport.getOffsetToRenderedContentStart())
.toBe(testComponent.itemSize, 'should be scrolled to bottom of 5 item list');
.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);
triggerScroll(viewport, testComponent.itemSize * 50);
fixture.detectChanges();
flush();

expect(viewport.getOffsetToRenderedContentStart())
.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(() => {
Expand Down Expand Up @@ -900,6 +934,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,
})
Expand Down Expand Up @@ -952,6 +995,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,
})
Expand Down Expand Up @@ -982,9 +1034,19 @@ class FixedSizeVirtualScrollWithRtlDirection {
@Component({
template: `
<cdk-virtual-scroll-viewport>
<div *cdkVirtualFor="let item of items">{{item}}</div>
<div class="item" *cdkVirtualFor="let item of items">{{item}}</div>
</cdk-virtual-scroll-viewport>
`
`,
styles: [`
.cdk-virtual-scroll-viewport {
background-color: #f5f5f5;
}

.item {
box-sizing: border-box;
border: 1px dashed #ccc;
}
`]
})
class VirtualScrollWithNoStrategy {
items = [];
Expand Down Expand Up @@ -1013,11 +1075,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
Expand Down