Skip to content

Commit

Permalink
ntp: expanded favorites widget can accept large updates in place
Browse files Browse the repository at this point in the history
  • Loading branch information
shakyShane committed Feb 23, 2025
1 parent b51367f commit 183f863
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 11 deletions.
23 changes: 15 additions & 8 deletions special-pages/pages/new-tab/app/favorites/components/Favorites.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) => {
Expand All @@ -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();
Expand All @@ -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');
Expand All @@ -331,7 +338,7 @@ function useVisibleRows(rows, rowHeight, safeAreaRef, expansion) {
clearTimeout(debounceTimer);
debounceTimer = setTimeout(() => {
updateGlobals();
setVisibleRowsForOffset();
setVisibleRowsForOffset(rows.length);
}, 50);
}
});
Expand All @@ -341,7 +348,7 @@ function useVisibleRows(rows, rowHeight, safeAreaRef, expansion) {
resizer.disconnect();
clearTimeout(debounceTimer);
};
}, []);
}, [rows.length]);

return { start, end };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
6 changes: 3 additions & 3 deletions special-pages/pages/new-tab/app/mock-transport.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 183f863

Please sign in to comment.