ntp: stats - don't show expansion controls without recent items (#1243)

* ntp: stats - don't show expansion controls without recent items * Ensure the bar visualization updates correctly --------- Co-authored-by: Shane Osbourne <[email protected]>
duckduckgo · Nov 20, 2024 · 12b3ab3 · 12b3ab3
1 parent 73d72ac
commit 12b3ab3
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 16 deletions.
diff --git a/special-pages/pages/new-tab/app/privacy-stats/PrivacyStats.js b/special-pages/pages/new-tab/app/privacy-stats/PrivacyStats.js
@@ -109,18 +109,19 @@ export function Heading({ expansion, trackerCompanies, totalCount, onToggle, but
             </span>
             {none && <p className={styles.title}>{t('trackerStatsNoRecent')}</p>}
             {some && <p className={styles.title}>{alltimeTitle}</p>}
-            <span className={styles.expander}>
-                <ShowHideButton
-                    buttonAttrs={{
-                        ...buttonAttrs,
-                        hidden: trackerCompanies.length === 0,
-                        'aria-expanded': expansion === 'expanded',
-                        'aria-pressed': expansion === 'expanded',
-                    }}
-                    onClick={onToggle}
-                    text={expansion === 'expanded' ? t('trackerStatsHideLabel') : t('trackerStatsToggleLabel')}
-                />
-            </span>
+            {recent > 0 && (
+                <span className={styles.expander}>
+                    <ShowHideButton
+                        buttonAttrs={{
+                            ...buttonAttrs,
+                            'aria-expanded': expansion === 'expanded',
+                            'aria-pressed': expansion === 'expanded',
+                        }}
+                        onClick={onToggle}
+                        text={expansion === 'expanded' ? t('trackerStatsHideLabel') : t('trackerStatsToggleLabel')}
+                    />
+                </span>
+            )}
             <p className={styles.subtitle}>
                 {recent === 0 && t('trackerStatsNoActivity')}
                 {recent > 0 && recentTitle}
@@ -136,10 +137,10 @@ export function Heading({ expansion, trackerCompanies, totalCount, onToggle, but
  */
 // eslint-disable-next-line no-redeclare
 export function Body({ trackerCompanies, listAttrs = {} }) {
-    const max = trackerCompanies[0]?.count ?? 0;
     const { t } = useTypedTranslation();
     const [formatter] = useState(() => new Intl.NumberFormat());
     const sorted = sortStatsForDisplay(trackerCompanies);
+    const max = sorted[0]?.count ?? 0;
 
     return (
         <ul {...listAttrs} class={styles.list} data-testid="CompanyList">

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
@@ -5,7 +5,7 @@ test.describe('newtab privacy stats', () => {
     test('fetches config + stats', async ({ page }, workerInfo) => {
         const ntp = NewtabPage.create(page, workerInfo);
         await ntp.reducedMotion();
-        await ntp.openPage();
+        await ntp.openPage({ additional: { stats: 'few' } });
 
         const calls1 = await ntp.mocks.waitForCallCount({ method: 'initialSetup', count: 1 });
         const calls2 = await ntp.mocks.waitForCallCount({ method: 'stats_getData', count: 1 });
@@ -22,5 +22,45 @@ test.describe('newtab privacy stats', () => {
         expect(await listItems.nth(2).textContent()).toBe('Amazon67');
         expect(await listItems.nth(3).textContent()).toBe('Google Ads2');
         expect(await listItems.nth(4).textContent()).toBe('Other210');
+
+        // show/hide
+        await page.getByLabel('Hide recent activity').click();
+        await page.getByLabel('Show recent activity').click();
     });
+    test(
+        'hiding the expander when empty',
+        {
+            annotation: {
+                type: 'issue',
+                description: 'https://app.asana.com/0/0/1208792040873366/f',
+            },
+        },
+        async ({ page }, workerInfo) => {
+            const ntp = NewtabPage.create(page, workerInfo);
+            await ntp.reducedMotion();
+            await ntp.openPage({ additional: { stats: 'none' } });
+            await page.getByText('No recent tracking activity').waitFor();
+            await expect(page.getByLabel('Hide recent activity')).not.toBeVisible();
+            await expect(page.getByLabel('Show recent activity')).not.toBeVisible();
+        },
+    );
+    test(
+        'bar width',
+        {
+            annotation: {
+                type: 'issue',
+                description: 'https://app.asana.com/0/0/1208800221025230/f',
+            },
+        },
+        async ({ page }, workerInfo) => {
+            const ntp = NewtabPage.create(page, workerInfo);
+            await ntp.reducedMotion();
+            await ntp.openPage({ additional: { stats: 'willUpdate', 'stats-update-count': '2' } });
+
+            //
+            // Checking the first + last bar widths due to a regression
+            await page.getByText('Google Ads5').locator('[style="width: 100%;"]').waitFor();
+            await page.getByText('Facebook1').locator('[style="width: 20%;"]').waitFor();
+        },
+    );
 });
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
@@ -47,4 +47,29 @@ export const stats = {
         totalCount: 0,
         trackerCompanies: [],
     },
+    willUpdate: {
+        totalCount: 481_113,
+        trackerCompanies: [
+            {
+                displayName: 'Facebook',
+                count: 1,
+            },
+            {
+                displayName: 'Google',
+                count: 1,
+            },
+            {
+                displayName: DDG_STATS_OTHER_COMPANY_IDENTIFIER,
+                count: 1,
+            },
+            {
+                displayName: 'Amazon',
+                count: 1,
+            },
+            {
+                displayName: 'Google Ads',
+                count: 1,
+            },
+        ],
+    },
 };
diff --git a/special-pages/pages/new-tab/app/privacy-stats/privacy-stats.utils.js b/special-pages/pages/new-tab/app/privacy-stats/privacy-stats.utils.js
@@ -8,7 +8,7 @@ import { DDG_STATS_OTHER_COMPANY_IDENTIFIER } from './constants.js';
  * @return {TrackerCompany[]}
  */
 export function sortStatsForDisplay(stats) {
-    const sorted = stats.sort((a, b) => b.count - a.count);
+    const sorted = stats.slice().sort((a, b) => b.count - a.count);
     const other = sorted.findIndex((x) => x.displayName === DDG_STATS_OTHER_COMPANY_IDENTIFIER);
     if (other > -1) {
         const popped = sorted.splice(other, 1);

diff --git a/special-pages/pages/new-tab/integration-tests/new-tab.page.js b/special-pages/pages/new-tab/integration-tests/new-tab.page.js
@@ -58,11 +58,12 @@ export class NewtabPage {
      * @param {boolean} [params.willThrow] - Optional flag to simulate an exception
      * @param {string|number} [params.favorites] - Optional flag to preload a list of favorites
      * @param {string|string[]} [params.nextSteps] - Optional flag to load Next Steps cards
+     * @param {Record<string, any>} [params.additional] - Optional map of key/values to add
      * @param {string} [params.rmf] - Optional flag to add certain rmf example
      * @param {string} [params.updateNotification] - Optional flag to point to display=components view with certain rmf example visible
      * @param {string} [params.platformName] - Optional parameters for opening the page.
      */
-    async openPage({ mode = 'debug', platformName, willThrow = false, favorites, nextSteps, rmf, updateNotification } = {}) {
+    async openPage({ mode = 'debug', additional, platformName, willThrow = false, favorites, nextSteps, rmf, updateNotification } = {}) {
         await this.mocks.install();
         const searchParams = new URLSearchParams({ mode, willThrow: String(willThrow) });
 
@@ -92,6 +93,10 @@ export class NewtabPage {
             searchParams.set('update-notification', updateNotification);
         }
 
+        for (const [key, value] of Object.entries(additional || {})) {
+            searchParams.set(key, value);
+        }
+
         await this.page.goto('/new-tab' + '?' + searchParams.toString());
     }
 

diff --git a/special-pages/pages/new-tab/src/js/mock-transport.js b/special-pages/pages/new-tab/src/js/mock-transport.js
@@ -254,6 +254,33 @@ export function mockTransport() {
 
                     return () => controller.abort();
                 }
+                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,
+                                };
+                            }),
+                        };
+                        cb(next);
+                        inc++;
+                    }, 500);
+                    return () => {
+                        clearInterval(int);
+                    };
+                }
                 case 'favorites_onConfigUpdate': {
                     const controller = new AbortController();
                     channel.addEventListener(
@@ -280,6 +307,10 @@ export function mockTransport() {
             const msg = /** @type {any} */ (_msg);
             switch (msg.method) {
                 case 'stats_getData': {
+                    const statsVariant = url.searchParams.get('stats');
+                    if (statsVariant && statsVariant in stats) {
+                        return Promise.resolve(stats[statsVariant]);
+                    }
                     return Promise.resolve(stats.few);
                 }
                 case 'stats_getConfig': {