Skip to content

Commit

Permalink
[NTP][Panorama] Select current color in chrome colors
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
Riley Tatum authored and Chromium LUCI CQ committed Jan 25, 2023
1 parent 0d3b45e commit 9421ead
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 21 deletions.
Expand Up @@ -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">
Expand Down
Expand Up @@ -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>
Expand All @@ -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"
Expand All @@ -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>
Expand Up @@ -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';
Expand All @@ -21,7 +23,9 @@ export interface ChromeColorsElement {
backButton: HTMLElement,
colorPicker: HTMLInputElement,
colorPickerIcon: HTMLElement,
defaultColor: HTMLElement,
defaultColor: ColorElement,
customColor: ColorElement,
customColorContainer: HTMLElement,
};
}

Expand All @@ -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_)',
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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'));
}
Expand Down
20 changes: 10 additions & 10 deletions 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;
Expand All @@ -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 {
Expand Down
Expand Up @@ -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;
Expand All @@ -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> {
Expand Down Expand Up @@ -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]);
});
});
Expand Up @@ -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)');
});

Expand Down

0 comments on commit 9421ead

Please sign in to comment.