Skip to content

Commit

Permalink
fix: preserve large items when new items are added (#7987)
Browse files Browse the repository at this point in the history
* fix: preserve large items when new items are added

* fix: fix refactor typo

* test: extract initialization with large item count
  • Loading branch information
ugur-vaadin authored Oct 18, 2024
1 parent 29449f9 commit 632a8dd
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 7 deletions.
47 changes: 47 additions & 0 deletions packages/component-base/src/virtualizer-iron-list-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,53 @@ export class IronListAdapter {
}
}

/**
* An optimal physical size such that we will have enough physical items
* to fill up the viewport and recycle when the user scrolls.
*
* This default value assumes that we will at least have the equivalent
* to a viewport of physical items above and below the user's viewport.
* @override
*/
get _optPhysicalSize() {
const optPhysicalSize = super._optPhysicalSize;
// No need to adjust
if (optPhysicalSize <= 0 || this.__hasPlaceholders()) {
return optPhysicalSize;
}
// Item height buffer accounts for the cases where some items are much larger than the average.
// This can lead to some items not being rendered and leaving empty space in the viewport.
// https://github.com/vaadin/flow-components/issues/6651
return optPhysicalSize + this.__getItemHeightBuffer();
}

/**
* Extra item height buffer used when calculating optimal physical size.
*
* The iron list core uses the optimal physical size when determining whether to increase the item pool.
* For the cases where some items are much larger than the average, the iron list core might not increase item pool.
* This can lead to the large item not being rendered.
*
* @returns {Number} - Extra item height buffer
* @private
*/
__getItemHeightBuffer() {
// No need for a buffer with no items
if (this._physicalCount === 0) {
return 0;
}
// The regular buffer zone height for either top or bottom
const bufferZoneHeight = Math.ceil((this._viewportHeight * (this._maxPages - 1)) / 2);
// The maximum height of the currently rendered items
const maxItemHeight = Math.max(...this._physicalSizes);
// Only add buffer if the item is larger that the other items
if (maxItemHeight > Math.min(...this._physicalSizes)) {
// Add a buffer height since the large item can still be in the viewport and out of the original buffer
return Math.max(0, maxItemHeight - bufferZoneHeight);
}
return 0;
}

/**
* @returns {Number|undefined} - The browser's default font-size in pixels
* @private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,19 @@ describe('virtualizer - variable row height - large variance', () => {
}
}

/**
* Sets a large size, and expands the first and the last items.
*/
function initWithLargeSize() {
// This is kept separate because some tests need physical items
// to be initialized during the test.
virtualizer.size = 200000;
// Expand the first and the last item
expandedItems = [0, virtualizer.size - 1];
virtualizer.update();
virtualizer.scrollToIndex(0);
}

beforeEach(() => {
scrollTarget = fixtureSync(`
<div style="height: 300px; width: 200px;">
Expand Down Expand Up @@ -128,16 +141,10 @@ describe('virtualizer - variable row height - large variance', () => {
});

sinon.spy(virtualizer.__adapter, '__fixInvalidItemPositioning');

virtualizer.size = 200000;

// Expand the first and the last item
expandedItems = [0, virtualizer.size - 1];
virtualizer.update();
virtualizer.scrollToIndex(0);
});

it('should reveal new items when scrolling downwards', async () => {
initWithLargeSize();
const rect = scrollTarget.getBoundingClientRect();

await scrollDownwardsFromStart();
Expand All @@ -155,6 +162,7 @@ describe('virtualizer - variable row height - large variance', () => {
});

it('should reveal new items when scrolling upwards', async () => {
initWithLargeSize();
const rect = scrollTarget.getBoundingClientRect();

await scrollToEnd();
Expand All @@ -173,6 +181,7 @@ describe('virtualizer - variable row height - large variance', () => {
});

it('should not update the item at last index', async () => {
initWithLargeSize();
updateElement.resetHistory();
await scrollDownwardsFromStart();
await fixItemPositioningTimeout();
Expand All @@ -185,6 +194,7 @@ describe('virtualizer - variable row height - large variance', () => {
});

it('should not update the item at first index', async () => {
initWithLargeSize();
await scrollToEnd();
updateElement.resetHistory();
await scrollUpwardsFromEnd();
Expand All @@ -197,6 +207,7 @@ describe('virtualizer - variable row height - large variance', () => {
});

it('should allow scrolling to end of a padded scroll target', async () => {
initWithLargeSize();
scrollTarget.style.padding = '60px 0 ';
scrollTarget.style.boxSizing = 'border-box';

Expand All @@ -208,6 +219,7 @@ describe('virtualizer - variable row height - large variance', () => {
});

it('should allow scrolling to start', async () => {
initWithLargeSize();
// Expand every 10th item
expandedItems = Array.from(Array(virtualizer.size).keys()).filter((index) => index % 10 === 0);

Expand All @@ -221,17 +233,39 @@ describe('virtualizer - variable row height - large variance', () => {
});

it('should not jam when when size is changed after fix', async () => {
initWithLargeSize();
await scrollDownwardsFromStart();
await fixItemPositioningTimeout();
virtualizer.size = 1;
});

it('should not invoke when size is changed after scrolling', async () => {
initWithLargeSize();
await scrollDownwardsFromStart();
virtualizer.size = 0;
await fixItemPositioningTimeout();
expect(virtualizer.__adapter.__fixInvalidItemPositioning.callCount).to.equal(0);
});

it('should preserve large item in viewport when new item is added', async () => {
// Initially only have a large item
virtualizer.size = 1;
expandedItems = [0];
virtualizer.update();
await nextFrame();

// Calculated using the virtualizer height of 300px and regular item height of 30px
const requiredRegularItemCountToFillViewport = 10;

// Add items until the large item disappears
for (let i = 0; i < requiredRegularItemCountToFillViewport - 1; i++) {
// Add a new item and scroll to the end
virtualizer.size += 1;
virtualizer.scrollToIndex(Infinity);
await nextFrame();
expect(virtualizer.firstVisibleIndex).to.equal(0);
}
});
});

describe('virtualizer - variable row height - size changes', () => {
Expand Down

0 comments on commit 632a8dd

Please sign in to comment.