handle resetting the list when collapsed

shakyShane · shakyShane · commit a7c91d56183d · 2025-01-31T18:37:32.000Z
diff --git a/special-pages/pages/new-tab/app/activity/ActivityProvider.js b/special-pages/pages/new-tab/app/activity/ActivityProvider.js
@@ -95,51 +95,49 @@ export function ActivityProvider(props) {
  * @return {NormalizedActivity}
  */
 function normalizeItems(prev, data) {
-    return {
-        favorites: Object.fromEntries(
-            data.activity.map((x) => {
-                return [x.url, x.favorite];
-            }),
-        ),
-        items: Object.fromEntries(
-            data.activity.map((x) => {
-                /** @type {Item} */
-                const next = {
-                    etldPlusOne: x.etldPlusOne,
-                    title: x.title,
-                    url: x.url,
-                    faviconMax: x.favicon?.maxAvailableSize ?? DDG_DEFAULT_ICON_SIZE,
-                    favoriteSrc: x.favicon?.src,
-                    trackersFound: x.trackersFound,
-                };
-                const differs = shallowDiffers(next, prev.items[x.url] || {});
-                return [x.url, differs ? next : prev.items[x.url] || {}];
-            }),
-        ),
-        history: Object.fromEntries(
-            data.activity.map((x) => {
-                const differs = shallowDiffers(x.history, prev.history[x.url] || []);
-                return [x.url, differs ? [...x.history] : prev.history[x.url] || []];
-            }),
-        ),
-        trackingStatus: Object.fromEntries(
-            data.activity.map((x) => {
-                const prevItem = prev.trackingStatus[x.url] || {
-                    totalCount: 0,
-                    trackerCompanies: [],
-                };
-                const differs = shallowDiffers(x.trackingStatus.trackerCompanies, prevItem.trackerCompanies);
-                if (prevItem.totalCount !== x.trackingStatus.totalCount || differs) {
-                    const next = {
-                        totalCount: x.trackingStatus.totalCount,
-                        trackerCompanies: [...x.trackingStatus.trackerCompanies],
-                    };
-                    return [x.url, next];
-                }
-                return [x.url, prevItem];
-            }),
-        ),
+    /** @type {NormalizedActivity} */
+    const output = {
+        favorites: {},
+        items: {},
+        history: {},
+        trackingStatus: {},
     };
+    for (const item of data.activity) {
+        const id = item.url;
+
+        output.favorites[id] = item.favorite;
+
+        /** @type {Item} */
+        const next = {
+            etldPlusOne: item.etldPlusOne,
+            title: item.title,
+            url: id,
+            faviconMax: item.favicon?.maxAvailableSize ?? DDG_DEFAULT_ICON_SIZE,
+            favoriteSrc: item.favicon?.src,
+            trackersFound: item.trackersFound,
+        };
+        const differs = shallowDiffers(next, prev.items[id] || {});
+        output.items[id] = differs ? next : prev.items[id] || {};
+
+        const historyDiff = shallowDiffers(item.history, prev.history[id] || []);
+        output.history[id] = historyDiff ? [...item.history] : prev.history[id] || [];
+
+        const prevItem = prev.trackingStatus[id] || {
+            totalCount: 0,
+            trackerCompanies: [],
+        };
+        const trackersDiffer = shallowDiffers(item.trackingStatus.trackerCompanies, prevItem.trackerCompanies);
+        if (prevItem.totalCount !== item.trackingStatus.totalCount || trackersDiffer) {
+            const next = {
+                totalCount: item.trackingStatus.totalCount,
+                trackerCompanies: [...item.trackingStatus.trackerCompanies],
+            };
+            output.trackingStatus[id] = next;
+        } else {
+            output.trackingStatus[id] = prevItem;
+        }
+    }
+    return output;
 }
 
 /**
@@ -195,16 +193,16 @@ export function SignalStateProvider({ children }) {
         if (!target) return;
         if (!service) return;
         const anchor = /** @type {HTMLAnchorElement|null} */ (target.closest('a[href][data-url]'));
+        const button = /** @type {HTMLButtonElement|null} */ (target.closest('button[value][data-action]'));
+        const toggle = /** @type {HTMLButtonElement|null} */ (target.closest('button[data-toggle]'));
         if (anchor) {
             const url = anchor.dataset.url;
             if (!url) return;
             event.preventDefault();
             event.stopImmediatePropagation();
             const openTarget = eventToTarget(event, platformName);
             service.openUrl(url, openTarget);
-        } else {
-            const button = /** @type {HTMLButtonElement|null} */ (target.closest('button[value][data-action]'));
-            if (!button) return;
+        } else if (button) {
             event.preventDefault();
             event.stopImmediatePropagation();
 
@@ -226,10 +224,15 @@ export function SignalStateProvider({ children }) {
             } else {
                 console.warn('unhandled action:', action);
             }
+        } else if (toggle) {
+            if (state.config?.expansion === 'collapsed') {
+                const next = urls.value.slice(0, Math.min(service.INITIAL, urls.value.length));
+                setVisibleRange(next);
+            }
         }
     }
 
-    const didClick = useCallback(didClick_, []);
+    const didClick = useCallback(didClick_, [service, state.config.expansion]);
 
     const keys = useSignal(
         normalizeKeys(
@@ -297,15 +300,19 @@ export function SignalStateProvider({ children }) {
             fillHoles();
         });
 
-        window.addEventListener('activity.next', showNextChunk);
-
         return () => {
             unsub();
             unsubPatch();
-            window.removeEventListener('activity.next', showNextChunk);
         };
     });
 
