Skip to content

Commit

Permalink
ntp: privacy stats ship review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
shakyShane committed Jan 17, 2025
1 parent 92039ea commit 1e14179
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { test, expect } from '@playwright/test';
import { expect, test } from '@playwright/test';
import { NewtabPage } from '../../../integration-tests/new-tab.page.js';
import { CustomizerPage } from './customizer.page.js';

Expand Down Expand Up @@ -255,7 +255,7 @@ test.describe('newtab customizer', () => {
await ntp.reducedMotion();
await ntp.openPage({ additional: { customizerDrawer: 'enabled', theme: 'light' } });
await cp.opensCustomizer();
await cp.hidesSection('Tracking Activity');
await cp.hidesSection('Protection Stats');
});
test('opening settings', async ({ page }, workerInfo) => {
const ntp = NewtabPage.create(page, workerInfo);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { h } from 'preact';
import { noop } from '../../utils.js';
import { PrivacyStatsMockProvider } from '../mocks/PrivacyStatsMockProvider.js';
import { PrivacyStatsConsumer, PrivacyStatsBody, Heading } from './PrivacyStats.js';
import { PrivacyStatsConsumer, PrivacyStatsBody } from './PrivacyStats.js';
import { stats } from '../mocks/stats.js';

/** @type {Record<string, {factory: () => import("preact").ComponentChild}>} */
Expand Down Expand Up @@ -45,16 +44,6 @@ export const privacyStatsExamples = {
'stats.list': {
factory: () => <PrivacyStatsBody trackerCompanies={stats.few.trackerCompanies} listAttrs={{ id: 'example-stats.list' }} />,
},
'stats.heading': {
factory: () => (
<Heading trackerCompanies={stats.few.trackerCompanies} expansion={'expanded'} onToggle={noop('stats.heading onToggle')} />
),
},
'stats.heading.none': {
factory: () => (
<Heading trackerCompanies={stats.none.trackerCompanies} expansion={'expanded'} onToggle={noop('stats.heading onToggle')} />
),
},
};

export const otherPrivacyStatsExamples = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Fragment, h } from 'preact';
import cn from 'classnames';
import styles from './PrivacyStats.module.css';
import { useMessaging, useTypedTranslationWith } from '../../types.js';
import { useContext, useState, useId, useCallback } from 'preact/hooks';
import { useContext, useState, useId, useCallback, useMemo } from 'preact/hooks';
import { PrivacyStatsContext, PrivacyStatsProvider } from '../PrivacyStatsProvider.js';
import { useVisibility } from '../../widget-list/widget-config.provider.js';
import { viewTransition } from '../../utils.js';
Expand Down Expand Up @@ -60,7 +60,18 @@ function WithViewTransitions({ expansion, data, toggle }) {
*/
function PrivacyStatsConfigured({ parentRef, expansion, data, toggle }) {
const expanded = expansion === 'expanded';
const someCompanies = data.trackerCompanies.length > 0;

const { hasNamedCompanies, recent } = useMemo(() => {
let recent = 0;
let hasNamedCompanies = false;
for (let i = 0; i < data.trackerCompanies.length; i++) {
recent += data.trackerCompanies[i].count;
if (!hasNamedCompanies && data.trackerCompanies[i].displayName !== DDG_STATS_OTHER_COMPANY_IDENTIFIER) {
hasNamedCompanies = true;
}
}
return { hasNamedCompanies, recent };
}, [data.trackerCompanies]);

// see: https://www.w3.org/WAI/ARIA/apg/patterns/accordion/examples/accordion/
const WIDGET_ID = useId();
Expand All @@ -69,30 +80,31 @@ function PrivacyStatsConfigured({ parentRef, expansion, data, toggle }) {
return (
<div class={styles.root} ref={parentRef}>
<Heading
trackerCompanies={data.trackerCompanies}
recent={recent}
onToggle={toggle}
expansion={expansion}
canExpand={hasNamedCompanies}
buttonAttrs={{
'aria-controls': WIDGET_ID,
id: TOGGLE_ID,
}}
/>
{expanded && someCompanies && <PrivacyStatsBody trackerCompanies={data.trackerCompanies} listAttrs={{ id: WIDGET_ID }} />}
{hasNamedCompanies && expanded && <PrivacyStatsBody trackerCompanies={data.trackerCompanies} listAttrs={{ id: WIDGET_ID }} />}
</div>
);
}

/**
* @param {object} props
* @param {Expansion} props.expansion
* @param {TrackerCompany[]} props.trackerCompanies
* @param {number} props.recent
* @param {boolean} props.canExpand
* @param {() => void} props.onToggle
* @param {import("preact").ComponentProps<'button'>} [props.buttonAttrs]
*/
export function Heading({ expansion, trackerCompanies, onToggle, buttonAttrs = {} }) {
export function Heading({ expansion, canExpand, recent, onToggle, buttonAttrs = {} }) {
const { t } = useTypedTranslationWith(/** @type {enStrings} */ ({}));
const [formatter] = useState(() => new Intl.NumberFormat());
const recent = trackerCompanies.reduce((sum, item) => sum + item.count, 0);

const none = recent === 0;
const some = recent > 0;
Expand All @@ -106,7 +118,7 @@ export function Heading({ expansion, trackerCompanies, onToggle, buttonAttrs = {
</span>
{none && <h2 className={styles.title}>{t('stats_noRecent')}</h2>}
{some && <h2 className={styles.title}>{alltimeTitle}</h2>}
{recent > 0 && (
{recent > 0 && canExpand && (
<span className={styles.widgetExpander}>
<ShowHideButton
buttonAttrs={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@
.name {
font-size: var(--title-3-em-font-size);
font-weight: var(--title-3-em-font-weight);
line-height: var(--title-3-em-line-height);
text-overflow: ellipsis;
display: block;
overflow: hidden;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ export class PrivacyStatsPage {
expect(await rows.count()).toBe(count);

Check failure on line 44 in special-pages/pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.page.js

View workflow job for this annotation

GitHub Actions / integration

[integration] › pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.spec.js:121:5 › newtab privacy stats › secondary expansion

1) [integration] › pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.spec.js:121:5 › newtab privacy stats › secondary expansion Error: expect(received).toBe(expected) // Object.is equality Expected: 5 Received: 0 at pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.page.js:44 42 | async hasRows(count) { 43 | const rows = this.rows(); > 44 | expect(await rows.count()).toBe(count); | ^ 45 | } 46 | 47 | /** at PrivacyStatsPage.hasRows (/home/runner/work/content-scope-scripts/content-scope-scripts/special-pages/pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.page.js:44:36) at /home/runner/work/content-scope-scripts/content-scope-scripts/special-pages/pages/new-tab/app/privacy-stats/integration-tests/privacy-stats.spec.js:137:13
}

/**
* @param {string} heading
*/
async hasHeading(heading) {
await expect(this.context().getByRole('heading')).toContainText(heading);
}

async showMoreSecondary() {
await this.context().getByLabel('Show More', { exact: true }).click();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ test.describe('newtab privacy stats', () => {
const ntp = NewtabPage.create(page, workerInfo);
await ntp.reducedMotion();
await ntp.openPage({ additional: { stats: 'none' } });
await page.getByText('No recent tracking activity').waitFor();
await page.getByText('Tracking protections active').waitFor();
await expect(page.getByLabel('Hide recent activity')).not.toBeVisible();
await expect(page.getByLabel('Show recent activity')).not.toBeVisible();
},
Expand Down Expand Up @@ -145,4 +145,21 @@ test.describe('newtab privacy stats', () => {
await psp.hasRows(2);
},
);
test(
'when all trackers are not within the top 100 companies',
{
annotation: {
type: 'issue',
description: 'https://app.asana.com/0/1201141132935289/1209123210947322/f',
},
},
async ({ page }, workerInfo) => {
const ntp = NewtabPage.create(page, workerInfo);
const psp = new PrivacyStatsPage(page, ntp);
await ntp.reducedMotion();
await ntp.openPage({ additional: { stats: 'onlyother' } });
await psp.hasRows(0);
await psp.hasHeading('2 tracking attempts blocked');
},
);
});
9 changes: 9 additions & 0 deletions special-pages/pages/new-tab/app/privacy-stats/mocks/stats.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ export const stats = {
totalCount: 0,
trackerCompanies: [],
},
onlyother: {
totalCount: 2,
trackerCompanies: [
{
displayName: DDG_STATS_OTHER_COMPANY_IDENTIFIER,
count: 2,
},
],
},
willUpdate: {
totalCount: 481_113,
trackerCompanies: [
Expand Down
6 changes: 3 additions & 3 deletions special-pages/pages/new-tab/app/privacy-stats/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
"note": "Used as a label in a customization menu"
},
"stats_menuTitle_v2": {
"title": "Tracking Activity",
"title": "Protection Stats",
"note": "Used as a label in a customization menu"
},
"stats_noActivity": {
"title": "Blocked tracking attempts will appear here. Keep browsing to see how many we block.",
"title": "DuckDuckGo blocks tracking attempts as you browse. Visit a few sites to see how many we block!",
"note": "Placeholder for when we cannot report any blocked trackers yet"
},
"stats_noRecent": {
"title": "No recent tracking activity",
"title": "Tracking protections active",
"note": "Placeholder to indicate that no tracking activity was blocked in the last 7 days"
},
"stats_countBlockedSingular": {
Expand Down
6 changes: 3 additions & 3 deletions special-pages/pages/new-tab/public/locales/en/new-tab.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@
"note": "Used as a label in a customization menu"
},
"stats_menuTitle_v2": {
"title": "Tracking Activity",
"title": "Protection Stats",
"note": "Used as a label in a customization menu"
},
"stats_noActivity": {
"title": "Blocked tracking attempts will appear here. Keep browsing to see how many we block.",
"title": "DuckDuckGo blocks tracking attempts as you browse. Visit a few sites to see how many we block!",
"note": "Placeholder for when we cannot report any blocked trackers yet"
},
"stats_noRecent": {
"title": "No recent tracking activity",
"title": "Tracking protections active",
"note": "Placeholder to indicate that no tracking activity was blocked in the last 7 days"
},
"stats_countBlockedSingular": {
Expand Down

0 comments on commit 1e14179

Please sign in to comment.