Privacy Review: move header element into card fragments.

This patch moves the header div from privacy_review_page.html to a duplicated header div for each card fragment, which should be a visual no-op. This is required so that the header will animate along with the card contents, once animations are added. Bug: 1215630 Change-Id: I13e46641f19f519792830392cbbe7a27a2347901 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3320398 Reviewed-by: Rainhard Findling <rainhard@chromium.org> Commit-Queue: Bret Sepulveda <bsep@chromium.org> Cr-Commit-Position: refs/heads/main@{#949988}
chromium · Dec 9, 2021 · 4a3fec8 · 4a3fec8
1 parent d33edbc
commit 4a3fec8
Show file tree

Hide file tree

Showing 9 changed files with 61 additions and 91 deletions.
diff --git a/...resources/settings/privacy_page/privacy_review/privacy_review_clear_on_exit_fragment.html b/...resources/settings/privacy_page/privacy_review/privacy_review_clear_on_exit_fragment.html
@@ -1,4 +1,13 @@
 <style include="privacy-review-fragment-shared"></style>
+<div class="header">
+  <picture>
+    <source
+        srcset="./images/privacy_review/clear_on_exit_graphic_dark.svg"
+        media="(prefers-color-scheme: dark)">
+    <img alt="" src="./images/privacy_review/clear_on_exit_graphic.svg">
+  </picture>
+  <div class="header-label">$i18n{privacyReviewClearOnExitCardHeader}</div>
+</div>
 <div class="embedded-setting-wrapper">
   <settings-toggle-button id="clearOnExit"
       pref="{{prefs.generated.cookie_session_only}}"

diff --git a/...owser/resources/settings/privacy_page/privacy_review/privacy_review_cookies_fragment.html b/...owser/resources/settings/privacy_page/privacy_review/privacy_review_cookies_fragment.html
@@ -1,4 +1,13 @@
 <style include="privacy-review-fragment-shared"></style>
+<div class="header">
+  <picture>
+    <source
+        srcset="./images/privacy_review/cookies_graphic_dark.svg"
+        media="(prefers-color-scheme: dark)">
+    <img alt="" src="./images/privacy_review/cookies_graphic.svg">
+  </picture>
+  <div class="header-label">$i18n{privacyReviewCookiesCardHeader}</div>
+</div>
 <settings-radio-group id="cookiesRadioGroup"
     pref="{{prefs.generated.cookie_primary_setting}}"
     selectable-elements="settings-collapse-radio-button">

diff --git a/...er/resources/settings/privacy_page/privacy_review/privacy_review_fragment_shared_css.html b/...er/resources/settings/privacy_page/privacy_review/privacy_review_fragment_shared_css.html
@@ -37,6 +37,21 @@
       --settings-collapse-separator-line: var(--cr-separator-line);
     }
 