+    useEffect(() => {
+        window.addEventListener('activity.next', showNextChunk);
+        return () => {
+            window.removeEventListener('activity.next', showNextChunk);
+        };
+    }, []);
+
     useEffect(() => {
         const handler = () => {
             if (document.visibilityState === 'visible') {
diff --git a/special-pages/pages/new-tab/app/activity/batched-activity.service.js b/special-pages/pages/new-tab/app/activity/batched-activity.service.js
@@ -4,6 +4,7 @@
  * @typedef {import("../../types/new-tab.js").UrlInfo} UrlInfo
  * @typedef {import("../../types/new-tab.js").PatchData} PatchData
  * @typedef {import('../../types/new-tab.js').DomainActivity} DomainActivity
+ * @typedef {import('../service.js').InvocationSource} InvocationSource
  */
 import { Service } from '../service.js';
 
@@ -47,7 +48,10 @@ export class BatchedActivityService {
                 }
             },
             subscribe: (cb) => ntp.messaging.subscribe('activity_onDataUpdate', cb),
-        }).withUpdater((old, next) => {
+        }).withUpdater((old, next, source) => {
+            if (source === 'manual') {
+                return next;
+            }
             if (this.batched) {
                 return { activity: old.activity.concat(next.activity) };
             }
@@ -115,7 +119,7 @@ export class BatchedActivityService {
     }
 
     /**
-     * @param {(evt: {data: UrlInfo & PatchData, source: 'manual' | 'subscription'}) => void} cb
+     * @param {(evt: {data: UrlInfo & PatchData, source: InvocationSource}) => void} cb
      * @internal
      */
     onUrlData(cb) {
@@ -136,7 +140,7 @@ export class BatchedActivityService {
     }
 
     /**
-     * @param {(evt: {data: ActivityData, source: 'manual' | 'subscription'}) => void} cb
+     * @param {(evt: {data: ActivityData, source: InvocationSource}) => void} cb
      * @internal
      */
     onData(cb) {
@@ -155,7 +159,7 @@ export class BatchedActivityService {
     }
 
     /**
-     * @param {(evt: {data: ActivityConfig, source: 'manual' | 'subscription'}) => void} cb
+     * @param {(evt: {data: ActivityConfig, source: InvocationSource}) => void} cb
      * @internal
      */
     onConfig(cb) {
@@ -232,12 +236,6 @@ export class BatchedActivityService {
         });
         this.ntp.messaging.notify('activity_removeItem', { url });
     }
-    /**
-     * @param {string} url
-     */
-    removeOnly(url) {
-        this.ntp.messaging.notify('activity_removeItem', { url });
-    }
     /**
      * @param {string} url
      * @param {import('../../types/new-tab.js').OpenTarget} target
diff --git a/special-pages/pages/new-tab/app/activity/components/Activity.js b/special-pages/pages/new-tab/app/activity/components/Activity.js
@@ -40,6 +40,7 @@ function ActivityConfigured({ expansion, toggle }) {
     const batched = useBatchedActivityApi();
     const expanded = expansion === 'expanded';
     const { activity } = useContext(SignalStateContext);
+    const { didClick } = useContext(ActivityApiContext);
     const count = useComputed(() => {
         return Object.values(activity.value.trackingStatus).reduce((acc, item) => {
             return acc + item.totalCount;
@@ -56,7 +57,7 @@ function ActivityConfigured({ expansion, toggle }) {
 
     return (
         <Fragment>
-            <div class={styles.root}>
+            <div class={styles.root} onClick={didClick}>
                 <ActivityHeading
                     trackerCount={count.value}
                     itemCount={itemCount.value}
@@ -70,7 +71,7 @@ function ActivityConfigured({ expansion, toggle }) {
                 />
                 {itemCount.value > 0 && expanded && <ActivityBody canBurn={canBurn} />}
             </div>
-            {batched && <Loader />}
+            {batched && itemCount.value > 0 && expanded && <Loader />}
         </Fragment>
     );
 }
@@ -80,15 +81,14 @@ function ActivityConfigured({ expansion, toggle }) {
  * @param {boolean} props.canBurn
  */
 function ActivityBody({ canBurn }) {
-    const { didClick } = useContext(ActivityApiContext);
     const documentVisibility = useDocumentVisibility();
     const { isReducedMotion } = useEnv();
     const { keys } = useContext(SignalStateContext);
     const { burning, exiting } = useContext(ActivityBurningSignalContext);
     const busy = useComputed(() => burning.value.length > 0 || exiting.value.length > 0);
 
     return (
-        <ul class={styles.activity} onClick={didClick} data-busy={busy}>
+        <ul class={styles.activity} data-busy={busy}>
             {keys.value.map((id, index) => {
                 if (canBurn && !isReducedMotion) return <BurnableItem id={id} key={id} documentVisibility={documentVisibility} />;
                 return <RemovableItem id={id} key={id} canBurn={canBurn} documentVisibility={documentVisibility} />;
diff --git a/special-pages/pages/new-tab/app/activity/integration-tests/activity.page.js b/special-pages/pages/new-tab/app/activity/integration-tests/activity.page.js
@@ -99,6 +99,15 @@ export class ActivityPage {
         await page.getByLabel('Show recent activity').click();
     }
 
+    async collapsesList() {
+        const { page } = this;
+        await page.getByLabel('Hide recent activity').click();
+    }
+    async expandsList() {
+        const { page } = this;
+        await page.getByLabel('Show recent activity').click();
+    }
+
     async addsFavorite() {
         await this.context().getByRole('button', { name: 'Add example.com to favorites' }).click();
         const result = await this.ntp.mocks.waitForCallCount({ method: 'activity_addFavorite', count: 1 });
diff --git a/special-pages/pages/new-tab/app/activity/integration-tests/activity.spec.js b/special-pages/pages/new-tab/app/activity/integration-tests/activity.spec.js
@@ -218,5 +218,25 @@ test.describe('activity widget', () => {
 
             await batching.itemsReorder();
         });
+        test('resets on collapse', async ({ page }, workerInfo) => {
+            const ntp = NewtabPage.create(page, workerInfo);
+            await ntp.reducedMotion();
+
+            // 20 entries, plenty to be triggered
+            const widget = new ActivityPage(page, ntp).withEntries(20);
+            const batching = new BatchingPage(page, ntp, widget);
+
+            await ntp.openPage({
+                additional: { feed: 'activity', 'activity.api': 'batched', platform: 'windows', activity: widget.entries },
+            });
+
+            await batching.fetchedRows(5);
+            await widget.hasRows(5);
+            await batching.triggerNext();
+            await widget.hasRows(15);
+            await widget.collapsesList();
+            await widget.expandsList();
+            await widget.hasRows(5);
+        });
     });
 });
diff --git a/special-pages/pages/new-tab/app/components/ShowHideButton.jsx b/special-pages/pages/new-tab/app/components/ShowHideButton.jsx
@@ -19,6 +19,7 @@ export function ShowHideButton({ text, onClick, buttonAttrs = {}, shape = 'none'
             {...buttonAttrs}
             class={cn(styles.button, shape === 'round' && styles.round, !!showText && styles.withText)}
             aria-label={text}
+            data-toggle="true"
             onClick={onClick}
         >
             {showText ? (
diff --git a/special-pages/pages/new-tab/app/service.hooks.js b/special-pages/pages/new-tab/app/service.hooks.js
@@ -12,6 +12,7 @@
 
 /**
  * @typedef {'entering' | 'exiting' | 'entered'} Ui
+ * @typedef {import('./service.js').InvocationSource} InvocationSource
  */
 
 /**
@@ -186,7 +187,7 @@ export function useDataSubscription({ dispatch, service }) {
  * @param {object} params
  * @param {import("preact/hooks").Dispatch<Events<any, Config>>} params.dispatch
  * @param {import("preact").RefObject<{
- *    onConfig: (cb: (event: { data:Config, source: 'manual' | 'subscription'}) => void) => () => void;
+ *    onConfig: (cb: (event: { data: Config, source: InvocationSource}) => void) => () => void;
  *    toggleExpansion: () => void;
  * }>} params.service
  */
diff --git a/special-pages/pages/new-tab/app/service.js b/special-pages/pages/new-tab/app/service.js
@@ -1,3 +1,6 @@
+/**
+ * @typedef {'initial' | 'subscription' | 'manual' | 'trigger-fetch'} InvocationSource
+ */
 /**
  * @template Data - the data format this service produces/stores
  *
@@ -14,7 +17,7 @@ export class Service {
     eventTarget = new EventTarget();
     DEBOUNCE_TIME_MS = 200;
     _broadcast = true;
-    /** @type {undefined|((old: Data, next: Data) => Data)} */
+    /** @type {undefined|((old: Data, next: Data, trigger: InvocationSource) => Data)} */
     accept;
     /**
      * @param {object} props
@@ -34,6 +37,9 @@ export class Service {
         }
     }
 
+    /**
+     * @param {(old: Data, next: Data, trigger: string) => Data} fn
+     */
     withUpdater(fn) {
         this.accept = fn;
         return this;
@@ -65,7 +71,7 @@ export class Service {
      * @param {Data} d
      */
     publish(d) {
-        this._accept(d, 'manual');
+        this._accept(d, 'subscription');
     }
 
     /**
@@ -76,14 +82,14 @@ export class Service {
      *
      * A function is returned, which can be used to remove the event listener
      *
-     * @param {(evt: {data: Data, source: 'manual' | 'subscription'}) => void} cb
+     * @param {(evt: {data: Data, source: InvocationSource}) => void} cb
      */
     onData(cb) {
         this._setupSubscription();
         const controller = new AbortController();
         this.eventTarget.addEventListener(
             'data',
-            (/** @type {CustomEvent<{data: Data, source: 'manual' | 'subscription'}>} */ evt) => {
+            (/** @type {CustomEvent<{data: Data, source: InvocationSource}>} */ evt) => {
                 cb(evt.detail);
             },
             { signal: controller.signal },
@@ -140,12 +146,12 @@ export class Service {
     }
     /**
      * @param {Data} data
-     * @param {'initial' | 'subscription' | 'manual' | 'trigger-fetch'} source
+     * @param {InvocationSource} source
      * @private
      */
     _accept(data, source) {
         if (this.accept && source !== 'initial') {
-            this.data = /** @type {NonNullable<Data>} */ (this.accept(/** @type {NonNullable<Data>} */ (this.data), data));
+            this.data = /** @type {NonNullable<Data>} */ (this.accept(/** @type {NonNullable<Data>} */ (this.data), data, source));
         } else {
             this.data = /** @type {NonNullable<Data>} */ (data);
         }

Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,7 @@ export function ShowHideButton({ text, onClick, buttonAttrs = {}, shape = 'none'`
`19`	`19`	`{...buttonAttrs}`
`20`	`20`	`class={cn(styles.button, shape === 'round' && styles.round, !!showText && styles.withText)}`
`21`	`21`	`aria-label={text}`
	`22`	`+ data-toggle="true"`
`22`	`23`	`onClick={onClick}`
`23`	`24`	`>`
`24`	`25`	`{showText ? (`