From 9421ead0f4d127d5c0306dcf1606360d61192bfd Mon Sep 17 00:00:00 2001
From: Riley Tatum <rtatum@google.com>
Date: Wed, 25 Jan 2023 17:57:04 +0000
Subject: [PATCH] [NTP][Panorama] Select current color in chrome colors

Wrapped the currently selected chrome color in a check_mark_wrapper. If it is not classic chrome/no background, nothing should be checked. Anything that isn't a chrome color will have the custom color selector selected.

Also, changed the box-shadow for the wrapper to a border instead. Accomodating for the size of the shadow and shrinking the size of the element, causes the element to move since the shadow doesn't actually take up any space like a border does. This fixes the existing problem of this also happening in Overview colors.

Chrome Colors screenshot: screenshot/9hQZyiSfX3EHgQK
Overview screenshot:  screenshot/3Q8AmZUb46fWkfZ

Bug: 1384250, 1402969
Change-Id: Id6b33dd865095ae37955b3aea013ab94af3bd1d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189570
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Riley Tatum <rtatum@google.com>
Cr-Commit-Position: refs/heads/main@{#1096852}
---
 .../customize_chrome/check_mark_wrapper.html  |  2 +-
 .../customize_chrome/chrome_colors.html       | 19 +++++--
 .../customize_chrome/chrome_colors.ts         | 19 ++++++-
 .../side_panel/customize_chrome/color.html    | 20 ++++----
 .../customize_chrome/chrome_colors_test.ts    | 51 ++++++++++++++++++-
 .../side_panel/customize_chrome/color_test.ts |  4 +-
 6 files changed, 94 insertions(+), 21 deletions(-)

diff --git a/chrome/browser/resources/side_panel/customize_chrome/check_mark_wrapper.html b/chrome/browser/resources/side_panel/customize_chrome/check_mark_wrapper.html
index 0817641c99574..404a4c87017f1 100644
--- a/chrome/browser/resources/side_panel/customize_chrome/check_mark_wrapper.html
+++ b/chrome/browser/resources/side_panel/customize_chrome/check_mark_wrapper.html
@@ -48,7 +48,7 @@
   }
 
   :host([checked]) ::slotted(*) {
-    box-shadow: 0 0 0 2px var(--customize-chrome-check-mark-wrapper-color);
+    border: 2px solid var(--customize-chrome-check-mark-wrapper-color);
   }
 </style>
 <svg id="svg" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 48 48">
diff --git a/chrome/browser/resources/side_panel/customize_chrome/chrome_colors.html b/chrome/browser/resources/side_panel/customize_chrome/chrome_colors.html
index d63a6b0a36e72..7400352d481b2 100644
--- a/chrome/browser/resources/side_panel/customize_chrome/chrome_colors.html
+++ b/chrome/browser/resources/side_panel/customize_chrome/chrome_colors.html
@@ -62,6 +62,9 @@
 
   customize-chrome-color {
     --customize-chrome-color-border-radius: 12px;
+    --customize-chrome-color-check-mark-end: -4px;
+    --customize-chrome-color-check-mark-size: 20px;
+    --customize-chrome-color-check-mark-top: -6px;
     padding: 0;
   }
 </style>
@@ -75,13 +78,15 @@ <h1 id="header">$i18n{chromeColors}</h1>
   <div id="customColorContainer"
       class="tile"
       title="$i18n{colorPickerLabel}"
-      aria-label="$i18n{colorPickerLabel}"
+      aria-label$="$i18n{colorPickerLabel}"
+      aria-checked$="[[isCustomColorSelected_]]"
       tabindex="0"
       on-click="onCustomColorClick_">
     <customize-chrome-color
         id="customColor"
         background-color="[[customColor_.background]]"
-        foreground-color="[[customColor_.foreground]]">
+        foreground-color="[[customColor_.foreground]]"
+        checked="[[isCustomColorSelected_]]">
     </customize-chrome-color>
     <div id="colorPickerIcon"></div>
     <input id="colorPicker" type="color" tabindex="-1" aria-hidden="true"