+    .header {
+      align-items: center;
+      column-gap: 16px;
+      display: flex;
+      flex-direction: row;
+      flex-wrap: nowrap;
+      justify-content: space-between;
+      padding: 24px 0 16px 0;
+    }
+
+    .header-label {
+      flex: 1;
+      font-size: 123%;  /* Should be 16px when 100% is 13px. */
+    }
+
     .headline {
       color: var(--google-grey-800);
       font-size: 22px;

diff --git a/.../resources/settings/privacy_page/privacy_review/privacy_review_history_sync_fragment.html b/.../resources/settings/privacy_page/privacy_review/privacy_review_history_sync_fragment.html
@@ -1,4 +1,13 @@
 <style include="privacy-review-fragment-shared"></style>
+<div class="header">
+  <picture>
+    <source
+        srcset="./images/privacy_review/history_sync_graphic_dark.svg"
+        media="(prefers-color-scheme: dark)">
+    <img alt="" src="./images/privacy_review/history_sync_graphic.svg">
+  </picture>
+  <div class="header-label">$i18n{privacyReviewHistorySyncCardHeader}</div>
+</div>
 <div class="embedded-setting-wrapper">
   <settings-toggle-button id="historyToggle"
       pref="{{historySyncVirtualPref_}}"

diff --git a/.../browser/resources/settings/privacy_page/privacy_review/privacy_review_msbb_fragment.html b/.../browser/resources/settings/privacy_page/privacy_review/privacy_review_msbb_fragment.html
@@ -1,4 +1,13 @@
 <style include="privacy-review-fragment-shared"></style>
+<div class="header">
+  <picture>
+    <source
+        srcset="./images/privacy_review/msbb_graphic_dark.svg"
+        media="(prefers-color-scheme: dark)">
+    <img alt="" src="./images/privacy_review/msbb_graphic.svg">
+  </picture>
+  <div class="header-label">$i18n{privacyReviewMsbbCardHeader}</div>
+</div>
 <div class="embedded-setting-wrapper">
   <settings-toggle-button id="urlCollectionToggle"
       pref="{{prefs.url_keyed_anonymized_data_collection.enabled}}"

diff --git a/chrome/browser/resources/settings/privacy_page/privacy_review/privacy_review_page.html b/chrome/browser/resources/settings/privacy_page/privacy_review/privacy_review_page.html
@@ -1,19 +1,4 @@
 <style include="cr-shared-style settings-shared">
-  #header {
-    align-items: center;
-    column-gap: 16px;
-    display: flex;
-    flex-direction: row;
-    flex-wrap: nowrap;
-    justify-content: space-between;
-    padding: 24px 24px 16px 24px;
-  }
-
-  #headerLabel {
-    flex: 1;
-    font-size: 123%;  /* Should be 16px when 100% is 13px. */
-  }
-
   .footer {
     align-items: center;
     display: flex;
@@ -40,15 +25,6 @@
   }
 </style>
 <div class="privacy-review-card">
-  <div id="header" hidden="[[hideHeader_]]">
-    <picture>
-      <source
-          srcset="[[headerModel_.darkImage]]"
-          media="(prefers-color-scheme: dark)">
-      <img id="headerImage" alt="" src="[[headerModel_.lightImage]]">
-    </picture>
-    <div id="headerLabel">[[headerModel_.title]]</div>
-  </div>
   <template is="dom-if"
       if="[[showFragment_(
              privacyReviewStepEnum_.WELCOME, privacyReviewStep_)]]">

diff --git a/chrome/browser/resources/settings/privacy_page/privacy_review/privacy_review_page.ts b/chrome/browser/resources/settings/privacy_page/privacy_review/privacy_review_page.ts
@@ -36,14 +36,7 @@ import {CookiePrimarySetting} from '../../site_settings/site_settings_prefs_brow
 import {PrivacyReviewStep} from './constants.js';
 import {StepIndicatorModel} from './step_indicator.js';
 
