Skip to content

Commit 32f8244

Browse files
committed
ntp: fixed the empty state of activity widget when trackers are present
1 parent 30cf67e commit 32f8244

File tree

7 files changed

+98
-8
lines changed

7 files changed

+98
-8
lines changed

special-pages/pages/new-tab/app/activity/components/Activity.examples.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ export const activityExamples = {
3131
</Activity>
3232
),
3333
},
34+
'activity.noActivity.someTrackers': {
35+
factory: () => (
36+
<Activity expansion={'collapsed'} itemCount={0} trackerCount={56} toggle={noop('toggle')} batched={false}>
37+
<Mock size={0}>
38+
<ActivityBody canBurn={false} visibility={'visible'} />
39+
</Mock>
40+
</Activity>
41+
),
42+
},
3443
};
3544

3645
/**

special-pages/pages/new-tab/app/activity/integration-tests/activity.page.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,4 +426,13 @@ export class ActivityPage {
426426
- paragraph: Past 7 days
427427
`);
428428
}
429+
430+
async hasTrackingInfoWithoutButtons() {
431+
const { page } = this;
432+
await expect(page.getByTestId('ActivityHeading')).toMatchAriaSnapshot(`
433+
- img "Privacy Shield"
434+
- heading "56 tracking attempts blocked" [level=2]
435+
- paragraph: Past 7 days
436+
`);
437+
}
429438
}

special-pages/pages/new-tab/app/activity/integration-tests/activity.spec.js

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,35 @@ test.describe('activity widget', () => {
219219
});
220220

221221
await widget.hasRows(5);
222-
await batching.removesItem(0);
222+
await batching.itemRemovedViaPatch(0);
223223
await widget.hasRows(4);
224224
});
225+
test('patching removes last item and maintains tracker count', async ({ page }, workerInfo) => {
226+
test.info().annotations.push({
227+
type: 'link',
228+
description: 'https://app.asana.com/1/137249556945/project/1207414201589134/task/1210188026604205?focus=true',
229+
});
230+
test.info().annotations.push({
231+
type: 'info',
232+
description: `
233+
This test simulates removing the last item in the list. When that occurs, the tracker count
234+
should remain if it was greater than zero. The bug here was that we'd reset back to 'no tracking activity'.
235+
`,
236+
});
237+
const ntp = NewtabPage.create(page, workerInfo);
238+
await ntp.reducedMotion();
239+
240+
const widget = new ActivityPage(page, ntp);
241+
const batching = new BatchingPage(page, ntp, widget);
242+
243+
await ntp.openPage({
244+
additional: { feed: 'activity', 'activity.api': 'batched', platform: 'windows', activity: 'singleWithTrackers' },
245+
});
246+
247+
await widget.hasRows(1);
248+
await batching.removesItem({ index: 0, nextTrackerCount: 56 }); // from activity/mocks/activity.mocks.js
249+
await widget.hasTrackingInfoWithoutButtons();
250+
});
225251
test('items are fetched to replace patched removals', async ({ page }, workerInfo) => {
226252
const ntp = NewtabPage.create(page, workerInfo);
227253
await ntp.reducedMotion();

special-pages/pages/new-tab/app/activity/integration-tests/batching.page.js

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,28 @@ export class BatchingPage {
106106
*
107107
* @param {number} index - The index of the item to remove from the sample data array.
108108
*/
109-
async removesItem(index) {
109+
async itemRemovedViaPatch(index) {
110110
const data = generateSampleData(this.ap.entries);
111111
data.splice(index, 1);
112112
const update = toPatch(data);
113113
await this.ntp.mocks.simulateSubscriptionMessage(sub('activity_onDataPatch'), update);
114114
}
115115

116+
/**
117+
* Simulates removing all items
118+
* @param {object} params
119+
* @param {number} params.index - The index of the item to remove from the sample data array.
120+
* @param {number} params.nextTrackerCount
121+
*/
122+
async removesItem({ index, nextTrackerCount }) {
123+
const { page } = this;
124+
await page.locator('button[data-action="remove"]').nth(index).click();
125+
126+
const update = toPatch([]);
127+
update.totalTrackersBlocked = nextTrackerCount;
128+
await this.ntp.mocks.simulateSubscriptionMessage(sub('activity_onDataPatch'), update);
129+
}
130+
116131
async fillsHoleWhenItemRemoved() {
117132
if (this.ap.entries !== 6) throw new Error('this scenario expects 6 initial items');
118133

@@ -131,7 +146,7 @@ export class BatchingPage {
131146
});
132147

133148
// now remove the first item via subscription
134-
await this.removesItem(0);
149+
await this.itemRemovedViaPatch(0);
135150

136151
// now there must have been 2 calls
137152
const [, second] = await this.ntp.mocks.waitForCallCount({ method: 'activity_getDataForUrls', count: 2 });

special-pages/pages/new-tab/app/activity/mocks/activity.mock-transport.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,17 @@ export function activityMockTransport() {
3636
/** @type {import('../../../types/new-tab.ts').NewTabMessages['notifications']} */
3737
const msg = /** @type {any} */ (_msg);
3838
switch (msg.method) {
39+
case 'activity_removeItem': {
40+
const oldCount = dataset.activity.reduce((acc, item) => acc + item.trackingStatus.totalCount, 0);
41+
dataset.activity = dataset.activity.filter((x) => x.url !== msg.params.url);
42+
const cb = subs.get('activity_onDataPatch');
43+
const p = toPatch(dataset.activity);
44+
p.totalTrackersBlocked = oldCount;
45+
setTimeout(() => {
46+
cb(p);
47+
}, 0);
48+
break;
49+
}
3950
default: {
4051
console.warn('unhandled notification', msg);
4152
}
@@ -53,6 +64,9 @@ export function activityMockTransport() {
5364
if (sub === 'activity_onDataUpdate') {
5465
subs.set('activity_onDataUpdate', cb);
5566
}
67+
if (sub === 'activity_onDataPatch') {
68+
subs.set('activity_onDataPatch', cb);
69+
}
5670
if (sub === 'activity_onDataUpdate' && url.searchParams.has('flood')) {
5771
let count = 0;
5872
const int = setInterval(() => {

special-pages/pages/new-tab/app/activity/mocks/activity.mocks.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,23 @@ export const activityMocks = {
4040
},
4141
],
4242
},
43+
singleWithTrackers: {
44+
activity: [
45+
{
46+
favicon: { src: 'selco-icon.png' },
47+
url: 'https://example.com',
48+
title: 'example.com',
49+
etldPlusOne: 'example.com',
50+
favorite: false,
51+
trackersFound: true,
52+
trackingStatus: {
53+
trackerCompanies: [{ displayName: 'Google' }, { displayName: 'Facebook' }, { displayName: 'Amazon' }],
54+
totalCount: 56,
55+
},
56+
history: [],
57+
},
58+
],
59+
},
4360
few: {
4461
activity: [
4562
{

special-pages/pages/new-tab/app/privacy-stats/components/ActivityHeading.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ export function ActivityHeading({ expansion, canExpand, itemCount, trackerCount,
2424
const [formatter] = useState(() => new Intl.NumberFormat());
2525
const adBlocking = useAdBlocking();
2626

27-
const none = itemCount === 0;
28-
const someItems = itemCount > 0;
27+
const none = itemCount === 0 && trackerCount === 0;
28+
const some = itemCount > 0 || trackerCount > 0;
2929
const trackerCountFormatted = formatter.format(trackerCount);
3030

3131
let allTimeString;
@@ -46,7 +46,7 @@ export function ActivityHeading({ expansion, canExpand, itemCount, trackerCount,
4646
<img src={adBlocking ? './icons/shield-green.svg' : './icons/shield.svg'} alt="Privacy Shield" />
4747
</span>
4848
{none && <h2 className={styles.title}>{t('activity_noRecent_title')}</h2>}
49-
{someItems && (
49+
{some && (
5050
<h2 className={styles.title}>
5151
<Trans str={allTimeString} values={{ count: trackerCountFormatted }} />
5252
</h2>
@@ -64,12 +64,12 @@ export function ActivityHeading({ expansion, canExpand, itemCount, trackerCount,
6464
/>
6565
</span>
6666
)}
67-
{itemCount === 0 && (
67+
{none && (
6868
<p className={cn(styles.subtitle, { [styles.indented]: !adBlocking })}>
6969
{adBlocking ? t('activity_noRecentAdsAndTrackers_subtitle') : t('activity_noRecent_subtitle')}
7070
</p>
7171
)}
72-
{itemCount > 0 && (
72+
{some && (
7373
<p className={cn(styles.subtitle, styles.indented, { [styles.uppercase]: !adBlocking })}>
7474
{t('stats_feedCountBlockedPeriod')}
7575
</p>

0 commit comments

Comments
 (0)