ntp: display tweaks + stats show more fix (#1288)

* ntp: display tweaks * added a failing test case * fixed the implementation * More robust tests --------- Co-authored-by: Shane Osbourne <[email protected]>
duckduckgo · Nov 28, 2024 · 7923678 · 7923678
1 parent f5ad800
commit 7923678
Show file tree

Hide file tree

Showing 9 changed files with 211 additions and 49 deletions.
diff --git a/messaging/lib/messaging.types.d.ts b/messaging/lib/messaging.types.d.ts
@@ -20,6 +20,7 @@ interface Window {
     __playwright_01: {
         mockResponses: Record<string, import('../index.js').MessageResponse>;
         subscriptionEvents: import('../index.js').SubscriptionEvent[];
+        publishSubscriptionEvent?: (evt: import('../index.js').SubscriptionEvent) => void;
         mocks: {
             outgoing: UnstableMockCall[];
         };

diff --git a/messaging/lib/test-utils.mjs b/messaging/lib/test-utils.mjs
@@ -427,6 +427,14 @@ export function simulateSubscriptionMessage(params) {
             window[params.name](subscriptionEvent);
             break;
         }
+        case 'integration': {
+            if (!('publishSubscriptionEvent' in window.__playwright_01))
+                throw new Error(
+                    `subscription event '${subscriptionEvent.subscriptionName}' was not published because 'window.__playwright_01.publishSubscriptionEvent' was missing`,
+                );
+            window.__playwright_01.publishSubscriptionEvent?.(subscriptionEvent);
+            break;
+        }
         default:
             throw new Error('platform not supported yet: ' + params.injectName);
     }

diff --git a/special-pages/pages/new-tab/app/entry-points/privacyStats.js b/special-pages/pages/new-tab/app/entry-points/privacyStats.js
@@ -4,7 +4,7 @@ import { Centered } from '../components/Layout.js';
 
 export function factory() {
     return (
-        <Centered>
+        <Centered data-entry-point="privacyStats">
             <PrivacyStatsCustomized />
         </Centered>
     );

diff --git a/special-pages/pages/new-tab/app/mock-transport.js b/special-pages/pages/new-tab/app/mock-transport.js
@@ -16,13 +16,23 @@ import { variants as nextSteps } from './next-steps/nextsteps.data.js';
  * @typedef {import('../../../types/new-tab').NextStepsData} NextStepsData
  * @typedef {import('../../../types/new-tab').UpdateNotificationData} UpdateNotificationData
  * @typedef {import('../../../types/new-tab').NewTabMessages['subscriptions']['subscriptionEvent']} SubscriptionNames
+ * @typedef {import('@duckduckgo/messaging/lib/test-utils.mjs').SubscriptionEvent} SubscriptionEvent
  */
 
 const VERSION_PREFIX = '__ntp_29__.';
 const url = new URL(window.location.href);
 
 export function mockTransport() {
     const channel = new BroadcastChannel('ntp');
+    /** @type {Map<string, (d: any)=>void>} */
+    const subscriptions = new Map();
+    if ('__playwright_01' in window) {
+        window.__playwright_01.publishSubscriptionEvent = (/** @type {SubscriptionEvent} */ evt) => {
+            const matchingCallback = subscriptions.get(evt.subscriptionName);
+            if (!matchingCallback) return console.error('no matching callback for subscription', evt);
+            matchingCallback(evt.params);
+        };
+    }
 
     function broadcast(named) {
         setTimeout(() => {
@@ -153,9 +163,17 @@ export function mockTransport() {
             }
         },
         subscribe(_msg, cb) {
-            window.__playwright_01?.mocks?.outgoing?.push?.({ payload: structuredClone(_msg) });
             /** @type {import('../../../types/new-tab.ts').NewTabMessages['subscriptions']['subscriptionEvent']} */
             const sub = /** @type {any} */ (_msg.subscriptionName);
+
+            if ('__playwright_01' in window) {
+                window.__playwright_01?.mocks?.outgoing?.push?.({ payload: structuredClone(_msg) });
+                subscriptions.set(sub, cb);
+                return () => {
+                    subscriptions.delete(sub);
+                };
+            }
+
             switch (sub) {
                 case 'widgets_onConfigUpdated': {
                     const controller = new AbortController();
@@ -256,30 +274,46 @@ export function mockTransport() {
                 }
                 case 'stats_onDataUpdate': {
                     const statsVariant = url.searchParams.get('stats');
-                    if (statsVariant !== 'willUpdate') return () => {};
-
                     const count = url.searchParams.get('stats-update-count');
-                    const max = Math.min(parseInt(count || '0'), 10);
-                    if (max === 0) return () => {};
-
-                    let inc = 1;
-                    const int = setInterval(() => {
-                        if (inc === max) return clearInterval(int);
-                        const next = {
-                            ...stats.willUpdate,
-                            trackerCompanies: stats.willUpdate.trackerCompanies.map((x, index) => {
-                                return {
-                                    ...x,
-                                    count: x.count + inc * index,
-                                };
-                            }),
+                    const updateMaxCount = parseInt(count || '0');
+                    if (updateMaxCount === 0) return () => {};
+                    if (statsVariant === 'willUpdate') {
+                        let inc = 1;
+                        const max = Math.min(updateMaxCount, 10);
+                        const int = setInterval(() => {
+                            if (inc === max) return clearInterval(int);
+                            const next = {
+                                ...stats.willUpdate,
+                                trackerCompanies: stats.willUpdate.trackerCompanies.map((x, index) => {
+                                    return {
+                                        ...x,
+                                        count: x.count + inc * index,
+                                    };
+                                }),
+                            };
+                            cb(next);
+                            inc++;
+                        }, 500);
+                        return () => {
+                            clearInterval(int);
                         };
-                        cb(next);
-                        inc++;
-                    }, 500);
-                    return () => {
-                        clearInterval(int);
-                    };
+                    } else if (statsVariant === 'growing') {
+                        const list = stats.many.trackerCompanies;
+                        let index = 0;
+                        const max = Math.min(updateMaxCount, list.length);
+                        const int = setInterval(() => {
+                            if (index === max) return clearInterval(int);
+                            console.log({ index, max });
+                            cb({
+                                trackerCompanies: list.slice(0, index + 1),
+                            });
+                            index++;
+                        }, 200);
+                        return () => {};
+                    } else {
+                        console.log(statsVariant);
+                        return () => {};
+                    }
                 }
                 case 'favorites_onConfigUpdate': {
                     const controller = new AbortController();

diff --git a/special-pages/pages/new-tab/app/privacy-stats/components/PrivacyStats.js b/special-pages/pages/new-tab/app/privacy-stats/components/PrivacyStats.js
@@ -9,7 +9,7 @@ import { viewTransition } from '../../utils.js';
 import { ShowHideButton } from '../../components/ShowHideButton.jsx';
 import { useCustomizer } from '../../customizer/components/Customizer.js';
 import { DDG_STATS_OTHER_COMPANY_IDENTIFIER } from '../constants.js';
-import { sortStatsForDisplay } from '../privacy-stats.utils.js';
+import { displayNameForCompany, sortStatsForDisplay } from '../privacy-stats.utils.js';
 
 /**
  * @import enStrings from "../strings.json"
@@ -138,35 +138,30 @@ export function PrivacyStatsBody({ trackerCompanies, listAttrs = {} }) {
     const defaultRowMax = 5;
     const sorted = sortStatsForDisplay(trackerCompanies);
     const max = sorted[0]?.count ?? 0;
-    const [visible, setVisible] = useState(defaultRowMax);
-    const hasmore = sorted.length > visible;
+    const [expansion, setExpansion] = useState(/** @type {Expansion} */ ('collapsed'));
 
     const toggleListExpansion = () => {
-        if (hasmore) {
+        if (expansion === 'collapsed') {
             messaging.statsShowMore();
         } else {
             messaging.statsShowLess();
         }
-        if (visible === defaultRowMax) {
-            setVisible(sorted.length);
-        }
-        if (visible === sorted.length) {
-            setVisible(defaultRowMax);
-        }
+        setExpansion(expansion === 'collapsed' ? 'expanded' : 'collapsed');
     };
 
+    const rows = expansion === 'expanded' ? sorted : sorted.slice(0, defaultRowMax);
+
     return (
         <Fragment>
             <ul {...listAttrs} class={styles.list} data-testid="CompanyList">
-                {sorted.slice(0, visible).map((company) => {
+                {rows.map((company) => {
                     const percentage = Math.min((company.count * 100) / max, 100);
                     const valueOrMin = Math.max(percentage, 10);
                     const inlineStyles = {
                         width: `${valueOrMin}%`,
                     };
                     const countText = formatter.format(company.count);
-                    // prettier-ignore
-                    const displayName = company.displayName
+                    const displayName = displayNameForCompany(company.displayName);
                     if (company.displayName === DDG_STATS_OTHER_COMPANY_IDENTIFIER) {
                         const otherText = t('stats_otherCount', { count: String(company.count) });
                         return (
@@ -178,7 +173,7 @@ export function PrivacyStatsBody({ trackerCompanies, listAttrs = {} }) {
                     return (
                         <li key={company.displayName} class={styles.row}>
                             <div class={styles.company}>
-                                <CompanyIcon displayName={company.displayName} />
+                                <CompanyIcon displayName={displayName} />
                                 <span class={styles.name}>{displayName}</span>
                             </div>
                             <span class={styles.count}>{countText}</span>
@@ -192,11 +187,11 @@ export function PrivacyStatsBody({ trackerCompanies, listAttrs = {} }) {
                 <div class={styles.listExpander}>
                     <ShowHideButton
                         onClick={toggleListExpansion}
-                        text={hasmore ? t('ntp_show_more') : t('ntp_show_less')}
+                        text={expansion === 'collapsed' ? t('ntp_show_more') : t('ntp_show_less')}
                         showText={true}
                         buttonAttrs={{
-                            'aria-expanded': !hasmore,
-                            'aria-pressed': visible === sorted.length,
+                            'aria-expanded': expansion === 'expanded',
+                            'aria-pressed': expansion === 'expanded',
                         }}
                     />
                 </div>

diff --git a/special-pages/pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.page.js b/special-pages/pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.page.js
@@ -0,0 +1,54 @@
+import { stats } from '../mocks/stats.js';
+import { expect } from '@playwright/test';
+
+export class PrivacyStatsPage {
+    /**
+     * @param {import("@playwright/test").Page} page
+     * @param {import("../../../integration-tests/new-tab.page.js").NewtabPage} ntp
+     */
+    constructor(page, ntp) {
+        this.page = page;
+        this.ntp = ntp;
+    }
+
+    /**
+     * @param {object} params
+     * @param {number} params.count
+     */
+    async receive({ count }) {
+        /** @type {import("../../../../../types/new-tab.js").PrivacyStatsData} */
+        const next = { totalCount: 0, trackerCompanies: stats.many.trackerCompanies.slice(0, count) };
+        await this.ntp.mocks.simulateSubscriptionMessage('stats_onDataUpdate', next);
+    }
+
+    /**
+     * @param {import("../../../../../types/new-tab.js").PrivacyStatsData} data
+     */
+    async receiveData(data) {
+        await this.ntp.mocks.simulateSubscriptionMessage('stats_onDataUpdate', data);
+    }
+
+    context() {
+        return this.page.locator('[data-entry-point="privacyStats"]');
+    }
+
+    rows() {
+        return this.context().getByTestId('CompanyList').locator('li');
+    }
+
+    /**
+     * @param {number} count
+     */
+    async hasRows(count) {
+        const rows = this.rows();
+        expect(await rows.count()).toBe(count);
+    }
+
+    async showMoreSecondary() {
+        await this.context().getByLabel('Show More', { exact: true }).click();
+    }
+
+    async showLessSecondary() {
+        await this.context().getByLabel('Show Less', { exact: true }).click();
+    }
+}
diff --git a/special-pages/pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.spec.js b/special-pages/pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.spec.js
@@ -1,5 +1,6 @@
-import { test, expect } from '@playwright/test';
+import { expect, test } from '@playwright/test';
 import { NewtabPage } from '../../../integration-tests/new-tab.page.js';
+import { PrivacyStatsPage } from './privacy-stats.page.js';
 
 test.describe('newtab privacy stats', () => {
     test('fetches config + stats', async ({ page }, workerInfo) => {
@@ -29,12 +30,19 @@ test.describe('newtab privacy stats', () => {
     });
     test('sending a pixel when show more is clicked', async ({ page }, workerInfo) => {
         const ntp = NewtabPage.create(page, workerInfo);
+        const psp = new PrivacyStatsPage(page, ntp);
         await ntp.reducedMotion();
         await ntp.openPage({ additional: { stats: 'many' } });
-        await page.getByLabel('Show More', { exact: true }).click();
-        await page.getByLabel('Show Less').click();
+
+        // show + hide
+        await psp.showMoreSecondary();
+        await psp.showLessSecondary();
+
+        // assert the event were sent
         await ntp.mocks.waitForCallCount({ method: 'stats_showMore', count: 1 });
         await ntp.mocks.waitForCallCount({ method: 'stats_showLess', count: 1 });
+
+        // to re-instate later
         // expect(calls1.length).toBe(2);
         // expect(calls1).toStrictEqual([
         //     {
@@ -82,13 +90,59 @@ test.describe('newtab privacy stats', () => {
         },
         async ({ page }, workerInfo) => {
             const ntp = NewtabPage.create(page, workerInfo);
+            const psp = new PrivacyStatsPage(page, ntp);
             await ntp.reducedMotion();
-            await ntp.openPage({ additional: { stats: 'willUpdate', 'stats-update-count': '2' } });
+            await ntp.openPage({ additional: { stats: 'none' } });
+
+            await psp.receiveData({
+                totalCount: 2,
+                trackerCompanies: [
+                    { displayName: 'Google', count: 1 },
+                    { displayName: 'Facebook', count: 1 },
+                ],
+            });
+
+            await page.getByText('Google1').locator('[style="width: 100%;"]').waitFor();
+            await page.getByText('Facebook1').locator('[style="width: 100%;"]').waitFor();
+
+            await psp.receiveData({
+                totalCount: 2,
+                trackerCompanies: [
+                    { displayName: 'Google', count: 5 },
+                    { displayName: 'Facebook', count: 1 },
+                ],
+            });
 
-            //
             // Checking the first + last bar widths due to a regression
-            await page.getByText('Google Ads5').locator('[style="width: 100%;"]').waitFor();
+            await page.getByText('Google5').locator('[style="width: 100%;"]').waitFor();
             await page.getByText('Facebook1').locator('[style="width: 20%;"]').waitFor();
         },
     );
+    test(
+        'secondary expansion',
+        {
+            annotation: {
+                type: 'issue',
+                description: 'https://app.asana.com/0/1201141132935289/1208861172991227/f',
+            },
+        },
+        async ({ page }, workerInfo) => {
+            const ntp = NewtabPage.create(page, workerInfo);
+            const psp = new PrivacyStatsPage(page, ntp);
+            await ntp.reducedMotion();
+            await ntp.openPage({ additional: { stats: 'none' } });
+
+            // deliver enough companies to show the 'show more' toggle
+            await psp.receive({ count: 6 });
+            await psp.hasRows(5); // 1 is hidden
+            await psp.showMoreSecondary();
+
+            await psp.receive({ count: 7 });
+            await psp.hasRows(7);
+            await psp.showLessSecondary();
+
+            await psp.receive({ count: 2 });
+            await psp.hasRows(2);
+        },
+    );
 });
diff --git a/special-pages/pages/new-tab/app/privacy-stats/mocks/stats.js b/special-pages/pages/new-tab/app/privacy-stats/mocks/stats.js
@@ -21,7 +21,7 @@ export const stats = {
                 count: 210,
             },
             {
-                displayName: 'Amazon',
+                displayName: 'Amazon.com',
                 count: 67,
             },
             {
@@ -63,7 +63,7 @@ export const stats = {
                 count: 1,
             },
             {
-                displayName: 'Amazon',
+                displayName: 'Amazon.com',
                 count: 1,
             },
             {
@@ -72,6 +72,10 @@ export const stats = {
             },
         ],
     },
+    growing: {
+        totalCount: 0,
+        trackerCompanies: [],
+    },
     many: {
         totalCount: 890,
         trackerCompanies: [