Cleanup the fix, improve tests

Author: tomivirkki

packages/component-base/src/virtualizer-iron-list-adapter.js

@@ -180,9 +180,6 @@ export class IronListAdapter {
     this._resizeHandler();
     flush();
     this._scrollHandler();
-    if (this.__physicalSizesChangedDebouncer) {
-      this.__physicalSizesChangedDebouncer.flush();
-    }
     if (this.__fixInvalidItemPositioningDebouncer) {
       this.__fixInvalidItemPositioningDebouncer.flush();
     }
@@ -231,16 +228,15 @@ export class IronListAdapter {
     const prevAvgCount = this._physicalAverageCount;
     const prevPhysicalAvg = this._physicalAverage;
 
-    let physicalSizesChanged = false;
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
     this._iterateItems((pidx, vidx) => {
       oldPhysicalSize += this._physicalSizes[pidx];
-      const elementOldPhysicalSize = this._physicalSizes[pidx] || 0;
+      const elementOldPhysicalSize = this._physicalSizes[pidx];
       this._physicalSizes[pidx] = Math.ceil(this.__getBorderBoxHeight(this._physicalItems[pidx]));
 
       if (this._physicalSizes[pidx] !== elementOldPhysicalSize) {
         // Physical size changed, but resize observer may not catch it if the original size is restored quickly.
-        physicalSizesChanged = true;
+        this.__schedulePhysicalSizesChangedDebouncer();
       }
 
       newPhysicalSize += this._physicalSizes[pidx];
@@ -255,13 +251,27 @@ export class IronListAdapter {
         (prevPhysicalAvg * prevAvgCount + newPhysicalSize) / this._physicalAverageCount,
       );
     }
+  }
 
-    if (physicalSizesChanged) {
-      // There were changes in physical sizes, schedule a resize handler call
-      this.__physicalSizesChangedDebouncer = Debouncer.debounce(this.__physicalSizesChangedDebouncer, microTask, () => {
-        this._resizeHandler();
-      });
-    }
+  __schedulePhysicalSizesChangedDebouncer() {
+    this.__physicalSizesChangedDebouncer = Debouncer.debounce(
+      this.__physicalSizesChangedDebouncer,
+      animationFrame,
+      () => {
+        if (this.__hasSizeChanges()) {
+          this._updateMetrics();
+          this._positionItems();
+          this._updateScrollerSize();
+        }
+      },
+    );
+  }
+
+  __hasSizeChanges() {
+    return this._physicalItems?.some((item, pidx) => {
+      const currentSize = Math.ceil(this.__getBorderBoxHeight(item));
+      return currentSize !== this._physicalSizes[pidx];
+    });
   }
 
   __getBorderBoxHeight(el) {
@@ -646,11 +656,6 @@ export class IronListAdapter {
   _resizeHandler() {
     super._resizeHandler();
 
-    if (this.__physicalSizesChangedDebouncer && this.__physicalSizesChangedDebouncer.isActive()) {
-      // Cancel any pending debounced calls to avoid unnecessary extra work
-      this.__physicalSizesChangedDebouncer.cancel();
-    }
-
     // Fixes an issue where the new items are not created on scroll target resize when the scroll position is around the end.
     // See https://github.com/vaadin/flow-components/issues/7307
     const lastIndexVisible = this.adjustedLastVisibleIndex === this.size - 1;

packages/component-base/test/virtualizer-item-height.test.js

@@ -1,5 +1,5 @@
 import { expect } from '@vaadin/chai-plugins';
-import { aTimeout, fixtureSync, nextFrame, nextResize } from '@vaadin/testing-helpers';
+import { aTimeout, fixtureSync, nextFrame, nextResize, oneEvent } from '@vaadin/testing-helpers';
 import sinon from 'sinon';
 import { Virtualizer } from '../src/virtualizer.js';
 
@@ -523,8 +523,10 @@ describe('virtualizer - item height - self-resizing items', () => {
   it('should not overlap items after scrolling', async () => {
     await contentUpdate();
     // Scroll manually to the end
-    scrollTarget.scrollTop = scrollTarget.scrollHeight;
-    await contentUpdate();
+    while (Math.ceil(scrollTarget.scrollTop) < scrollTarget.scrollHeight - scrollTarget.clientHeight) {
+      scrollTarget.scrollTop += 100;
+      await oneEvent(scrollTarget, 'scroll');
+    }
 
     // Ensure that the first two visible items do not overlap
     const firstVisibleItem = scrollTarget.querySelector(`#item-${virtualizer.firstVisibleIndex}`);

packages/component-base/test/virtualizer-variable-row-height.test.js

@@ -64,7 +64,6 @@ describe('virtualizer - variable row height - large variance', () => {
     // Scroll to last item
     virtualizer.scrollToIndex(virtualizer.size - 1);
     await nextFrame();
-    virtualizer.scrollToIndex(virtualizer.size - 1);
     await nextFrame();
     // Manually scroll to end
     scrollTarget.scrollTop = scrollTarget.scrollHeight - scrollTarget.offsetHeight;

test/integration/virtual-list-card.test.js

@@ -1,5 +1,5 @@
 import { expect } from '@vaadin/chai-plugins';
-import { aTimeout, fixtureSync } from '@vaadin/testing-helpers';
+import { aTimeout, fixtureSync, nextFrame, oneEvent } from '@vaadin/testing-helpers';
 import '@vaadin/virtual-list';
 import '@vaadin/card';
 
@@ -28,10 +28,27 @@ describe('virtual-list with card items', () => {
     };
 
     virtualList.items = Array.from({ length: 100 }, (_, i) => ({ index: i }));
-    await contentUpdate();
+    await nextFrame();
   });
 
   it('should not overlap items after scrolling', async () => {
+    // Scroll manually to the end
+    while (Math.ceil(virtualList.scrollTop) < virtualList.scrollHeight - virtualList.clientHeight) {
+      virtualList.scrollTop += 100;
+      await oneEvent(virtualList, 'scroll');
+    }
+
+    await contentUpdate();
+
+    // Ensure that the first two visible items do not overlap
+    const firstVisibleItem = virtualList.querySelector(`#card-${virtualList.firstVisibleIndex}`);
+    const secondVisibleItem = virtualList.querySelector(`#card-${virtualList.firstVisibleIndex + 1}`);
+    expect(firstVisibleItem.getBoundingClientRect().bottom).to.be.at.most(
+      secondVisibleItem.getBoundingClientRect().top,
+    );
+  });
+
+  it('should not overlap items after changing scroll position', async () => {
     virtualList.scrollTop = virtualList.scrollHeight;
 
     await contentUpdate();