@@ -91,21 +96,25 @@ <h1 id="header">$i18n{chromeColors}</h1>
       id="defaultColor"
       class="tile"
       title="$i18n{defaultColorName}"
-      aria-label="$i18n{defaultColorName}"
+      aria-label$="$i18n{defaultColorName}"
+      aria-checked$="[[isDefaultColorSelected_]]"
       tabindex="0"
       on-click="onDefaultColorClick_"
       background-color="[[defaultColor_.background]]"
-      foreground-color="[[defaultColor_.foreground]]">
+      foreground-color="[[defaultColor_.foreground]]"
+      checked="[[isDefaultColorSelected_]]">
   </customize-chrome-color>
   <template is="dom-repeat" id="chromeColorsRepeat" items="[[colors_]]">
     <customize-chrome-color
         class="chrome-color tile"
         title="[[item.name]]"
         aria-label$="[[item.name]]"
+        aria-checked$="[[isChromeColorSelected_(item.seed, selectedColor_)]]"
         tabindex="0"
         on-click="onChromeColorClick_"
         background-color="[[item.background]]"
-        foreground-color="[[item.foreground]]">
+        foreground-color="[[item.foreground]]"
+        checked="[[isChromeColorSelected_(item.seed, selectedColor_)]]">
     </customize-chrome-color>
   </template>
 </cr-grid>
diff --git a/chrome/browser/resources/side_panel/customize_chrome/chrome_colors.ts b/chrome/browser/resources/side_panel/customize_chrome/chrome_colors.ts
index 085201c7acc94..176d91e229ad5 100644
--- a/chrome/browser/resources/side_panel/customize_chrome/chrome_colors.ts
+++ b/chrome/browser/resources/side_panel/customize_chrome/chrome_colors.ts
@@ -9,9 +9,11 @@ import './color.js';
 
 import {hexColorToSkColor, skColorToRgba} from 'chrome://resources/js/color_utils.js';
 import {FocusOutlineManager} from 'chrome://resources/js/focus_outline_manager.js';
+import {SkColor} from 'chrome://resources/mojo/skia/public/mojom/skcolor.mojom-webui.js';
 import {DomRepeatEvent, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 import {getTemplate} from './chrome_colors.html.js';
+import {ColorElement} from './color.js';
 import {Color, ColorType, DARK_DEFAULT_COLOR, LIGHT_DEFAULT_COLOR, SelectedColor} from './color_utils.js';
 import {ChromeColor, CustomizeChromePageHandlerInterface, Theme} from './customize_chrome.mojom-webui.js';
 import {CustomizeChromeApiProxy} from './customize_chrome_api_proxy.js';
@@ -21,7 +23,9 @@ export interface ChromeColorsElement {
     backButton: HTMLElement,
     colorPicker: HTMLInputElement,
     colorPickerIcon: HTMLElement,
-    defaultColor: HTMLElement,
+    defaultColor: ColorElement,
+    customColor: ColorElement,
+    customColorContainer: HTMLElement,
   };
 }
 
@@ -46,6 +50,10 @@ export class ChromeColorsElement extends PolymerElement {
         type: Object,
         computed: 'computeSelectedColor_(theme_, colors_)',
       },
+      isDefaultColorSelected_: {
+        type: Object,
+        computed: 'computeIsDefaultColorSelected_(selectedColor_)',
+      },
       isCustomColorSelected_: {
         type: Object,
         computed: 'computeIsCustomColorSelected_(selectedColor_)',
@@ -100,6 +108,10 @@ export class ChromeColorsElement extends PolymerElement {
         this.setThemeListenerId_!);
   }
 
+  private computeIsDefaultColorSelected_(): boolean {
+    return this.selectedColor_.type === ColorType.DEFAULT;
+  }
+
   private computeIsCustomColorSelected_(): boolean {
     return this.selectedColor_.type === ColorType.CUSTOM;
   }
