[Settings] Update Sync section to be Account section

Please see the screenshots in the attached bug.

Changes are:
* Started showing the account info even if the sync is not enabled.
* Added a not signed in state for the case when the user is not signed
in. Previously, it was showing "You need to enable sync" warning and
the link was going to "about:blank".
* Updated the warnings to be rendered in a tooltip that's triggered
by the info icon.

Refactorings are:
* Updated "Sync" texts to be "Account" texts.
* Updated Sync category to be Account category for the settings.

Bypass-Check-License: Only updates files, no new files are added.
Fixed: 436201259
Change-Id: Ifee27b1796737b5dae5b5b7864b8379b05f9f18d
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6862745
Commit-Queue: Ergün Erdoğmuş <[email protected]>
Reviewed-by: Simon Zünd <[email protected]>

Author: ergunsh

front_end/core/common/SettingRegistration.ts

@@ -74,10 +74,10 @@ const UIStrings = {
    */
   adorner: 'Adorner',
   /**
-   * @description Header for the "Sync" section in the settings UI. The "Sync"
-   * section allows users to configure which DevTools data is synced via Chrome Sync.
+   * @description Header for the "Account" section in the settings UI. The "Account"
+   * section allows users see their signed in account and configure which DevTools data is synced via Chrome Sync.
    */
-  sync: 'Sync',
+  account: 'Account',
   /**
    * @description Text for the privacy section of the page.
    */
@@ -148,7 +148,7 @@ export const enum SettingCategory {
   MEMORY = 'MEMORY',
   EXTENSIONS = 'EXTENSIONS',
   ADORNER = 'ADORNER',
-  SYNC = 'SYNC',
+  ACCOUNT = 'ACCOUNT',
   PRIVACY = 'PRIVACY',
 }
 
@@ -190,8 +190,8 @@ export function getLocalizedSettingsCategory(category: SettingCategory): Platfor
       return i18nString(UIStrings.adorner);
     case SettingCategory.NONE:
       return i18n.i18n.lockedString('');
-    case SettingCategory.SYNC:
-      return i18nString(UIStrings.sync);
+    case SettingCategory.ACCOUNT:
+      return i18nString(UIStrings.account);
     case SettingCategory.PRIVACY:
       return i18nString(UIStrings.privacy);
   }

front_end/entrypoints/main/main-meta.ts

