From 632a8dd96c2c8bd1c03ca63ddc706bfd7daa2b97 Mon Sep 17 00:00:00 2001 From: Ugur Saglam <106508695+ugur-vaadin@users.noreply.github.com> Date: Fri, 18 Oct 2024 09:53:30 +0300 Subject: [PATCH] fix: preserve large items when new items are added (#7987) * fix: preserve large items when new items are added * fix: fix refactor typo * test: extract initialization with large item count --- .../src/virtualizer-iron-list-adapter.js | 47 ++++++++++++++++++ .../virtualizer-variable-row-height.test.js | 48 ++++++++++++++++--- 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/packages/component-base/src/virtualizer-iron-list-adapter.js b/packages/component-base/src/virtualizer-iron-list-adapter.js index 34479015b5..d283823816 100644 --- a/packages/component-base/src/virtualizer-iron-list-adapter.js +++ b/packages/component-base/src/virtualizer-iron-list-adapter.js @@ -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 diff --git a/packages/component-base/test/virtualizer-variable-row-height.test.js b/packages/component-base/test/virtualizer-variable-row-height.test.js index c4baaaa52d..c5074c32b9 100644 --- a/packages/component-base/test/virtualizer-variable-row-height.test.js +++ b/packages/component-base/test/virtualizer-variable-row-height.test.js @@ -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(`
@@ -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(); @@ -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(); @@ -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(); @@ -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(); @@ -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'; @@ -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); @@ -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', () => {