@@ -128,6 +140,11 @@ export class ChromeColorsElement extends PolymerElement {
                                         LIGHT_DEFAULT_COLOR;
   }
 
+  private isChromeColorSelected_(color: SkColor): boolean {
+    return this.selectedColor_.type === ColorType.CHROME &&
+        this.selectedColor_.chromeColor!.value === color.value;
+  }
+
   private onBackClick_() {
     this.dispatchEvent(new Event('back-click'));
   }
diff --git a/chrome/browser/resources/side_panel/customize_chrome/color.html b/chrome/browser/resources/side_panel/customize_chrome/color.html
index e39a2327e5ec5..c538a4a5470be 100644
--- a/chrome/browser/resources/side_panel/customize_chrome/color.html
+++ b/chrome/browser/resources/side_panel/customize_chrome/color.html
@@ -1,6 +1,9 @@
 <style>
   :host {
     --customize-chrome-color-border-radius: 50%;
+    --customize-chrome-color-check-mark-end: -1px;
+    --customize-chrome-color-check-mark-size: 16px;
+    --customize-chrome-color-check-mark-top: 0;
     box-sizing: border-box;
     cursor: pointer;
     display: block;
@@ -14,28 +17,25 @@
   }
 
   svg {
-    border: 1px solid var(--customize-chrome-color-border-color,
-                          var(--customize-chrome-color-foreground-color));
     border-radius: var(--customize-chrome-color-border-radius);
     box-sizing: border-box;
     display: block;
     width: 100%;
   }
 
-  :host([checked]) svg {
-    width: calc(100% - 4px);
-  }
-
-  customize-chrome-check-mark {
-    --customize-chrome-check-mark-wrapper-end: -1px;
+  customize-chrome-check-mark-wrapper {
+    --customize-chrome-check-mark-wrapper-end: var(--customize-chrome-color-check-mark-end);
+    --customize-chrome-check-mark-wrapper-size: var(--customize-chrome-color-check-mark-size);
+    --customize-chrome-check-mark-wrapper-top: var(--customize-chrome-color-check-mark-top);
   }
 
   #background {
     fill: var(--customize-chrome-color-background-color);
   }
 
