Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ntp: add telemetry events for show/hide #1280

Merged
merged 2 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions special-pages/messages/new-tab/telemetryEvent.notify.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "NTP TelemetryEvent",
"type": "object",
"required": ["attributes"],
"properties": {
"attributes": {
"oneOf": [
{
"type": "object",
"title": "Stats Show More",
"required": ["name", "value"],
"properties": {
"name": {
"const": "stats_toggle"
},
"value": {
"type": "string",
"enum": ["show_more", "show_less"]
}
}
},
{
"type": "object",
"title": "Example Telemetry Event",
"required": ["name"],
"properties": {
"name": {
"const": "ntp_example"
}
}
}
]
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ export class FavoritesService {
});
}

name() {
return 'FavoritesService';
}

/**
* @returns {Promise<{data: FavoritesData; config: FavoritesConfig}>}
* @internal
Expand Down
2 changes: 1 addition & 1 deletion special-pages/pages/new-tab/app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { callWithRetry } from '../../../shared/call-with-retry.js';
* @throws Error
*/
export async function init(root, messaging, telemetry, baseEnvironment) {
const result = await callWithRetry(() => messaging.init());
const result = await callWithRetry(() => messaging.initialSetup());

// handle fatal exceptions, the following things prevent anything from starting.
if ('error' in result) {
Expand Down
20 changes: 17 additions & 3 deletions special-pages/pages/new-tab/app/new-tab.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ children:

## Notifications

- {@link "NewTab Messages".ContextMenuNotification `contextMenu`}
### {@link "NewTab Messages".ContextMenuNotification `contextMenu`}
- Sent when the user right-clicks in the page
- Note: Other widgets might prevent this (and send their own, eg: favorites)
- Sends: {@link "NewTab Messages".ContextMenuNotify}
Expand All @@ -41,10 +41,24 @@ children:
}
```

- {@link "NewTab Messages".ReportInitExceptionNotification `reportInitException`}
### {@link "NewTab Messages".TelemetryEventNotification `telemetryEvent`}
- These are generic events that might be useful to observe. For example, you can use these to decide when to send pixels.
- Sends a standard format `{ attributes: { name: string', value?: any } }` - see {@link "NewTab Messages".TelemetryEventNotification `telemetryEvent`}
- Example:

```json
{
"attributes": {
"name": "stats_toggle",
"value": "show_more"
}
}
```

### {@link "NewTab Messages".ReportInitExceptionNotification `reportInitException`}
- Sent when the application fails to initialize (for example, a JavaScript exception prevented it)
- Sends: `{ message: string }` - see {@link "NewTab Messages".ReportInitExceptionNotify}

- {@link "NewTab Messages".ReportPageExceptionNotification `reportPageException`}
### {@link "NewTab Messages".ReportPageExceptionNotification `reportPageException`}
- Sent when the application failed after initialization (for example, a JavaScript exception prevented it)
- Sends: `{ message: string }` - see {@link "NewTab Messages".ReportPageExceptionNotify}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ export class NextStepsService {
});
}

name() {
return 'NextStepsService';
}

/**
* @returns {Promise<{data: NextStepsData; config: NextStepsConfig}>}
* @internal
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Fragment, h } from 'preact';
import cn from 'classnames';
import styles from './PrivacyStats.module.css';
import { useTypedTranslationWith } from '../../types.js';
import { useMessaging, useTypedTranslationWith } from '../../types.js';
import { useContext, useState, useId, useCallback } from 'preact/hooks';
import { PrivacyStatsContext, PrivacyStatsProvider } from '../PrivacyStatsProvider.js';
import { useVisibility } from '../../widget-list/widget-config.provider.js';
Expand Down Expand Up @@ -133,6 +133,7 @@ export function Heading({ expansion, trackerCompanies, onToggle, buttonAttrs = {

export function PrivacyStatsBody({ trackerCompanies, listAttrs = {} }) {
const { t } = useTypedTranslationWith(/** @type {enStrings} */ ({}));
const messaging = useMessaging();
const [formatter] = useState(() => new Intl.NumberFormat());
const defaultRowMax = 5;
const sorted = sortStatsForDisplay(trackerCompanies);
Expand All @@ -141,6 +142,11 @@ export function PrivacyStatsBody({ trackerCompanies, listAttrs = {} }) {
const hasmore = sorted.length > visible;

const toggleListExpansion = () => {
if (hasmore) {
messaging.telemetryEvent({ attributes: { name: 'stats_toggle', value: 'show_more' } });
} else {
messaging.telemetryEvent({ attributes: { name: 'stats_toggle', value: 'show_less' } });
}
if (visible === defaultRowMax) {
setVisible(sorted.length);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,33 @@ test.describe('newtab privacy stats', () => {
await page.getByLabel('Hide recent activity').click();
await page.getByLabel('Show recent activity').click();
});
test('sending a pixel when show more is clicked', async ({ page }, workerInfo) => {
const ntp = NewtabPage.create(page, workerInfo);
await ntp.reducedMotion();
await ntp.openPage({ additional: { stats: 'many' } });
await page.getByLabel('Show More', { exact: true }).click();
await page.getByLabel('Show Less').click();
const calls1 = await ntp.mocks.waitForCallCount({ method: 'telemetryEvent', count: 2 });
expect(calls1.length).toBe(2);
expect(calls1).toStrictEqual([
{
payload: {
context: 'specialPages',
featureName: 'newTabPage',
method: 'telemetryEvent',
params: { attributes: { name: 'stats_toggle', value: 'show_more' } },
},
},
{
payload: {
context: 'specialPages',
featureName: 'newTabPage',
method: 'telemetryEvent',
params: { attributes: { name: 'stats_toggle', value: 'show_less' } },
},
},
]);
});
test(
'hiding the expander when empty',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ export class PrivacyStatsService {
});
}

name() {
return 'PrivacyStatsService';
}

/**
* @returns {Promise<{data: PrivacyStatsData; config: StatsConfig}>}
* @internal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ export class RMFService {
});
}

name() {
return 'RMFService';
}

/**
* @returns {Promise<RMFData>}
* @internal
Expand Down
21 changes: 14 additions & 7 deletions special-pages/pages/new-tab/app/service.hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
*/

import { useCallback, useEffect } from 'preact/hooks';
import { useMessaging } from './types.js';

/**
* @template D
Expand Down Expand Up @@ -86,14 +87,16 @@ export function reducer(state, event) {
* @param {import("preact").RefObject<{
* getInitial: () => Promise<{data: D, config: C}>;
* destroy: () => void;
* name: () => string;
* }>} params.service
*/
export function useInitialDataAndConfig({ dispatch, service }) {
const messaging = useMessaging();
useEffect(() => {
if (!service.current) return console.warn('missing service');
const stats = service.current;
const srv = service.current;
shakyShane marked this conversation as resolved.
Show resolved Hide resolved
async function init() {
const { config, data } = await stats.getInitial();
const { config, data } = await srv.getInitial();
if (data) {
dispatch({ kind: 'initial-data', data, config });
} else {
Expand All @@ -107,12 +110,13 @@ export function useInitialDataAndConfig({ dispatch, service }) {
init().catch((e) => {
console.error('uncaught error', e);
dispatch({ kind: 'error', error: e });
messaging.reportPageException({ message: `${srv.name()}: failed to fetch initial data+config: ` + e.message });
});

return () => {
stats.destroy();
srv.destroy();
};
}, []);
}, [messaging]);
}

/**
Expand All @@ -122,14 +126,16 @@ export function useInitialDataAndConfig({ dispatch, service }) {
* @param {import("preact").RefObject<{
* getInitial: () => Promise<D>;
* destroy: () => void;
* name: () => string;
* }>} params.service
*/
export function useInitialData({ dispatch, service }) {
const messaging = useMessaging();
useEffect(() => {
if (!service.current) return console.warn('missing service');
const stats = service.current;
const srv = service.current;
async function init() {
const data = await stats.getInitial();
const data = await srv.getInitial();
if (data) {
dispatch({ kind: 'initial-data', data });
} else {
Expand All @@ -143,10 +149,11 @@ export function useInitialData({ dispatch, service }) {
init().catch((e) => {
console.error('uncaught error', e);
dispatch({ kind: 'error', error: e });
messaging.reportPageException({ message: `${srv.name()}: failed to fetch initial data: ` + e.message });
});

return () => {
stats.destroy();
srv.destroy();
};
}, []);
}
Expand Down
9 changes: 8 additions & 1 deletion special-pages/pages/new-tab/src/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class NewTabPage {
/**
* @return {Promise<import('../../../../types/new-tab.js').InitialSetupResponse>}
*/
init() {
initialSetup() {
return this.messaging.request('initialSetup');
}

Expand All @@ -55,6 +55,13 @@ export class NewTabPage {
contextMenu(params) {
this.messaging.notify('contextMenu', params);
}

/**
* @param {import("../../../../types/new-tab.js").NTPTelemetryEvent} event
*/
telemetryEvent(event) {
this.messaging.notify('telemetryEvent', event);
}
}

const baseEnvironment = new Environment().withInjectName(import.meta.injectName).withEnv(import.meta.env);
Expand Down
18 changes: 18 additions & 0 deletions special-pages/types/new-tab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export interface NewTabMessages {
| RmfPrimaryActionNotification
| RmfSecondaryActionNotification
| StatsSetConfigNotification
| TelemetryEventNotification
| UpdateNotificationDismissNotification
| WidgetsSetConfigNotification;
requests:
Expand Down Expand Up @@ -274,6 +275,23 @@ export interface StatsConfig {
expansion: Expansion;
animation?: Animation;
}
/**
* Generated from @see "../messages/new-tab/telemetryEvent.notify.json"
*/
export interface TelemetryEventNotification {
method: "telemetryEvent";
params: NTPTelemetryEvent;
}
export interface NTPTelemetryEvent {
attributes: StatsShowMore | ExampleTelemetryEvent;
}
export interface StatsShowMore {
name: "stats_toggle";
value: "show_more" | "show_less";
}
export interface ExampleTelemetryEvent {
name: "ntp_example";
}
/**
* Generated from @see "../messages/new-tab/updateNotification_dismiss.notify.json"
*/
Expand Down
Loading