diff --git a/packages/fxa-settings/src/lib/glean/index.test.ts b/packages/fxa-settings/src/lib/glean/index.test.ts index e282327ef8e..11adb2709ad 100644 --- a/packages/fxa-settings/src/lib/glean/index.test.ts +++ b/packages/fxa-settings/src/lib/glean/index.test.ts @@ -15,6 +15,7 @@ import * as login from 'fxa-shared/metrics/glean/web/login'; import * as accountPref from 'fxa-shared/metrics/glean/web/accountPref'; import * as passkey from 'fxa-shared/metrics/glean/web/passkey'; import * as promoQrMobile from 'fxa-shared/metrics/glean/web/promoQrMobile'; +import * as dtmDesktop from 'fxa-shared/metrics/glean/web/dtmDesktop'; import * as accountBanner from 'fxa-shared/metrics/glean/web/accountBanner'; import * as deleteAccount from 'fxa-shared/metrics/glean/web/deleteAccount'; import * as thirdPartyAuth from 'fxa-shared/metrics/glean/web/thirdPartyAuth'; @@ -910,6 +911,17 @@ describe('lib/glean', () => { }); }); + describe('dtmDesktop', () => { + it('submits a ping with the dtm_desktop_qr_skip name', async () => { + const spy = sandbox.spy(dtmDesktop.qrSkip, 'record'); + GleanMetrics.dtmDesktop.qrSkip(); + await GleanMetrics.isDone(); + sinon.assert.calledOnce(setEventNameStub); + sinon.assert.calledWith(setEventNameStub, 'dtm_desktop_qr_skip'); + sinon.assert.calledOnce(spy); + }); + }); + describe('loginTotpBackup', () => { it('submits a ping with the login_backup_code_view name', async () => { GleanMetrics.loginBackupCode.view(); diff --git a/packages/fxa-settings/src/lib/glean/index.ts b/packages/fxa-settings/src/lib/glean/index.ts index c8f18457857..44775f0759b 100644 --- a/packages/fxa-settings/src/lib/glean/index.ts +++ b/packages/fxa-settings/src/lib/glean/index.ts @@ -20,6 +20,7 @@ import * as event from 'fxa-shared/metrics/glean/web/event'; import * as email from 'fxa-shared/metrics/glean/web/email'; import * as error from 'fxa-shared/metrics/glean/web/error'; import * as promoQrMobile from 'fxa-shared/metrics/glean/web/promoQrMobile'; +import * as dtmDesktop from 'fxa-shared/metrics/glean/web/dtmDesktop'; import * as reg from 'fxa-shared/metrics/glean/web/reg'; import * as login from 'fxa-shared/metrics/glean/web/login'; import * as cachedLogin from 'fxa-shared/metrics/glean/web/cachedLogin'; @@ -962,6 +963,9 @@ const recordEventMetric = ( nimbus_user_id: gleanPingMetrics?.event?.['nimbusUserId'] || '', }); break; + case 'dtm_desktop_qr_skip': + dtmDesktop.qrSkip.record(); + break; } }; diff --git a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/container.test.tsx b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/container.test.tsx index 8951bdc3626..16bdb6d701e 100644 --- a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/container.test.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/container.test.tsx @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider'; import { MemoryRouter } from 'react-router'; import * as Sentry from '@sentry/react'; @@ -22,6 +23,16 @@ jest.mock('react-router', () => ({ useNavigate: () => mockNavigate, })); +const mockQrSkip = jest.fn(); +jest.mock('../../../../lib/glean', () => ({ + __esModule: true, + default: { + dtmDesktop: { + qrSkip: (...args: unknown[]) => mockQrSkip(...args), + }, + }, +})); + // Stub QRCode so the test can read the encoded value without decoding an SVG. // The container's contract is the value it hands down; how that value is drawn // is the page's concern, covered in index.test.tsx. @@ -228,6 +239,58 @@ describe('Pair2/Authority/ScanQR container', () => { expect(mockNavigate).not.toHaveBeenCalled(); }); + describe('skip', () => { + // Waits for the QR to render so the channel is fully set up before the + // user leaves, as it would be in the browser. + const skip = async () => { + const user = userEvent.setup(); + renderContainer(); + await waitFor(() => + expect(screen.getByTestId('scan-qr-code')).toHaveAttribute( + 'data-value', + MOCK_PAIR_URL + ) + ); + + await user.click(screen.getByRole('button', { name: 'Skip for now' })); + }; + + it('records the qr_skip Glean event', async () => { + await skip(); + + expect(mockQrSkip).toHaveBeenCalledTimes(1); + }); + + // Skipping ends the flow, so unlike the other exits from this page the + // channel does not outlive it. + it('closes the channel', async () => { + await skip(); + + expect(integration.destroy).toHaveBeenCalledTimes(1); + }); + + // Every pairing promo exits to settings. The pairing query parameters stay + // behind: nothing in settings reads them. + it('navigates to settings without the pairing query', async () => { + await skip(); + + expect(mockNavigate).toHaveBeenCalledTimes(1); + expect(mockNavigate).toHaveBeenCalledWith('/settings'); + }); + + // Leaving must not wait on the channel: one that will not close is the + // integration's problem to report, not a reason to hold the user here. + it('still navigates and reports to Sentry when the channel will not close', async () => { + const err = new Error('socket already gone'); + integration.destroy.mockRejectedValue(err); + + await skip(); + + expect(mockNavigate).toHaveBeenCalledWith('/settings'); + await waitFor(() => expect(captureException).toHaveBeenCalledWith(err)); + }); + }); + it('throws when handed an integration that is not the pairing authority', () => { expect(() => renderContainer(MOCK_NON_PAIRING_INTEGRATION)).toThrow( 'Invalid integration type. Expected PairingAuthorityIntegration.' diff --git a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/container.tsx b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/container.tsx index 6f22e4b84df..c56d845a4b6 100644 --- a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/container.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/container.tsx @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import React, { useEffect, useState } from 'react'; +import { useNavigate } from 'react-router'; import * as Sentry from '@sentry/react'; import ScanQR from '.'; import { @@ -11,15 +12,18 @@ import { PairingAuthorityIntegration, } from '../../../../models'; import { useNavigateWithQuery } from '../../../../lib/hooks'; +import GleanMetrics from '../../../../lib/glean'; /** * Owns the pairing channel for the authority. Mints a channel on mount so the * QR always scans to one that exists on the channel server. The channel then * outlives this page — the authority moves on while the supplicant is still - * joining — so it is only torn down when creation itself fails. + * joining — so it is only torn down when creation itself fails or the user + * skips pairing. */ const ScanQRContainer = ({ integration }: { integration: Integration }) => { const navigateWithQuery = useNavigateWithQuery(); + const navigate = useNavigate(); const [qrCodeValue, setQrCodeValue] = useState(''); if (!(integration instanceof PairingAuthorityIntegration)) { @@ -79,7 +83,18 @@ const ScanQRContainer = ({ integration }: { integration: Integration }) => { }; }, [integration, navigateWithQuery]); - return ; + const onSkip = () => { + GleanMetrics.dtmDesktop.qrSkip(); + // Skipping ends the flow, so the channel goes with it. `destroy()` drops + // the state handler first, so the close cannot route to the timeout page. + // Not awaited: a channel that will not close must not hold the user here. + integration.destroy().catch((err) => Sentry.captureException(err)); + // Settings is the exit from every pairing promo, so the pairing query + // parameters stop here rather than following the user there. + navigate('/settings'); + }; + + return ; }; export default ScanQRContainer; diff --git a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/en.ftl b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/en.ftl index a458faf1351..de22e63647c 100644 --- a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/en.ftl +++ b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/en.ftl @@ -9,3 +9,5 @@ pair2-authority-scan-qr-instruction = Scan the QR code with your phone or tablet pair2-authority-scan-qr-code-aria-label = QR code to connect your mobile device # Link to a support article for users having trouble scanning the QR code pair2-authority-scan-qr-help-link = Get help scanning +# Button shown below the QR code card. Leaves the pairing flow and takes the user to their account settings. +pair2-authority-scan-qr-skip-button = Skip for now diff --git a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/index.test.tsx b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/index.test.tsx index cdf4c9f69cd..996f8bb0bf4 100644 --- a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/index.test.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/index.test.tsx @@ -3,6 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { FluentBundle } from '@fluent/bundle'; import { getFtlBundle, testL10n } from 'fxa-react/lib/test-utils'; import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider'; @@ -121,10 +122,17 @@ describe('Pair2/Authority/ScanQR page', () => { renderWithLocalizationProvider(); const link = screen.getByRole('link', { name: /Get help scanning/ }); - expect(link).toHaveAttribute( - 'href', - Constants.SYNC_SUMO_URL - ); + expect(link).toHaveAttribute('href', Constants.SYNC_SUMO_URL); expect(link).toHaveAttribute('target', '_blank'); }); + + it('calls onSkip from the skip button', async () => { + const user = userEvent.setup(); + const onSkip = jest.fn(); + renderWithLocalizationProvider(); + + await user.click(screen.getByRole('button', { name: 'Skip for now' })); + + expect(onSkip).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/index.tsx b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/index.tsx index e53b43d664c..2c0f6db28f9 100644 --- a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/index.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/index.tsx @@ -18,14 +18,17 @@ export type ScanQRProps = { * pairing value lands with the flow wiring, and this card only renders it. */ qrCodeValue?: string; + /** Leaves the pairing flow without scanning. */ + onSkip?: () => void; }; /** * The desktop screen that shows the pairing QR code. The user scans it with - * their phone or tablet to start syncing; there is no button to press, so the - * only action is the link out to scanning help. + * their phone or tablet to start syncing. Pairing is one of several promos a + * sync sign-in can land on, so the card is followed by a way out of the flow + * alongside the link to scanning help. */ -const ScanQR = ({ qrCodeValue }: ScanQRProps) => { +const ScanQR = ({ qrCodeValue, onSkip }: ScanQRProps) => { const ftlMsgResolver = useFtlMsgResolver(); // `QRCode` takes a plain string, so this is the one label on the card that // cannot be resolved with `FtlMsg`. @@ -35,8 +38,11 @@ const ScanQR = ({ qrCodeValue }: ScanQRProps) => { ); return ( - -
+ // The card is rendered here rather than by `AppLayout` so the skip button + // can sit below it: inside, the artwork is flush with the card's bottom + // edge and leaves no room. + +

Scan to connect your mobile device

 @@ -96,6 +102,20 @@ const ScanQR = ({ qrCodeValue }: ScanQRProps) => {
+ + {/* Below `mobileLandscape` the card is transparent and has no bottom + margin, so the gap to the artwork is set here. */} +
+ + + +
); }; diff --git a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/mocks.tsx b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/mocks.tsx index f01a32def2b..46be0d48c28 100644 --- a/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/mocks.tsx +++ b/packages/fxa-settings/src/pages/Pair2/Authority/ScanQR/mocks.tsx @@ -19,7 +19,8 @@ export const MOCK_LONG_QR_CODE_VALUE = MOCK_QR_CODE_VALUE.repeat(12); export const Subject = ({ qrCodeValue = MOCK_QR_CODE_VALUE, -}: Partial = {}) => ; + onSkip = () => {}, +}: Partial = {}) => ; /** The channel URL a successfully created channel resolves to. */ export const MOCK_PAIR_URL = `${window.location.origin}/pair#channel_id=chan-1&channel_key=key-1&v=2`; diff --git a/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml b/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml index 0454b8dff9c..3137e057872 100644 --- a/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml +++ b/packages/fxa-shared/metrics/glean/fxa-ui-metrics.yaml @@ -4439,3 +4439,23 @@ promo_qr_mobile: branch: description: The value proposition experiment branch shown to the user type: string + +dtm_desktop: + qr_skip: + type: event + description: | + User clicked "Skip for now" on the desktop (authority) QR code screen, + leaving the pairing flow for account settings. + send_in_pings: + - events + notification_emails: + - vzare@mozilla.com + - fxa-staff@mozilla.com + bugs: + - https://mozilla-hub.atlassian.net/browse/FXA-14523 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1830504 + - https://bugzilla.mozilla.org/show_bug.cgi?id=1844121 + expires: never + data_sensitivity: + - interaction diff --git a/packages/fxa-shared/metrics/glean/web/dtmDesktop.ts b/packages/fxa-shared/metrics/glean/web/dtmDesktop.ts new file mode 100644 index 00000000000..d4c8b2f7dfd --- /dev/null +++ b/packages/fxa-shared/metrics/glean/web/dtmDesktop.ts @@ -0,0 +1,24 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +// AUTOGENERATED BY glean_parser v19.2.0. DO NOT EDIT. DO NOT COMMIT. + +import EventMetricType from '@mozilla/glean/private/metrics/event'; + +/** + * User clicked "Skip for now" on the desktop (authority) QR code screen, + * leaving the pairing flow for account settings. + * + * Generated from `dtm_desktop.qr_skip`. + */ +export const qrSkip = new EventMetricType( + { + category: 'dtm_desktop', + name: 'qr_skip', + sendInPings: ['events'], + lifetime: 'ping', + disabled: false, + }, + [] +); diff --git a/packages/fxa-shared/metrics/glean/web/index.ts b/packages/fxa-shared/metrics/glean/web/index.ts index f10f9621c5e..29c317a010b 100644 --- a/packages/fxa-shared/metrics/glean/web/index.ts +++ b/packages/fxa-shared/metrics/glean/web/index.ts @@ -361,4 +361,8 @@ export const eventsMap = { promoQrMobile: { view: 'promo_qr_mobile_view', }, + + dtmDesktop: { + qrSkip: 'dtm_desktop_qr_skip', + }, } as const;