-  :host([checked]) svg {
-    border: none;
+  :host(:not([checked])) svg {
+    border: 1px solid var(--customize-chrome-color-border-color,
+                          var(--customize-chrome-color-foreground-color));
   }
 
   :host([background-color-hidden]) #background {
diff --git a/chrome/test/data/webui/side_panel/customize_chrome/chrome_colors_test.ts b/chrome/test/data/webui/side_panel/customize_chrome/chrome_colors_test.ts
index 54272e1a3302f..bd20f2ff2115f 100644
--- a/chrome/test/data/webui/side_panel/customize_chrome/chrome_colors_test.ts
+++ b/chrome/test/data/webui/side_panel/customize_chrome/chrome_colors_test.ts
@@ -7,17 +7,19 @@ import 'chrome://customize-chrome-side-panel.top-chrome/chrome_colors.js';
 
 import {ChromeColorsElement} from 'chrome://customize-chrome-side-panel.top-chrome/chrome_colors.js';
 import {ColorElement} from 'chrome://customize-chrome-side-panel.top-chrome/color.js';
-import {ChromeColor, CustomizeChromePageCallbackRouter, CustomizeChromePageHandlerRemote} from 'chrome://customize-chrome-side-panel.top-chrome/customize_chrome.mojom-webui.js';
+import {ChromeColor, CustomizeChromePageCallbackRouter, CustomizeChromePageHandlerRemote, CustomizeChromePageRemote} from 'chrome://customize-chrome-side-panel.top-chrome/customize_chrome.mojom-webui.js';
 import {CustomizeChromeApiProxy} from 'chrome://customize-chrome-side-panel.top-chrome/customize_chrome_api_proxy.js';
 import {assertDeepEquals, assertEquals, assertGE, assertTrue} from 'chrome://webui-test/chai_assert.js';
+import {waitAfterNextRender} from 'chrome://webui-test/polymer_test_util.js';
 import {TestBrowserProxy} from 'chrome://webui-test/test_browser_proxy.js';
 import {eventToPromise} from 'chrome://webui-test/test_util.js';
 
-import {installMock} from './test_support.js';
+import {createTheme, installMock} from './test_support.js';
 
 suite('ChromeColorsTest', () => {
   let chromeColorsElement: ChromeColorsElement;
   let handler: TestBrowserProxy<CustomizeChromePageHandlerRemote>;
+  let callbackRouter: CustomizeChromePageRemote;
 
   setup(async () => {
     document.body.innerHTML = window.trustedTypes!.emptyHTML;
@@ -26,6 +28,8 @@ suite('ChromeColorsTest', () => {
         (mock: CustomizeChromePageHandlerRemote) =>
             CustomizeChromeApiProxy.setInstance(
                 mock, new CustomizeChromePageCallbackRouter()));
+    callbackRouter = CustomizeChromeApiProxy.getInstance()
+                         .callbackRouter.$.bindNewPipeAndPassRemote();
   });
 
   async function setInitialSettings(numColors: number): Promise<void> {
@@ -101,4 +105,47 @@ suite('ChromeColorsTest', () => {
     assertGE(1, args.length);
     assertEquals(0xffff0000, args.at(-1).value);
   });
+
+  test('checks selected color', async () => {
+    await setInitialSettings(2);
+    const theme = createTheme();
+
+    // Set default color.
+    theme.foregroundColor = undefined;
+    callbackRouter.setTheme(theme);
+    await callbackRouter.$.flushForTesting();
+    await waitAfterNextRender(chromeColorsElement);
+
+    // Check default color selected.
+    const defaultColorElement = chromeColorsElement.$.defaultColor;
+    let checkedColors =
+        chromeColorsElement.shadowRoot!.querySelectorAll('[checked]');
+    assertEquals(1, checkedColors.length);
+    assertEquals(defaultColorElement, checkedColors[0]);
+
+    // Set Chrome color.
+    theme.seedColor = {value: 1};
+    theme.foregroundColor = {value: 3};
+    callbackRouter.setTheme(theme);
+    await callbackRouter.$.flushForTesting();
+
+    // Check Chrome color selected.
+    checkedColors =
+        chromeColorsElement.shadowRoot!.querySelectorAll('[checked]');
+    assertEquals(1, checkedColors.length);
+    assertEquals('chrome-color tile', checkedColors[0]!.className);
+    assertEquals(3, (checkedColors[0]! as ColorElement).foregroundColor.value);
+
+    // Set custom color.
+    theme.seedColor = {value: 10};
+    theme.foregroundColor = {value: 5};
+    callbackRouter.setTheme(theme);
+    await callbackRouter.$.flushForTesting();
+
+    // Check custom color selected.
+    checkedColors =
+        chromeColorsElement.shadowRoot!.querySelectorAll('[checked]');
+    assertEquals(1, checkedColors.length);
+    assertEquals(chromeColorsElement.$.customColor, checkedColors[0]);
+  });
 });
diff --git a/chrome/test/data/webui/side_panel/customize_chrome/color_test.ts b/chrome/test/data/webui/side_panel/customize_chrome/color_test.ts
index bf21a9d7a290c..4b194e20aa171 100644
--- a/chrome/test/data/webui/side_panel/customize_chrome/color_test.ts
+++ b/chrome/test/data/webui/side_panel/customize_chrome/color_test.ts
@@ -35,8 +35,8 @@ suite('ColorTest', () => {
         'customize-chrome-check-mark-wrapper')!;
     assertTrue(wrapper.checked);
     const svg = colorElement.shadowRoot!.querySelector('svg')!;
-    assertStyle(svg, 'width', '46px');
-    assertStyle(svg, 'height', '46px');
+    assertStyle(svg, 'width', '50px');
+    assertStyle(svg, 'height', '50px');
     assertStyle(svg, 'border', '0px none rgb(0, 0, 0)');
   });