From 7abf9122013209e1b326861f90a6c9619b30e6c3 Mon Sep 17 00:00:00 2001 From: Lauren Zugai Date: Fri, 23 Aug 2024 09:34:09 -0500 Subject: [PATCH] feat(settings): Refactor 'Data Collection' for new content + UnitRow adjustments Because: * We want to add a new 'Firefox browser' row to our Data Collection and Use section This commit: * Refactors DataCollection to use our standard UnitRow component per the new content design * Adjusts UnitRow for new hideHeaderValue prop, doesn't render action div if action wasn't provided, renames noHeaderValueText to defaultHeaderValueText for clarity * Tweaks related stories and tests closes FXA-10297 --- .../components/Settings/DataCollection/en.ftl | 4 +- .../Settings/DataCollection/index.test.tsx | 25 ++- .../Settings/DataCollection/index.tsx | 96 +++++++----- .../src/components/Settings/Profile/index.tsx | 2 +- .../components/Settings/Security/index.tsx | 4 +- .../Settings/UnitRow/index.stories.tsx | 15 +- .../Settings/UnitRow/index.test.tsx | 31 ++-- .../src/components/Settings/UnitRow/index.tsx | 143 ++++++++++-------- .../src/components/Settings/UnitRow/mocks.tsx | 1 - .../Settings/UnitRowSecondaryEmail/index.tsx | 1 - .../Settings/UnitRowTwoStepAuth/index.tsx | 7 +- 11 files changed, 187 insertions(+), 142 deletions(-) diff --git a/packages/fxa-settings/src/components/Settings/DataCollection/en.ftl b/packages/fxa-settings/src/components/Settings/DataCollection/en.ftl index 9b5325290b..0e94173f56 100644 --- a/packages/fxa-settings/src/components/Settings/DataCollection/en.ftl +++ b/packages/fxa-settings/src/components/Settings/DataCollection/en.ftl @@ -1,8 +1,10 @@ ## Data collection section dc-heading = Data Collection and Use -dc-subheader-2 = Help improve { -product-mozilla-accounts } +dc-subheader-moz-accounts = { -product-mozilla-accounts } +dc-subheader-ff-browser = { -brand-firefox } browser dc-subheader-content-2 = Allow { -product-mozilla-accounts } to send technical and interaction data to { -brand-mozilla }. +dc-subheader-ff-content = To review or update your { -brand-firefox } browser technical and interaction data settings, open { -brand-firefox } settings and navigate to Privacy and Security. dc-opt-out-success-2 = Opt out successful. { -product-mozilla-accounts } won’t send technical or interaction data to { -brand-mozilla }. dc-opt-in-success-2 = Thanks! Sharing this data helps us improve { -product-mozilla-accounts }. dc-opt-in-out-error-2 = Sorry, there was a problem changing your data collection preference diff --git a/packages/fxa-settings/src/components/Settings/DataCollection/index.test.tsx b/packages/fxa-settings/src/components/Settings/DataCollection/index.test.tsx index 0f1974eba8..bb426dceef 100644 --- a/packages/fxa-settings/src/components/Settings/DataCollection/index.test.tsx +++ b/packages/fxa-settings/src/components/Settings/DataCollection/index.test.tsx @@ -10,7 +10,6 @@ import { mockSettingsContext, renderWithRouter, } from '../../../models/mocks'; -import { renderWithLocalizationProvider } from 'fxa-react/lib/test-utils/localizationProvider'; import { Account, AppContext } from '../../../models'; import { SettingsContext } from '../../../models/contexts/SettingsContext'; @@ -24,19 +23,35 @@ jest.mock('../../../models/AlertBarInfo'); describe('DataCollection', () => { it('renders as expected', () => { - const { container } = renderWithLocalizationProvider(); + renderWithRouter( + + + + + + ); - expect(container).toHaveTextContent('Data Collection and Use'); - expect(container).toHaveTextContent('Help improve Mozilla accounts'); - expect(container).toHaveTextContent( + screen.getByRole('heading', { level: 2, name: 'Data Collection and Use' }); + screen.getByRole('heading', { level: 3, name: 'Mozilla accounts' }); + screen.getByRole('heading', { level: 3, name: 'Firefox browser' }); + screen.getByText( 'Allow Mozilla accounts to send technical and interaction data to Mozilla.' ); + screen.getByText( + 'To review or update your Firefox browser technical and interaction data settings, open Firefox settings and navigate to Privacy and Security.' + ); expect( screen.getByTestId('link-external-telemetry-opt-out') ).toHaveAttribute( 'href', 'https://www.mozilla.org/privacy/mozilla-accounts/' ); + expect( + screen.getByTestId('link-external-firefox-telemetry') + ).toHaveAttribute( + 'href', + 'https://support.mozilla.org/kb/telemetry-clientid' + ); }); it('toggles', async () => { diff --git a/packages/fxa-settings/src/components/Settings/DataCollection/index.tsx b/packages/fxa-settings/src/components/Settings/DataCollection/index.tsx index 701e965dbd..092f9104d0 100644 --- a/packages/fxa-settings/src/components/Settings/DataCollection/index.tsx +++ b/packages/fxa-settings/src/components/Settings/DataCollection/index.tsx @@ -2,22 +2,23 @@ * 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/. */ -import { Localized, useLocalization } from '@fluent/react'; import LinkExternal from 'fxa-react/components/LinkExternal'; import Switch from '../Switch'; import React, { forwardRef, useCallback, useState } from 'react'; -import { useAlertBar } from '../../../models'; +import { useAlertBar, useFtlMsgResolver } from '../../../models'; import { useAccount } from '../../../models'; import { setEnabled } from '../../../lib/metrics'; +import UnitRow from '../UnitRow'; +import { FtlMsg } from 'fxa-react/lib/utils'; export const DataCollection = forwardRef((_, ref) => { const account = useAccount(); const [isSubmitting, setIsSubmitting] = useState(false); const alertBar = useAlertBar(); - const { l10n } = useLocalization(); + const ftlMsgResolver = useFtlMsgResolver(); const localizedHeader = ( - Data Collection and Use + Data Collection and Use ); const handleMetricsOptOutToggle = useCallback(async () => { @@ -27,30 +28,27 @@ export const DataCollection = forwardRef((_, ref) => { await account.metricsOpt(account.metricsEnabled ? 'out' : 'in'); setEnabled(account.metricsEnabled); - const alertArgs: [string, null, string] = account.metricsEnabled + const alertArgs: [string, string] = account.metricsEnabled ? [ 'dc-opt-in-success-2', - null, 'Thanks! Sharing this data helps us improve Mozilla accounts.', ] : [ 'dc-opt-out-success-2', - null, 'Opt out successful. Mozilla accounts won’t send technical or interaction data to Mozilla.', ]; - alertBar.success(l10n.getString.apply(l10n, alertArgs)); + alertBar.success(ftlMsgResolver.getMsg.apply(ftlMsgResolver, alertArgs)); } catch (err) { alertBar.error( - l10n.getString( + ftlMsgResolver.getMsg( 'dc-opt-in-out-error-2', - null, 'Sorry, there was a problem changing your data collection preference' ) ); } finally { setIsSubmitting(false); } - }, [account, alertBar, l10n, setIsSubmitting]); + }, [account, alertBar, ftlMsgResolver, setIsSubmitting]); return (
((_, ref) => { {localizedHeader} -
-
-
- -

- Help improve Mozilla accounts -

- - -

- - Allow Mozilla accounts to send technical and interaction data to - Mozilla. - {' '} - - Learn more - -

-
- -
+
+ ((_, ref) => { localizedLabel: localizedHeader, }} /> -
-
+ } + > +

+ + Allow Mozilla accounts to send technical and interaction data to + Mozilla. + {' '} + + Learn more + +

+ + +
+ + +

+ + To review or update your Firefox browser technical and interaction + data settings, open Firefox settings and navigate to Privacy and + Security. + {' '} + + Learn more + +

+
); diff --git a/packages/fxa-settings/src/components/Settings/Profile/index.tsx b/packages/fxa-settings/src/components/Settings/Profile/index.tsx index 574ded9603..81904512a9 100644 --- a/packages/fxa-settings/src/components/Settings/Profile/index.tsx +++ b/packages/fxa-settings/src/components/Settings/Profile/index.tsx @@ -43,7 +43,7 @@ export const Profile = forwardRef((_, ref) => { diff --git a/packages/fxa-settings/src/components/Settings/Security/index.tsx b/packages/fxa-settings/src/components/Settings/Security/index.tsx index 9509d0f876..5dd7f218f2 100644 --- a/packages/fxa-settings/src/components/Settings/Security/index.tsx +++ b/packages/fxa-settings/src/components/Settings/Security/index.tsx @@ -59,8 +59,8 @@ export const Security = forwardRef((_, ref) => { header="Password" headerId="password" headerValueClassName={hasPassword ? 'tracking-wider' : undefined} - headerValue={hasPassword ? '••••••••••••••••••' : null} - noHeaderValueText={localizedNotSet} + headerValue={hasPassword && '••••••••••••••••••'} + defaultHeaderValueText={localizedNotSet} ctaText={ hasPassword ? undefined diff --git a/packages/fxa-settings/src/components/Settings/UnitRow/index.stories.tsx b/packages/fxa-settings/src/components/Settings/UnitRow/index.stories.tsx index 26558bde97..80e809edea 100644 --- a/packages/fxa-settings/src/components/Settings/UnitRow/index.stories.tsx +++ b/packages/fxa-settings/src/components/Settings/UnitRow/index.stories.tsx @@ -32,27 +32,22 @@ export default { ], } as Meta; -export const NoHeaderValueAndNoCTA = () => ( - +export const NoHeaderValueAndNoCTA = () => ; +export const NoHeaderValueAndNoCTAHideHeader = () => ( + ); export const NoHeaderValueWithDefaultCTA = () => ( - + ); export const NoHeaderValueWithCustomCTA = () => ( - + ); export const NoHeaderValueWithCustomCTAAndReloadButton = () => ( } diff --git a/packages/fxa-settings/src/components/Settings/UnitRow/index.test.tsx b/packages/fxa-settings/src/components/Settings/UnitRow/index.test.tsx index c90bbcdb61..07a396986a 100644 --- a/packages/fxa-settings/src/components/Settings/UnitRow/index.test.tsx +++ b/packages/fxa-settings/src/components/Settings/UnitRow/index.test.tsx @@ -11,7 +11,7 @@ import { SETTINGS_PATH } from '../../../constants'; describe('UnitRow', () => { it('renders as expected with minimal required attributes', () => { - renderWithRouter(); + renderWithRouter(); expect(screen.getByTestId('unit-row-header').textContent).toContain('Foxy'); expect(screen.getByTestId('unit-row-header-value').textContent).toContain( @@ -25,7 +25,7 @@ describe('UnitRow', () => { it('renders the children', () => { renderWithRouter( - +

The children!

 ); @@ -65,33 +65,24 @@ describe('UnitRow', () => { }); it('renders as expected with `revealModal` prop', () => { - renderWithRouter( - {}} - /> - ); + renderWithRouter( {}} />); expect(screen.getByTestId('unit-row-modal').textContent).toContain('Add'); }); it('renders as expected with `hideCtaText` prop', () => { - renderWithRouter( - - ); + renderWithRouter(); const ctaTextElement = screen.queryByTestId('unit-row-route'); expect(ctaTextElement).not.toBeInTheDocument(); }); - it('renders non-default `noHeaderValueText` and `ctaText`', () => { + it('renders non-default `defaultHeaderValueText` and `ctaText`', () => { renderWithRouter( @@ -170,4 +161,14 @@ describe('UnitRow', () => { expect(screen.getByTestId('avatar-nondefault')).toBeInTheDocument(); expect(screen.queryByTestId('avatar-default')).toBeNull(); }); + + it('renders as expected when `hideHeaderValue=true` and no action', () => { + const { container } = renderWithRouter( + + ); + + expect(screen.getByTestId('unit-row-header').textContent).toContain('Foxy'); + expect(screen.queryByTestId('unit-row-header-value')).toBeNull(); + expect(container.querySelector('unit-row-actions')).not.toBeInTheDocument(); + }); }); diff --git a/packages/fxa-settings/src/components/Settings/UnitRow/index.tsx b/packages/fxa-settings/src/components/Settings/UnitRow/index.tsx index ff817910af..6de35c09b2 100644 --- a/packages/fxa-settings/src/components/Settings/UnitRow/index.tsx +++ b/packages/fxa-settings/src/components/Settings/UnitRow/index.tsx @@ -60,8 +60,9 @@ type UnitRowProps = { avatar?: Account['avatar']; header: string; headerId?: string; - headerValue: string | null | boolean; - noHeaderValueText?: string; + headerValue?: string | boolean; + hideHeaderValue?: boolean; + defaultHeaderValueText?: string; ctaText?: string; secondaryCtaText?: string; secondaryCtaRoute?: string; @@ -87,12 +88,13 @@ export const UnitRow = ({ header, headerId, headerValue, + hideHeaderValue = false, route, children, headerContent, actionContent, headerValueClassName, - noHeaderValueText, + defaultHeaderValueText, ctaText, secondaryCtaText, secondaryCtaRoute, @@ -119,8 +121,9 @@ export const UnitRow = ({ 'Change' ); - noHeaderValueText = - noHeaderValueText || l10n.getString('row-defaults-status', null, 'None'); + defaultHeaderValueText = + defaultHeaderValueText || + l10n.getString('row-defaults-status', null, 'None'); secondaryCtaText = secondaryCtaText || l10n.getString('row-defaults-action-disable', null, 'Disable'); @@ -153,75 +156,83 @@ export const UnitRow = ({ {...{ avatar }} /> ) : ( -

- {headerValue || noHeaderValueText} -

+ !hideHeaderValue && ( +

+ {headerValue || defaultHeaderValueText} +

+ ) )} {children} -
-
- {disabled ? ( - - ) : ( - <> - {!hideCtaText && route && ( - - {ctaText} - - )} + {(actionContent || + route || + revealModal || + secondaryCtaRoute || + revealSecondaryModal) && ( +
+
+ {disabled ? ( + + ) : ( + <> + {!hideCtaText && route && ( + + {ctaText} + + )} - {revealModal && ( - - )} + {revealModal && ( + + )} - {secondaryCtaRoute && ( - - {secondaryCtaText} - - )} + {secondaryCtaRoute && ( + + {secondaryCtaText} + + )} - {revealSecondaryModal && ( - - )} - - )} - {actionContent} + {revealSecondaryModal && ( + + )} + + )} + {actionContent} +
-
+ )}
); }; diff --git a/packages/fxa-settings/src/components/Settings/UnitRow/mocks.tsx b/packages/fxa-settings/src/components/Settings/UnitRow/mocks.tsx index 13120e4327..8585c75bf0 100644 --- a/packages/fxa-settings/src/components/Settings/UnitRow/mocks.tsx +++ b/packages/fxa-settings/src/components/Settings/UnitRow/mocks.tsx @@ -24,7 +24,6 @@ export const SubjectWithModal = () => { return ( { header={l10n.getString('se-heading', null, 'Secondary email')} headerId="secondary-email" prefixDataTestId="secondary-email" - headerValue={null} route={`${SETTINGS_PATH}/emails`} ctaOnClickAction={() => GleanMetrics.accountPref.secondaryEmailSubmit()} {...{ diff --git a/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.tsx b/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.tsx index 13d6834ceb..46b22b8fbf 100644 --- a/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.tsx +++ b/packages/fxa-settings/src/components/Settings/UnitRowTwoStepAuth/index.tsx @@ -70,8 +70,11 @@ export const UnitRowTwoStepAuth = () => { hideCtaText: true, } : { - headerValue: null, - noHeaderValueText: l10n.getString('tfa-row-not-set', null, 'Not Set'), + defaultHeaderValueText: l10n.getString( + 'tfa-row-not-set', + null, + 'Not Set' + ), ctaText: l10n.getString('tfa-row-action-add', null, 'Add'), secondaryCtaText: undefined, revealSecondaryModal: undefined,