fix(infinite-scroll): awaiting for DOM writes before firing infinite scroll event

Author: ShaneK

core/src/components/infinite-scroll/infinite-scroll.tsx

@@ -143,15 +143,18 @@ export class InfiniteScroll implements ComponentInterface {
 
     if (distanceFromInfinite < 0) {
       if (!this.didFire) {
+        this.isLoading = true;
+        this.didFire = true;
+
         if (this.preserveRerenderScrollPosition) {
           // Lock the min height of the siblings of the infinite scroll
           // if we are preserving the rerender scroll position
-          this.lockSiblingMinHeight(true);
+          this.lockSiblingMinHeight(true).then(() => {
+            this.ionInfinite.emit();
+          });
+        } else {
+          this.ionInfinite.emit();
         }
-
-        this.isLoading = true;
-        this.didFire = true;
-        this.ionInfinite.emit();
         return 3;
       }
     }
@@ -168,32 +171,44 @@ export class InfiniteScroll implements ComponentInterface {
    * We preserve existing min-height values, if they're set, so we don't erase what
    * has been previously set by the user when we restore after complete is called.
    */
-  private lockSiblingMinHeight(lock: boolean) {
-    const siblings = this.el.parentElement?.children || [];
-    for (const sibling of siblings) {
-      // Loop through all the siblings of the infinite scroll, but ignore ourself
-      if (sibling !== this.el && sibling instanceof HTMLElement) {
-        if (lock) {
-          const elementHeight = sibling.getBoundingClientRect().height;
-          if (this.minHeightLocked) {
-            // The previous min height is from us locking it before, so we can disregard it
-            // We still need to lock the min height if we're already locked, though, because
-            // the user could have triggered a new load before we've finished the previous one.
-            const previousMinHeight = sibling.style.minHeight;
-            if (previousMinHeight) {
-              sibling.style.setProperty('--ion-previous-min-height', previousMinHeight);
-            }
+  private lockSiblingMinHeight(lock: boolean): Promise<void> {
+    return new Promise((resolve) => {
+      const siblings = this.el.parentElement?.children || [];
+      const writes: (() => void)[] = [];
+
+      for (const sibling of siblings) {
+        // Loop through all the siblings of the infinite scroll, but ignore ourself
+        if (sibling !== this.el && sibling instanceof HTMLElement) {
+          if (lock) {
+            const elementHeight = sibling.getBoundingClientRect().height;
+            writes.push(() => {
+              if (this.minHeightLocked) {
+                // The previous min height is from us locking it before, so we can disregard it
+                // We still need to lock the min height if we're already locked, though, because
+                // the user could have triggered a new load before we've finished the previous one.
+                const previousMinHeight = sibling.style.minHeight;
+                if (previousMinHeight) {
+                  sibling.style.setProperty('--ion-previous-min-height', previousMinHeight);
+                }
+              }
+              sibling.style.minHeight = `${elementHeight}px`;
+            });
+          } else {
+            writes.push(() => {
+              const previousMinHeight = sibling.style.getPropertyValue('--ion-previous-min-height');
+              sibling.style.minHeight = previousMinHeight || 'auto';
+              sibling.style.removeProperty('--ion-previous-min-height');
+            });
           }
-          sibling.style.minHeight = `${elementHeight}px`;
-        } else {
-          const previousMinHeight = sibling.style.getPropertyValue('--ion-previous-min-height');
-          sibling.style.minHeight = previousMinHeight || 'auto';
-          sibling.style.removeProperty('--ion-previous-min-height');
         }
       }
-    }
 
-    this.minHeightLocked = lock;
+      writeTask(() => {
+        writes.forEach((w) => w());
+        this.minHeightLocked = lock;
+        resolve();
+      });
+    });
   }
 
   /**
@@ -264,8 +279,8 @@ export class InfiniteScroll implements ComponentInterface {
     // Unlock the min height of the siblings of the infinite scroll
     // if we are preserving the rerender scroll position
     if (this.preserveRerenderScrollPosition) {
-      setTimeout(() => {
-        this.lockSiblingMinHeight(false);
+      setTimeout(async () => {
+        await this.lockSiblingMinHeight(false);
       }, 100);
     }
   }
@@ -298,3 +313,4 @@ export class InfiniteScroll implements ComponentInterface {
     );
   }
 }
+

core/src/components/infinite-scroll/test/preserve-rerender-scroll-position/index.html

@@ -67,7 +67,7 @@
         }
         loading = true;
 
-        await wait(300);
+        await wait(0);
         replaceAllItems();
         infiniteScroll.complete();
 
@@ -99,7 +99,7 @@
 
         // Add new items with new "keys" (different content/identifiers)
         // Start with more items to ensure scrollable content
-        const totalItems = generation === 1 ? 30 : 30 + generation * 20;
+        const totalItems = generation === 1 ? 50 : 30 + generation * 20;
         itemCount = 0;
 
         for (let i = 0; i < totalItems; i++) {

core/src/components/infinite-scroll/test/preserve-rerender-scroll-position/infinite-scroll.e2e.ts

@@ -12,10 +12,10 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
       const content = page.locator('ion-content');
       const items = page.locator('ion-item');
       const innerScroll = page.locator('.inner-scroll');
-      expect(await items.count()).toBe(30);
+      expect(await items.count()).toBe(50);
 
       let previousScrollTop = 0;
-      for (let i = 0; i < 20; i++) {
+      for (let i = 0; i < 30; i++) {
         await content.evaluate((el: HTMLIonContentElement) => el.scrollToBottom(0));
         const currentScrollTop = await innerScroll.evaluate((el: HTMLIonContentElement) => el.scrollTop);
         expect(currentScrollTop).toBeGreaterThan(previousScrollTop);
@@ -26,13 +26,6 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
           previousScrollTop
         );
         previousScrollTop = currentScrollTop;
-
-        // Timeout to allow the browser to catch up.
-        // For some reason, without this, the scroll top gets reset to 0. Adding this
-        // prevents that, which implies it's an issue with Playwright, not the feature.
-        // For some reason, this delay needs to be longer than the time required to
-        // reset the minimum height in infinite scroll.
-        await new Promise((resolve) => setTimeout(resolve, 1001));
       }
     });
   });