-type HeaderModel = {
-  title: string,
-  lightImage: string,
-  darkImage: string,
-};
-
 interface PrivacyReviewStepComponents {
-  headerModel?: HeaderModel;
   onForwardNavigation(): void;
   onBackNavigation?(): void;
   isAvailable(): boolean;
@@ -88,7 +81,6 @@ export class SettingsPrivacyReviewPageElement extends PrivacyReviewBase {
       privacyReviewStep_: {
         type: String,
         value: PrivacyReviewStep.WELCOME,
-        observer: 'onPrivacyReviewStepChanged_',
       },
 
       /**
@@ -115,8 +107,6 @@ export class SettingsPrivacyReviewPageElement extends PrivacyReviewBase {
   private syncBrowserProxy_: SyncBrowserProxy =
       SyncBrowserProxyImpl.getInstance();
   private syncStatus_: SyncStatus;
-  private hideHeader_: boolean;
-  private headerModel_?: HeaderModel;
 
   constructor() {
     super();
@@ -175,11 +165,6 @@ export class SettingsPrivacyReviewPageElement extends PrivacyReviewBase {
       [
         PrivacyReviewStep.MSBB,
         {
-          headerModel: {
-            title: this.i18n('privacyReviewMsbbCardHeader'),
-            lightImage: './images/privacy_review/msbb_graphic.svg',
-            darkImage: './images/privacy_review/msbb_graphic_dark.svg',
-          },
           onForwardNavigation: () => {
             this.navigateToCard_(PrivacyReviewStep.CLEAR_ON_EXIT);
           },
@@ -189,11 +174,6 @@ export class SettingsPrivacyReviewPageElement extends PrivacyReviewBase {
       [
         PrivacyReviewStep.CLEAR_ON_EXIT,
         {
-          headerModel: {
-            title: this.i18n('privacyReviewClearOnExitCardHeader'),
-            lightImage: './images/privacy_review/clear_on_exit_graphic.svg',
-            darkImage: './images/privacy_review/clear_on_exit_graphic_dark.svg',
-          },
           onForwardNavigation: () => {
             this.navigateToCard_(PrivacyReviewStep.HISTORY_SYNC);
           },
@@ -207,11 +187,6 @@ export class SettingsPrivacyReviewPageElement extends PrivacyReviewBase {
       [
         PrivacyReviewStep.HISTORY_SYNC,
         {
-          headerModel: {
-            title: this.i18n('privacyReviewHistorySyncCardHeader'),
-            lightImage: './images/privacy_review/history_sync_graphic.svg',
-            darkImage: './images/privacy_review/history_sync_graphic_dark.svg',
-          },
           onForwardNavigation: () => {
             this.navigateToCard_(PrivacyReviewStep.SAFE_BROWSING);
           },
@@ -224,11 +199,6 @@ export class SettingsPrivacyReviewPageElement extends PrivacyReviewBase {
       [
         PrivacyReviewStep.SAFE_BROWSING,
         {
-          headerModel: {
-            title: this.i18n('privacyReviewSafeBrowsingCardHeader'),
-            lightImage: './images/privacy_review/safe_browsing_graphic.svg',
-            darkImage: './images/privacy_review/safe_browsing_graphic_dark.svg',
-          },
           onForwardNavigation: () => {
             this.navigateToCard_(PrivacyReviewStep.COOKIES);
           },
@@ -241,11 +211,6 @@ export class SettingsPrivacyReviewPageElement extends PrivacyReviewBase {
       [
         PrivacyReviewStep.COOKIES,
         {
-          headerModel: {
-            title: this.i18n('privacyReviewCookiesCardHeader'),
-            lightImage: './images/privacy_review/cookies_graphic.svg',
-            darkImage: './images/privacy_review/cookies_graphic_dark.svg',
-          },
           onForwardNavigation: () => {
             this.navigateToCard_(PrivacyReviewStep.COMPLETION);
             HatsBrowserProxyImpl.getInstance().trustSafetyInteractionOccurred(
@@ -361,12 +326,6 @@ export class SettingsPrivacyReviewPageElement extends PrivacyReviewBase {
     };
   }
 
-  private onPrivacyReviewStepChanged_() {
-    this.headerModel_ = this.privacyReviewStepToComponentsMap_
-                            .get(this.privacyReviewStep_)!.headerModel;
-    this.hideHeader_ = this.headerModel_ === undefined;
-  }
-
   private isSyncOn_(): boolean {
     return !!this.syncStatus_ && !!this.syncStatus_.signedIn &&
         !this.syncStatus_.hasError;

diff --git a/...resources/settings/privacy_page/privacy_review/privacy_review_safe_browsing_fragment.html b/...resources/settings/privacy_page/privacy_review/privacy_review_safe_browsing_fragment.html
@@ -1,4 +1,13 @@
 <style include="privacy-review-fragment-shared"></style>
+<div class="header">
+  <picture>
+    <source
+        srcset="./images/privacy_review/safe_browsing_graphic_dark.svg"
+        media="(prefers-color-scheme: dark)">
+    <img alt="" src="./images/privacy_review/safe_browsing_graphic.svg">
+  </picture>
+  <div class="header-label">$i18n{privacyReviewSafeBrowsingCardHeader}</div>
+</div>
 <settings-radio-group id="safeBrowsingRadioGroup"
     pref="{{prefs.generated.safe_browsing}}"
     selectable-elements="settings-collapse-radio-button">
@@ -76,4 +85,4 @@
         </div>
       </div>
   </settings-collapse-radio-button>
-</settings-radio-group>
+</settings-radio-group>
diff --git a/chrome/test/data/webui/settings/privacy_review_page_test.ts b/chrome/test/data/webui/settings/privacy_review_page_test.ts
@@ -153,8 +153,6 @@ suite('PrivacyReviewPage', function() {
   }
 
   type AssertCardComponentsVisibleParams = {
-    headerTextExpected?: string,
-    headerImgSrcExpected?: string,
     isSettingFooterVisibleExpected?: boolean,
     isBackButtonVisibleExpected?: boolean,
     isWelcomeFragmentVisibleExpected?: boolean,
@@ -167,8 +165,6 @@ suite('PrivacyReviewPage', function() {
   };
 
   function assertCardComponentsVisible({
-    headerTextExpected,
-    headerImgSrcExpected,
     isSettingFooterVisibleExpected,
     isBackButtonVisibleExpected,
     isWelcomeFragmentVisibleExpected,
@@ -179,19 +175,6 @@ suite('PrivacyReviewPage', function() {
     isSafeBrowsingFragmentVisibleExpected,
     isCookiesFragmentVisibleExpected,
   }: AssertCardComponentsVisibleParams) {
-    assertEquals(!!headerTextExpected, isChildVisible(page, '#header'));
-    if (headerTextExpected) {
-      assertEquals(
-          headerTextExpected,
-          page.shadowRoot!.querySelector<HTMLElement>(
-                              '#headerLabel')!.textContent);
-      assertTrue(!!headerImgSrcExpected);
-      assertEquals(
-          'chrome://settings/privacy/images/privacy_review/' +
-              headerImgSrcExpected,
-          page.shadowRoot!.querySelector<HTMLImageElement>(
-                              '#headerImage')!.src);
-    }
     assertEquals(
         !!isSettingFooterVisibleExpected,
         isChildVisible(page, '#settingFooter'));
@@ -267,8 +250,6 @@ suite('PrivacyReviewPage', function() {
   function assertMsbbCardVisible() {
     assertQueryParameter(PrivacyReviewStep.MSBB);
     assertCardComponentsVisible({
-      headerTextExpected: page.i18n('privacyReviewMsbbCardHeader'),
-      headerImgSrcExpected: 'msbb_graphic.svg',
       isSettingFooterVisibleExpected: true,
       isMsbbFragmentVisibleExpected: true,
     });
@@ -278,8 +259,6 @@ suite('PrivacyReviewPage', function() {
   function assertHistorySyncCardVisible() {
     assertQueryParameter(PrivacyReviewStep.HISTORY_SYNC);
     assertCardComponentsVisible({
-      headerTextExpected: page.i18n('privacyReviewHistorySyncCardHeader'),
-      headerImgSrcExpected: 'history_sync_graphic.svg',
       isSettingFooterVisibleExpected: true,
       isBackButtonVisibleExpected: true,
       isHistorySyncFragmentVisibleExpected: true,
@@ -290,8 +269,6 @@ suite('PrivacyReviewPage', function() {
   function assertSafeBrowsingCardVisible() {
     assertQueryParameter(PrivacyReviewStep.SAFE_BROWSING);
     assertCardComponentsVisible({
-      headerTextExpected: page.i18n('privacyReviewSafeBrowsingCardHeader'),
-      headerImgSrcExpected: 'safe_browsing_graphic.svg',
       isSettingFooterVisibleExpected: true,
       isBackButtonVisibleExpected: true,
       isSafeBrowsingFragmentVisibleExpected: true,
@@ -302,8 +279,6 @@ suite('PrivacyReviewPage', function() {
   function assertCookiesCardVisible() {
     assertQueryParameter(PrivacyReviewStep.COOKIES);
     assertCardComponentsVisible({
-      headerTextExpected: page.i18n('privacyReviewCookiesCardHeader'),
-      headerImgSrcExpected: 'cookies_graphic.svg',
       isSettingFooterVisibleExpected: true,
       isBackButtonVisibleExpected: true,
       isCookiesFragmentVisibleExpected: true,