@@ -781,7 +781,7 @@ function createOptionForLocale(localeString: string): Common.Settings.SettingExt
 }
 
 Common.Settings.registerSettingExtension({
-  category: Common.Settings.SettingCategory.SYNC,
+  category: Common.Settings.SettingCategory.ACCOUNT,
   // This name must be kept in sync with DevToolsSettings::kSyncDevToolsPreferencesFrontendName.
   settingName: 'sync-preferences',
   settingType: Common.Settings.SettingType.BOOLEAN,

front_end/panels/settings/SettingsScreen.ts

@@ -275,7 +275,7 @@ export class GenericSettingsTab extends UI.Widget.VBox implements SettingsTab {
       Common.Settings.SettingCategory.PERSISTENCE,
       Common.Settings.SettingCategory.DEBUGGER,
       Common.Settings.SettingCategory.GLOBAL,
-      Common.Settings.SettingCategory.SYNC,
+      Common.Settings.SettingCategory.ACCOUNT,
     ];
 
     // Some settings define their initial ordering.
@@ -360,9 +360,9 @@ export class GenericSettingsTab extends UI.Widget.VBox implements SettingsTab {
     // Always create the EXTENSIONS section and append the link handling control.
     if (category === Common.Settings.SettingCategory.EXTENSIONS) {
       this.createExtensionSection(settings);
-    } else if (category === Common.Settings.SettingCategory.SYNC && settings.length > 0) {
+    } else if (category === Common.Settings.SettingCategory.ACCOUNT && settings.length > 0) {
       const syncCard = createSettingsCard(
-          Common.SettingRegistration.getLocalizedSettingsCategory(Common.SettingRegistration.SettingCategory.SYNC),
+          Common.SettingRegistration.getLocalizedSettingsCategory(Common.SettingRegistration.SettingCategory.ACCOUNT),
           this.syncSection);
       this.containerElement.appendChild(syncCard);
     } else if (settings.length > 0) {

front_end/panels/settings/components/BUILD.gn

@@ -22,6 +22,7 @@ devtools_module("components") {
     "../../../ui/components/chrome_link:bundle",
     "../../../ui/components/helpers:bundle",
     "../../../ui/components/settings:bundle",
+    "../../../ui/components/tooltips:bundle",
     "../../../ui/lit:bundle",
   ]
 }

front_end/panels/settings/components/SyncSection.test.ts

@@ -20,28 +20,46 @@ async function renderSyncSection(data: PanelComponents.SyncSection.SyncSectionDa
 }
 
 describeWithLocale('SyncSection', () => {
-  it('shows a warning when sync is not active', async () => {
+  it('shows a warning tooltip when sync is not active and the user is signed in', async () => {
     const syncSetting = createFakeSetting<boolean>('setting', true);
-    const {shadowRoot} = await renderSyncSection({syncInfo: {isSyncActive: false}, syncSetting});
-    const warning = shadowRoot.querySelector('.warning');
+    const {shadowRoot} = await renderSyncSection({
+      syncInfo: {
+        isSyncActive: false,
+        accountEmail: '[email protected]',
+      },
+      syncSetting
+    });
+    const warning = shadowRoot.querySelector('devtools-tooltip');
     assert.instanceOf(warning, HTMLElement);
     assert.include(warning.innerText, 'To turn this setting on');
   });
 
-  it('shows a warning when sync is active but preferences bucket is not synced', async () => {
+  it('shows a warning tooltip when sync is active but preferences bucket is not synced', async () => {
     const syncSetting = createFakeSetting<boolean>('setting', true);
-    const {shadowRoot} =
-        await renderSyncSection({syncInfo: {isSyncActive: true, arePreferencesSynced: false}, syncSetting});
+    const {shadowRoot} = await renderSyncSection({
+      syncInfo: {
+        isSyncActive: true,
+        arePreferencesSynced: false,
+        accountEmail: '[email protected]',
+      },
+      syncSetting
+    });
 
-    const warning = shadowRoot.querySelector('.warning');
+    const warning = shadowRoot.querySelector('devtools-tooltip');
     assert.instanceOf(warning, HTMLElement);
 
     assert.include(warning.innerText, 'To turn this setting on');
   });
 
   it('disables the checkbox when sync is not active', async () => {
     const syncSetting = createFakeSetting<boolean>('setting', true);
-    const {shadowRoot} = await renderSyncSection({syncInfo: {isSyncActive: false}, syncSetting});
+    const {shadowRoot} = await renderSyncSection({
+      syncInfo: {
+        isSyncActive: false,
+        accountEmail: '[email protected]',
+      },
+      syncSetting
+    });
 
     const settingCheckbox = shadowRoot.querySelector('setting-checkbox');
     assert.instanceOf(settingCheckbox, SettingComponents.SettingCheckbox.SettingCheckbox);
@@ -53,7 +71,7 @@ describeWithLocale('SyncSection', () => {
     assert.isTrue(checkbox.disabled);
   });
 
-  it('shows the avatar and email of the logged in user when sync is active', async () => {
+  it('shows the avatar and email of the logged in user', async () => {
     const syncSetting = createFakeSetting<boolean>('setting', true);
     const {shadowRoot} = await renderSyncSection({
       syncInfo: {
@@ -73,4 +91,20 @@ describeWithLocale('SyncSection', () => {
 
     assert.include(email.innerText, '[email protected]');
   });
+
+  it('shows not signed in if the user is not logged in', async () => {
+    const syncSetting = createFakeSetting<boolean>('setting', true);
+    const {shadowRoot} = await renderSyncSection({
+      syncInfo: {
+        isSyncActive: false,
+        arePreferencesSynced: false,
+      },
+      syncSetting,
+    });
+
+    const email = shadowRoot.querySelector('.not-signed-in');
+    assert.instanceOf(email, HTMLElement);
+
+    assert.include(email.innerText, 'not signed into Chrome');
+  });
 });

front_end/panels/settings/components/SyncSection.ts

@@ -5,11 +5,13 @@
 
 import '../../../ui/components/chrome_link/chrome_link.js';
 import '../../../ui/components/settings/settings.js';
+import '../../../ui/components/tooltips/tooltips.js';
 
 import type * as Common from '../../../core/common/common.js';
 import type * as Host from '../../../core/host/host.js';
 import * as i18n from '../../../core/i18n/i18n.js';
 import type * as Platform from '../../../core/platform/platform.js';
+import * as Buttons from '../../../ui/components/buttons/buttons.js';
 import * as ComponentHelpers from '../../../ui/components/helpers/helpers.js';
 import * as Lit from '../../../ui/lit/lit.js';
 
@@ -28,16 +30,16 @@ const UIStrings = {
    * to a checkbox that is disabled.
    */
   preferencesSyncDisabled: 'To turn this setting on, you must first enable settings sync in Chrome.',
-  /**
-   * @description Label for a link that take the user to the "Sync" section of the
-   * chrome settings. The link is shown in the DevTools Settings UI.
-   */
-  settings: 'Go to Settings',
   /**
    * @description Label for the account email address. Shown in the DevTools Settings UI in
    * front of the email address currently used for Chrome Sync.
    */
   signedIn: 'Signed into Chrome as:',
+  /**
+   * @description Label for the account settings. Shown in the DevTools Settings UI in
+   * case the user is not logged in to Chrome.
+   */
+  notSignedIn: 'You\'re not signed into Chrome.',
 } as const;
 const str_ = i18n.i18n.registerUIStrings('panels/settings/components/SyncSection.ts', UIStrings);
 const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
@@ -61,7 +63,7 @@ export class SyncSection extends HTMLElement {
 
   #render(): void {
     if (!this.#syncSetting) {
-      throw new Error('SyncSection not properly initialized');
+      throw new Error('SyncSection is not properly initialized');
     }
 
     // TODO: this should not probably happen in render, instead, the setting
@@ -73,41 +75,72 @@ export class SyncSection extends HTMLElement {
     Lit.render(html`
       <style>${syncSectionStyles}</style>
       <fieldset>
-        ${renderAccountInfoOrWarning(this.#syncInfo)}
-        <setting-checkbox .data=${
-            {setting: this.#syncSetting}}>
-        </setting-checkbox>
+        ${renderAccountInfo(this.#syncInfo)}
+        ${renderSettingCheckboxIfNeeded(this.#syncInfo, this.#syncSetting)}
       </fieldset>
     `, this.#shadow, {host: this});
     // clang-format on
   }
 }
 
-/* x-link doesn't work with custom click/keydown handlers */
+function renderSettingCheckboxIfNeeded(
+    syncInfo: Host.InspectorFrontendHostAPI.SyncInformation,
+    syncSetting: Common.Settings.Setting<boolean>): Lit.LitTemplate {
+  if (!syncInfo.accountEmail) {
+    return Lit.nothing;
+  }
 
-function renderAccountInfoOrWarning(syncInfo: Host.InspectorFrontendHostAPI.SyncInformation): Lit.TemplateResult {
-  if (!syncInfo.isSyncActive) {
-    const link = 'chrome://settings/syncSetup' as Platform.DevToolsPath.UrlString;
-    // Disabled until https://crbug.com/1079231 is fixed.
-    // clang-format off
-    return html`
-      <span class="warning">
-        ${i18nString(UIStrings.syncDisabled)}
-        <devtools-chrome-link .href=${link}>${i18nString(UIStrings.settings)}</devtools-chrome-link>
-      </span>`;
-    // clang-format on
+  // clang-format off
+  return html`
+    <div class="setting-checkbox-container">
+      <setting-checkbox class="setting-checkbox" .data=${{setting: syncSetting}}>
+      </setting-checkbox>
+      ${renderWarningIfNeeded(syncInfo)}
+    </div>
+  `;
+  // clang-format on
+}
+
+function renderWarningIfNeeded(syncInfo: Host.InspectorFrontendHostAPI.SyncInformation): Lit.LitTemplate {
+  const hasWarning = !syncInfo.isSyncActive || !syncInfo.arePreferencesSynced;
+  if (!hasWarning) {
+    return Lit.nothing;
   }
-  if (!syncInfo.arePreferencesSynced) {
-    const link = 'chrome://settings/syncSetup/advanced' as Platform.DevToolsPath.UrlString;
-    // Disabled until https://crbug.com/1079231 is fixed.
+
+  const warningLink = !syncInfo.isSyncActive ?
+      'chrome://settings/syncSetup' as Platform.DevToolsPath.UrlString :
+      'chrome://settings/syncSetup/advanced' as Platform.DevToolsPath.UrlString;
+  const warningText =
+      !syncInfo.isSyncActive ? i18nString(UIStrings.syncDisabled) : i18nString(UIStrings.preferencesSyncDisabled);
+  // clang-format off
+  return html`
+    <devtools-chrome-link .href=${warningLink}>
+      <devtools-button
+        aria-describedby=settings-sync-info
+        .iconName=${'info'}
+        .variant=${Buttons.Button.Variant.ICON}
+        .size=${Buttons.Button.Size.SMALL}>
+      </devtools-button>
+    </devtools-chrome-link>
+    <devtools-tooltip
+        id=settings-sync-info
+        variant=simple>
+      ${warningText}
+    </devtools-tooltip>
+  `;
+  // clang-format on
+}
+
+function renderAccountInfo(syncInfo: Host.InspectorFrontendHostAPI.SyncInformation): Lit.LitTemplate {
+  if (!syncInfo.accountEmail) {
     // clang-format off
     return html`
-      <span class="warning">
-        ${i18nString(UIStrings.preferencesSyncDisabled)}
-        <devtools-chrome-link .href=${link}>${i18nString(UIStrings.settings)}</devtools-chrome-link>
-      </span>`;
+      <div class="not-signed-in">${i18nString(UIStrings.notSignedIn)}</div>
+    `;
     // clang-format on
   }
+
+  // clang-format off
   return html`
     <div class="account-info">
       <img src="data:image/png;base64, ${syncInfo.accountImage}" alt="Account avatar" />
@@ -116,6 +149,7 @@ function renderAccountInfoOrWarning(syncInfo: Host.InspectorFrontendHostAPI.Sync
         <span>${syncInfo.accountEmail}</span>
       </div>
     </div>`;
+  // clang-format on
 }
 
 customElements.define('devtools-sync-section', SyncSection);

front_end/panels/settings/components/syncSection.css

@@ -7,13 +7,13 @@
 :host {
   break-inside: avoid;
   display: block;
-  padding-bottom: 9px;
-  width: 288px;
+  width: 100%;
 }
 
 fieldset {
   border: 0;
   padding: 0;
+  padding: 4px 0 0;
 }
 
 .link {
@@ -31,18 +31,27 @@ img {
   width: var(--sys-size-9);
 }
 
-.warning {
-  display: block;
-}
-
 .account-info {
   display: flex;
   align-items: center;
-  margin-top: 12px;
 }
 
 .account-email {
   display: flex;
   flex-direction: column;
   margin-left: 8px;
 }
+
+.not-signed-in {
+  padding-bottom: 4px;
+}
+
+.setting-checkbox-container {
+  display: flex;
+  align-items: center;
+  gap: var(--sys-size-2);
+}
+
+.setting-checkbox {
+  display: inline-block;
+}