Merge pull request #17457 from mozilla/FXA-10297

feat(settings): Refactor 'Data Collection' for new content + UnitRow adjustments
2024-08-26 10:48:21 -05:00 · 2024-08-26 10:48:21 -05:00 · 56ee0ae82e
--- 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
--- 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(<DataCollection />);
+    renderWithRouter(
+      <AppContext.Provider value={mockAppContext({ account })}>
+        <SettingsContext.Provider value={mockSettingsContext()}>
+          <DataCollection />
+        </SettingsContext.Provider>
+      </AppContext.Provider>
+    );

-    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 () => {
--- 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<HTMLDivElement>((_, ref) => {
  const account = useAccount();
  const [isSubmitting, setIsSubmitting] = useState<boolean>(false);
  const alertBar = useAlertBar();
-  const { l10n } = useLocalization();
+  const ftlMsgResolver = useFtlMsgResolver();

  const localizedHeader = (
-    <Localized id="dc-heading">Data Collection and Use</Localized>
+    <FtlMsg id="dc-heading">Data Collection and Use</FtlMsg>
  );

  const handleMetricsOptOutToggle = useCallback(async () => {
@ -27,30 +28,27 @@ export const DataCollection = forwardRef<HTMLDivElement>((_, 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 (
    <section
@ -63,31 +61,14 @@ export const DataCollection = forwardRef<HTMLDivElement>((_, ref) => {
        <span id="data-collection" className="nav-anchor" />
        {localizedHeader}
      </h2>
-      <div className="bg-white tablet:rounded-xl shadow px-4 tablet:px-6 pt-7 pb-5">
-        <div className="flex mb-4">
-          <div className="flex-5 tablet:flex-7 ltr:pr-6 tablet:ltr:pr-12 rtl:pl-6 tablet:rtl:pl-12">
-            <Localized id="dc-subheader-2">
-              <h3 className="font-header mb-4">
-                Help improve Mozilla accounts
-              </h3>
-            </Localized>
-
-            <p className="text-sm">
-              <Localized id="dc-subheader-content-2">
-                Allow Mozilla accounts to send technical and interaction data to
-                Mozilla.
-              </Localized>{' '}
-              <LinkExternal
-                href="https://www.mozilla.org/privacy/mozilla-accounts/"
-                className="link-blue"
-                data-testid="link-external-telemetry-opt-out"
-              >
-                <Localized id="dc-learn-more">Learn more</Localized>
-              </LinkExternal>
-            </p>
-          </div>
-
-          <div className="flex-1 flex justify-center tablet:justify-end tablet:pr-4 tablet:pt-1">
+      <div className="bg-white tablet:rounded-xl shadow">
+        <UnitRow
+          header={ftlMsgResolver.getMsg(
+            'dc-subheader-moz-accounts',
+            'Mozilla accounts'
+          )}
+          hideHeaderValue
+          actionContent={
            <Switch
              {...{
                isSubmitting,
@ -97,8 +78,47 @@ export const DataCollection = forwardRef<HTMLDivElement>((_, ref) => {
                localizedLabel: localizedHeader,
              }}
            />
-          </div>
-        </div>
+          }
+        >
+          <p className="mb-4">
+            <FtlMsg id="dc-subheader-content-2">
+              Allow Mozilla accounts to send technical and interaction data to
+              Mozilla.
+            </FtlMsg>{' '}
+            <LinkExternal
+              href="https://www.mozilla.org/privacy/mozilla-accounts/"
+              className="link-blue"
+              data-testid="link-external-telemetry-opt-out"
+            >
+              <FtlMsg id="dc-learn-more">Learn more</FtlMsg>
+            </LinkExternal>
+          </p>
+        </UnitRow>
+
+        <hr className="unit-row-hr" />
+
+        <UnitRow
+          header={ftlMsgResolver.getMsg(
+            'dc-subheader-ff-browser',
+            'Firefox browser'
+          )}
+          hideHeaderValue
+        >
+          <p>
+            <FtlMsg id="dc-subheader-ff-content">
+              To review or update your Firefox browser technical and interaction
+              data settings, open Firefox settings and navigate to Privacy and
+              Security.
+            </FtlMsg>{' '}
+            <LinkExternal
+              href="https://support.mozilla.org/kb/telemetry-clientid"
+              className="link-blue"
+              data-testid="link-external-firefox-telemetry"
+            >
+              <FtlMsg id="dc-learn-more">Learn more</FtlMsg>
+            </LinkExternal>
+          </p>
+        </UnitRow>
      </div>
    </section>
  );
--- 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<HTMLDivElement>((_, ref) => {
          <UnitRow
            header="Display name"
            headerId="display-name"
-            headerValue={displayName}
+            headerValue={displayName || undefined}
            headerValueClassName="break-all"
            route="/settings/display_name"
            ctaOnClickAction={() =>
--- 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<HTMLDivElement>((_, ref) => {
            header="Password"
            headerId="password"
            headerValueClassName={hasPassword ? 'tracking-wider' : undefined}
-            headerValue={hasPassword ? '••••••••••••••••••' : null}
-            noHeaderValueText={localizedNotSet}
+            headerValue={hasPassword && '••••••••••••••••••'}
+            defaultHeaderValueText={localizedNotSet}
            ctaText={
              hasPassword
                ? undefined
--- 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 = () => (
-  <UnitRow header={MOCK_HEADER} headerValue={null} />
+export const NoHeaderValueAndNoCTA = () => <UnitRow header={MOCK_HEADER} />;
+export const NoHeaderValueAndNoCTAHideHeader = () => (
+  <UnitRow header={MOCK_HEADER} hideHeaderValue />
 );

 export const NoHeaderValueWithDefaultCTA = () => (
-  <UnitRow header={MOCK_HEADER} headerValue={null} route="#" />
+  <UnitRow header={MOCK_HEADER} route="#" />
 );

 export const NoHeaderValueWithCustomCTA = () => (
-  <UnitRow
-    header={MOCK_HEADER}
-    headerValue={null}
-    ctaText={MOCK_CTA_TEXT}
-    route="#"
-  />
+  <UnitRow header={MOCK_HEADER} ctaText={MOCK_CTA_TEXT} route="#" />
 );

 export const NoHeaderValueWithCustomCTAAndReloadButton = () => (
  <UnitRow
    header={MOCK_HEADER}
-    headerValue={null}
    route="#"
    ctaText={MOCK_CTA_TEXT}
    actionContent={<ButtonIconReload title="Retry" />}
--- 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(<UnitRow header="Foxy" headerValue={null} />);
+    renderWithRouter(<UnitRow header="Foxy" />);

    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(
-      <UnitRow header="Display name" headerValue={null}>
+      <UnitRow header="Display name">
        <p data-testid="children">The children!</p>
      </UnitRow>
    );
@ -65,33 +65,24 @@ describe('UnitRow', () => {
  });

  it('renders as expected with `revealModal` prop', () => {
-    renderWithRouter(
-      <UnitRow
-        header="Display name"
-        headerValue={null}
-        revealModal={() => {}}
-      />
-    );
+    renderWithRouter(<UnitRow header="Display name" revealModal={() => {}} />);

    expect(screen.getByTestId('unit-row-modal').textContent).toContain('Add');
  });

  it('renders as expected with `hideCtaText` prop', () => {
-    renderWithRouter(
-      <UnitRow header="Display name" headerValue={null} hideCtaText={true} />
-    );
+    renderWithRouter(<UnitRow header="Display name" hideCtaText={true} />);

    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(
      <UnitRow
        header="Display name"
-        headerValue={null}
-        noHeaderValueText="Not Set"
+        defaultHeaderValueText="Not Set"
        ctaText="Create"
        route="/display_name"
      />
@ -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(
+      <UnitRow header="Foxy" hideHeaderValue />
+    );
+
+    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();
+  });
 });
--- 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 }}
          />
        ) : (
-          <p
-            className={classNames('font-bold', headerValueClassName)}
-            data-testid={formatDataTestId('unit-row-header-value')}
-          >
-            {headerValue || noHeaderValueText}
-          </p>
+          !hideHeaderValue && (
+            <p
+              className={classNames('font-bold', headerValueClassName)}
+              data-testid={formatDataTestId('unit-row-header-value')}
+            >
+              {headerValue || defaultHeaderValueText}
+            </p>
+          )
        )}
        {children}
      </div>

-      <div className="unit-row-actions">
-        <div className="flex items-center h-8 gap-2">
-          {disabled ? (
-            <button
-              className="cta-neutral cta-base cta-base-p transition-standard me-1"
-              data-testid={formatDataTestId('unit-row-route')}
-              title={disabledReason}
-              disabled={disabled}
-            >
-              {!hideCtaText && ctaText}
-            </button>
-          ) : (
-            <>
-              {!hideCtaText && route && (
-                <Link
-                  className="cta-neutral cta-base cta-base-p transition-standard me-1"
-                  data-testid={formatDataTestId('unit-row-route')}
-                  to={`${route}${location.search}`}
-                  onClick={ctaOnClickAction}
-                >
-                  {ctaText}
-                </Link>
-              )}
+      {(actionContent ||
+        route ||
+        revealModal ||
+        secondaryCtaRoute ||
+        revealSecondaryModal) && (
+        <div className="unit-row-actions">
+          <div className="flex items-center h-8 gap-2">
+            {disabled ? (
+              <button
+                className="cta-neutral cta-base cta-base-p transition-standard me-1"
+                data-testid={formatDataTestId('unit-row-route')}
+                title={disabledReason}
+                disabled={disabled}
+              >
+                {!hideCtaText && ctaText}
+              </button>
+            ) : (
+              <>
+                {!hideCtaText && route && (
+                  <Link
+                    className="cta-neutral cta-base cta-base-p transition-standard me-1"
+                    data-testid={formatDataTestId('unit-row-route')}
+                    to={`${route}${location.search}`}
+                    onClick={ctaOnClickAction}
+                  >
+                    {ctaText}
+                  </Link>
+                )}

-              {revealModal && (
-                <ModalButton
-                  {...{
-                    revealModal,
-                    ctaText,
-                    alertBarRevealed,
-                    prefixDataTestId,
-                  }}
-                />
-              )}
+                {revealModal && (
+                  <ModalButton
+                    {...{
+                      revealModal,
+                      ctaText,
+                      alertBarRevealed,
+                      prefixDataTestId,
+                    }}
+                  />
+                )}

-              {secondaryCtaRoute && (
-                <Link
-                  className="cta-neutral cta-base cta-base-p transition-standard me-1"
-                  data-testid={formatDataTestId('unit-row-route')}
-                  to={`${secondaryCtaRoute}${location.search}`}
-                >
-                  {secondaryCtaText}
-                </Link>
-              )}
+                {secondaryCtaRoute && (
+                  <Link
+                    className="cta-neutral cta-base cta-base-p transition-standard me-1"
+                    data-testid={formatDataTestId('unit-row-route')}
+                    to={`${secondaryCtaRoute}${location.search}`}
+                  >
+                    {secondaryCtaText}
+                  </Link>
+                )}

-              {revealSecondaryModal && (
-                <ModalButton
-                  revealModal={revealSecondaryModal}
-                  ctaText={secondaryCtaText}
-                  className={secondaryButtonClassName}
-                  alertBarRevealed={alertBarRevealed}
-                  prefixDataTestId={secondaryButtonTestId}
-                />
-              )}
-            </>
-          )}
-          {actionContent}
+                {revealSecondaryModal && (
+                  <ModalButton
+                    revealModal={revealSecondaryModal}
+                    ctaText={secondaryCtaText}
+                    className={secondaryButtonClassName}
+                    alertBarRevealed={alertBarRevealed}
+                    prefixDataTestId={secondaryButtonTestId}
+                  />
+                )}
+              </>
+            )}
+            {actionContent}
+          </div>
        </div>
-      </div>
+      )}
    </div>
  );
 };
--- 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 (
    <UnitRow
      header={MOCK_HEADER}
-      headerValue={null}
      {...{
        revealModal,
      }}
--- a/packages/fxa-settings/src/components/Settings/UnitRowSecondaryEmail/index.tsx
+++ b/packages/fxa-settings/src/components/Settings/UnitRowSecondaryEmail/index.tsx
@ -135,7 +135,6 @@ export const UnitRowSecondaryEmail = () => {
        header={l10n.getString('se-heading', null, 'Secondary email')}
        headerId="secondary-email"
        prefixDataTestId="secondary-email"
-        headerValue={null}
        route={`${SETTINGS_PATH}/emails`}
        ctaOnClickAction={() => GleanMetrics.accountPref.secondaryEmailSubmit()}
        {...{
--- 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,