Merge 183f863 into b51367f

Author: shakyShane

special-pages/pages/new-tab/app/favorites/components/Favorites.js

@@ -257,8 +257,9 @@ function useVisibleRows(rows, rowHeight, safeAreaRef, expansion) {
     /**
      * decide which the start/end indexes should be, based on scroll position.
      * NOTE: this is called on scroll, so must not incur expensive checks/measurements - math only!
+     * @param {number} rowCount - the number of rows in the dataset
      */
-    function setVisibleRowsForOffset() {
+    function setVisibleRowsForOffset(rowCount) {
         if (!safeAreaRef.current) return console.warn('cannot access ref');
         const scrollY = mainScrollerRef.current?.scrollTop ?? 0;
         const offset = gridOffsetRef.current;
@@ -270,7 +271,7 @@ function useVisibleRows(rows, rowHeight, safeAreaRef, expansion) {
             start = scrollY - offset;
         }
         const startIndex = Math.floor(start / rowHeight);
-        const endIndex = Math.min(Math.ceil(end / rowHeight), rows.length);
+        const endIndex = Math.min(Math.ceil(end / rowHeight), rowCount);
 
         // don't set state if the offset didn't change
         setVisibleRange((prev) => {
@@ -293,12 +294,18 @@ function useVisibleRows(rows, rowHeight, safeAreaRef, expansion) {
         updateGlobals();
 
         // and set visible rows once the size is known
-        setVisibleRowsForOffset();
+        setVisibleRowsForOffset(rows.length);
 
         const controller = new AbortController();
 
         // when the main area is scrolled, update the visible offset for the rows.
-        mainScrollerRef.current?.addEventListener('scroll', setVisibleRowsForOffset, { signal: controller.signal });
+        mainScrollerRef.current?.addEventListener(
+            'scroll',
+            () => {
+                setVisibleRowsForOffset(rows.length);
+            },
+            { signal: controller.signal },
+        );
 
         return () => {
             controller.abort();
@@ -311,13 +318,13 @@ function useVisibleRows(rows, rowHeight, safeAreaRef, expansion) {
             if (lastWindowHeight === window.innerHeight) return;
             lastWindowHeight = window.innerHeight;
             updateGlobals();
-            setVisibleRowsForOffset();
+            setVisibleRowsForOffset(rows.length);
         }
         window.addEventListener('resize', handler);
         return () => {
             return window.removeEventListener('resize', handler);
         };
-    }, []);
+    }, [rows.length]);
 
     useEffect(() => {
         if (!contentTubeRef.current) return console.warn('cannot find content tube');
@@ -331,7 +338,7 @@ function useVisibleRows(rows, rowHeight, safeAreaRef, expansion) {
                 clearTimeout(debounceTimer);
                 debounceTimer = setTimeout(() => {
                     updateGlobals();
-                    setVisibleRowsForOffset();
+                    setVisibleRowsForOffset(rows.length);
                 }, 50);
             }
         });
@@ -341,7 +348,7 @@ function useVisibleRows(rows, rowHeight, safeAreaRef, expansion) {
             resizer.disconnect();
             clearTimeout(debounceTimer);
         };
-    }, []);
+    }, [rows.length]);
 
     return { start, end };
 }

special-pages/pages/new-tab/app/favorites/integration-tests/favorites.page.js

@@ -339,4 +339,20 @@ export class FavoritesPage {
         // verify the DOM is updated
         await expect(page.getByTestId('FavoritesConfigured').locator('a[href]')).toHaveCount(to);
     }
+
+    /**
+     * Simulate getting entirely fresh data
+     */
+    async receivesNewDataAndShowsAll(count) {
+        const { page } = this.ntp;
+
+        const items = gen(count);
+        await this.ntp.mocks.simulateSubscriptionMessage('favorites_onDataUpdate', items);
+
+        // The bug occurred after a debounced timer, so this timer reproduces it
+        await page.waitForTimeout(100);
+
+        // Every tile should be shown
+        await this.waitForNumFavorites(count);
+    }
 }

special-pages/pages/new-tab/app/favorites/integration-tests/favorites.spec.js

@@ -153,4 +153,24 @@ test.describe('newtab favorites', () => {
         // assert there's still 16 showing
         await favorites.waitForNumFavorites(16);
     });
+    test('accepts new data when already expanded', async ({ page }, workerInfo) => {
+        const ntp = NewtabPage.create(page, workerInfo);
+        await ntp.reducedMotion();
+
+        const favorites = new FavoritesPage(ntp);
+
+        // open the page with enough next-step + favorites for both to be expanded
+        await ntp.openPage({
+            favorites: '12',
+            additional: {
+                'favorites.config.expansion': 'expanded',
+            },
+        });
+
+        // control: ensure it's just the expected start amount
+        await favorites.waitForNumFavorites(12);
+
+        // then, assert all further elements are shown
+        await favorites.receivesNewDataAndShowsAll(26);
+    });
 });

special-pages/pages/new-tab/app/mock-transport.js

@@ -466,10 +466,10 @@ export function mockTransport() {
                 }
                 case 'favorites_getConfig': {
                     /** @type {FavoritesConfig} */
-                    const defaultConfig = { expansion: 'collapsed', animation: { kind: 'none' } };
+                    const defaultConfig = { expansion: 'collapsed', animation: { kind: 'view-transitions' } };
                     const fromStorage = read('favorites_config') || defaultConfig;
-                    if (url.searchParams.get('favorites.animation') === 'view-transitions') {
-                        fromStorage.animation = { kind: 'view-transitions' };
+                    if (url.searchParams.get('favorites.config.expansion') === 'expanded') {
+                        defaultConfig.expansion = 'expanded';
                     }
                     return Promise.resolve(fromStorage);
                 }