Skip to content

Commit

Permalink
[NTP][Panorama] Add Chrome Colors page and fill it with data
Browse files Browse the repository at this point in the history
This does not include any layout or styling. This CL adds navigation to the chrome colors page and grabs the color data.

The chrome colors themes page shows the full list of kGeneratedColorsInfo, rather than the partial list that shows on the Overview page. Therefore, I renamed the current function to GetPartialChromeColors and made a new function GetChromeColors for use in theme selection.

I originally wanted to reuse the colors component and have it conditionally shown on the themes page, but the code was getting a lot more complicated with that approach. Also, a lot of the functionality of the colors component is not needed in the Chrome Colors collection.

Screenshot: screenshot/7iMkcsqBeN7hQ6F

Bug: 1384250
Change-Id: I9eaeaf80169ff88263c755d40b52e55a1d4d7673
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4177788
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Riley Tatum <rtatum@google.com>
Cr-Commit-Position: refs/heads/main@{#1095773}
  • Loading branch information
Riley Tatum authored and Chromium LUCI CQ committed Jan 23, 2023
1 parent 692cc5e commit 8a30271
Show file tree
Hide file tree
Showing 20 changed files with 266 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ build_webui("build") {
"cards.ts",
"categories.ts",
"check_mark_wrapper.ts",
"chrome_colors.ts",
"color.ts",
"colors.ts",
"hover_button.ts",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,14 @@ <h1 class="header">$i18n{cardsHeader}</h1>
</div>
<customize-chrome-categories on-back-click="onBackClick_"
on-collection-select="onCollectionSelect_" page-name="categories"
id="categoriesPage" on-local-image-upload="onLocalImageUpload_">
id="categoriesPage" on-local-image-upload="onLocalImageUpload_"
on-chrome-colors-select="onChromeColorsSelect_">
</customize-chrome-categories>
<customize-chrome-themes on-back-click="onBackClick_"
page-name="themes" id="themesPage"
selected-collection="[[selectedCollection_]]">
</customize-chrome-themes>
<customize-chrome-chrome-colors on-back-click="onBackClick_"
page-name="chrome-colors" id="chromeColorsPage">
</customize-chrome-chrome-colors>
</iron-pages>
11 changes: 11 additions & 0 deletions chrome/browser/resources/side_panel/customize_chrome/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'chrome://resources/polymer/v3_0/iron-pages/iron-pages.js';
import './appearance.js';
import './cards.js';
import './categories.js';
import './chrome_colors.js';
import './shortcuts.js';
import './themes.js';

Expand All @@ -14,13 +15,15 @@ import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bu
import {getTemplate} from './app.html.js';
import {AppearanceElement} from './appearance.js';
import {CategoriesElement} from './categories.js';
import {ChromeColorsElement} from './chrome_colors.js';
import {BackgroundCollection} from './customize_chrome.mojom-webui.js';
import {ThemesElement} from './themes.js';

export enum CustomizeChromePage {
OVERVIEW = 'overview',
CATEGORIES = 'categories',
THEMES = 'themes',
CHROME_COLORS = 'chrome-colors',
}

export interface AppElement {
Expand All @@ -29,6 +32,7 @@ export interface AppElement {
categoriesPage: CategoriesElement,
themesPage: ThemesElement,
appearanceElement: AppearanceElement,
chromeColorsPage: ChromeColorsElement,
};
}

Expand Down Expand Up @@ -69,6 +73,9 @@ export class AppElement extends PolymerElement {
case CustomizeChromePage.THEMES:
this.page_ = CustomizeChromePage.CATEGORIES;
break;
case CustomizeChromePage.CHROME_COLORS:
this.page_ = CustomizeChromePage.CATEGORIES;
break;
}
}

Expand All @@ -84,6 +91,10 @@ export class AppElement extends PolymerElement {
private onLocalImageUpload_() {
this.page_ = CustomizeChromePage.OVERVIEW;
}

private onChromeColorsSelect_() {
this.page_ = CustomizeChromePage.CHROME_COLORS;
}
}

declare global {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ <h1>$i18n{categoriesHeader}</h1>
</div>
<div class="label">$i18n{uploadImage}</div>
</div>
<div class="tile" tabindex="0" role="button">
<div class="tile" tabindex="0" id="chromeColorsTile"
role="button" on-click="onChromeColorsClick_">
<div class="image-container">
<div class="chrome-color"></div>
<div class="chrome-color"></div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface CategoriesElement {
classicChromeTile: HTMLElement,
uploadImageTile: HTMLElement,
chromeWebStoreTile: HTMLElement,
chromeColorsTile: HTMLElement,
};
}

Expand Down Expand Up @@ -60,6 +61,10 @@ export class CategoriesElement extends PolymerElement {
}
}

private async onChromeColorsClick_() {
this.dispatchEvent(new Event('chrome-colors-select'));
}

private onCollectionClick_(e: DomRepeatEvent<BackgroundCollection>) {
this.dispatchEvent(new CustomEvent<BackgroundCollection>(
'collection-select', {detail: e.model.item}));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<button on-click="onBackClick_" id="backButton"
title="$i18n{backButton}">
Go Back
</button>
<h1>$i18n{chromeColors}</h1>
<template id="chromeColors" is="dom-repeat" items="[[colors_]]">
<p class="color">[[item.name]]</p>
</template>
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import './color.js';

import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {getTemplate} from './chrome_colors.html.js';
import {ChromeColor} from './customize_chrome.mojom-webui.js';
import {CustomizeChromeApiProxy} from './customize_chrome_api_proxy.js';

export interface ChromeColorsElement {
$: {
backButton: HTMLElement,
};
}

export class ChromeColorsElement extends PolymerElement {
static get is() {
return 'customize-chrome-chrome-colors';
}

static get template() {
return getTemplate();
}

static get properties() {
return {
colors_: Array,
};
}

private colors_: ChromeColor[];

constructor() {
super();
CustomizeChromeApiProxy.getInstance().handler.getChromeColors().then(
({colors}) => {
this.colors_ = colors;
});
}

private onBackClick_() {
this.dispatchEvent(new Event('back-click'));
}
}

declare global {
interface HTMLElementTagNameMap {
'customize-chrome-chrome-colors': ChromeColorsElement;
}
}

customElements.define(ChromeColorsElement.is, ChromeColorsElement);
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,9 @@ export class ColorsElement extends PolymerElement {

constructor() {
super();
CustomizeChromeApiProxy.getInstance().handler.getChromeColors().then(
({colors}) => {
CustomizeChromeApiProxy.getInstance()
.handler.getOverviewChromeColors()
.then(({colors}) => {
this.colors_ = colors;
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@ interface CustomizeChromePageHandler {
// Triggers a call to |CustomizeChromePage.SetPageMostVisitedSettings|.
UpdateMostVisitedSettings();

// Returns the chrome colors used in the customize chrome side panel.
// Returns the chrome colors used in the customize chrome side panel overview.
GetOverviewChromeColors() => (array<ChromeColor> colors);

// Returns the chrome colors used in the customize chrome side panel themes.
GetChromeColors() => (array<ChromeColor> colors);

// Returns the collections of background images.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h"
#include "chrome/browser/new_tab_page/chrome_colors/chrome_colors_factory.h"
#include "chrome/browser/new_tab_page/chrome_colors/generated_colors_info.h"
#include "chrome/browser/new_tab_page/chrome_colors/selected_colors_info.h"
#include "chrome/browser/new_tab_page/modules/new_tab_page_modules.h"
#include "chrome/browser/new_tab_page/new_tab_page_util.h"
Expand Down Expand Up @@ -35,6 +36,19 @@
#include "ui/color/color_provider.h"
#include "ui/native_theme/native_theme.h"

namespace {
side_panel::mojom::ChromeColorPtr CreateChromeColor(
chrome_colors::ColorInfo color_info) {
auto theme_colors = GetAutogeneratedThemeColors(color_info.color);
auto color = side_panel::mojom::ChromeColor::New();
color->name = l10n_util::GetStringUTF8(color_info.label_id);
color->seed = color_info.color;
color->background = theme_colors.active_tab_color;
color->foreground = theme_colors.frame_color;
return color;
}
} // namespace

CustomizeChromePageHandler::CustomizeChromePageHandler(
mojo::PendingReceiver<side_panel::mojom::CustomizeChromePageHandler>
pending_page_handler,
Expand Down Expand Up @@ -103,17 +117,20 @@ void CustomizeChromePageHandler::SetSeedColor(SkColor seed_color) {
theme_service_->BuildAutogeneratedThemeFromColor(seed_color);
}

void CustomizeChromePageHandler::GetOverviewChromeColors(
GetOverviewChromeColorsCallback callback) {
std::vector<side_panel::mojom::ChromeColorPtr> colors;
for (const auto& color_info : kCustomizeChromeColors) {
colors.push_back(CreateChromeColor(color_info));
}
std::move(callback).Run(std::move(colors));
}

void CustomizeChromePageHandler::GetChromeColors(
GetChromeColorsCallback callback) {
std::vector<side_panel::mojom::ChromeColorPtr> colors;
for (const auto& color_info : kCustomizeChromeColors) {
auto theme_colors = GetAutogeneratedThemeColors(color_info.color);
auto color = side_panel::mojom::ChromeColor::New();
color->name = l10n_util::GetStringUTF8(color_info.label_id);
color->seed = color_info.color;
color->background = theme_colors.active_tab_color;
color->foreground = theme_colors.frame_color;
colors.push_back(std::move(color));
for (const auto& color_info : chrome_colors::kGeneratedColorsInfo) {
colors.push_back(CreateChromeColor(color_info));
}
std::move(callback).Run(std::move(colors));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ class CustomizeChromePageHandler
// side_panel::mojom::CustomizeChromePageHandler:
void SetDefaultColor() override;
void SetSeedColor(SkColor seed_color) override;
void GetOverviewChromeColors(
GetOverviewChromeColorsCallback callback) override;
void GetChromeColors(GetChromeColorsCallback callback) override;
void SetBackgroundImage(const std::string& attribution_1,
const std::string& attribution_2,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/new_tab_page/chrome_colors/chrome_colors_factory.h"
#include "chrome/browser/new_tab_page/chrome_colors/chrome_colors_service.h"
#include "chrome/browser/new_tab_page/chrome_colors/generated_colors_info.h"
#include "chrome/browser/new_tab_page/chrome_colors/selected_colors_info.h"
#include "chrome/browser/search/background/ntp_background_data.h"
#include "chrome/browser/search/background/ntp_background_service_factory.h"
Expand Down Expand Up @@ -349,7 +350,7 @@ TEST_F(CustomizeChromePageHandlerTest, SetMostVisitedSettings) {
histogram_tester().ExpectTotalCount("NewTabPage.CustomizeShortcutAction", 2);
}

TEST_F(CustomizeChromePageHandlerTest, GetChromeColors) {
TEST_F(CustomizeChromePageHandlerTest, GetOverviewChromeColors) {
std::vector<side_panel::mojom::ChromeColorPtr> colors;
base::MockCallback<CustomizeChromePageHandler::GetChromeColorsCallback>
callback;
Expand All @@ -359,7 +360,7 @@ TEST_F(CustomizeChromePageHandlerTest, GetChromeColors) {
[&colors](std::vector<side_panel::mojom::ChromeColorPtr> colors_arg) {
colors = std::move(colors_arg);
}));
handler().GetChromeColors(callback.Get());
handler().GetOverviewChromeColors(callback.Get());

ASSERT_EQ(kCustomizeChromeColors.size(), colors.size());
for (size_t i = 0; i < kCustomizeChromeColors.size(); i++) {
Expand All @@ -375,6 +376,37 @@ TEST_F(CustomizeChromePageHandlerTest, GetChromeColors) {
}
}

TEST_F(CustomizeChromePageHandlerTest, GetChromeColors) {
std::vector<side_panel::mojom::ChromeColorPtr> colors;
base::MockCallback<CustomizeChromePageHandler::GetChromeColorsCallback>
callback;
EXPECT_CALL(callback, Run(testing::_))
.Times(1)
.WillOnce(testing::Invoke(
[&colors](std::vector<side_panel::mojom::ChromeColorPtr> colors_arg) {
colors = std::move(colors_arg);
}));
handler().GetChromeColors(callback.Get());

auto num_colors = sizeof(chrome_colors::kGeneratedColorsInfo) /
sizeof(chrome_colors::kGeneratedColorsInfo[0]);
ASSERT_EQ(num_colors, colors.size());
for (size_t i = 0; i < num_colors; i++) {
EXPECT_EQ(l10n_util::GetStringUTF8(
chrome_colors::kGeneratedColorsInfo[i].label_id),
colors[i]->name);
EXPECT_EQ(chrome_colors::kGeneratedColorsInfo[i].color, colors[i]->seed);
EXPECT_EQ(GetAutogeneratedThemeColors(
chrome_colors::kGeneratedColorsInfo[i].color)
.active_tab_color,
colors[i]->background);
EXPECT_EQ(GetAutogeneratedThemeColors(
chrome_colors::kGeneratedColorsInfo[i].color)
.frame_color,
colors[i]->foreground);
}
}

enum class ThemeUpdateSource {
kMojo,
kThemeService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ ts_library("build_ts") {
"cards_test.ts",
"categories_test.ts",
"check_mark_wrapper_test.ts",
"chrome_colors_test.ts",
"color_test.ts",
"colors_test.ts",
"colors_focus_test.ts",
Expand Down
18 changes: 17 additions & 1 deletion chrome/test/data/webui/side_panel/customize_chrome/app_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,25 @@ suite('AppTest', () => {
assertTrue(
customizeChromeApp.$.overviewPage.classList.contains('iron-selected'));

// Set page back to categories and go back a page.
// Set page back to categories.
customizeChromeApp.$.appearanceElement.dispatchEvent(
new Event('edit-theme-click'));

// Send event for chrome colors select.
customizeChromeApp.$.categoriesPage.dispatchEvent(
new Event('chrome-colors-select'));
// Current page should now be chrome colors.
assertTrue(customizeChromeApp.$.chromeColorsPage.classList.contains(
'iron-selected'));

// Send event for back click.
customizeChromeApp.$.chromeColorsPage.dispatchEvent(
new Event('back-click'));
// Current page should now be categories.
assertTrue(customizeChromeApp.$.categoriesPage.classList.contains(
'iron-selected'));

// Send event for back click.
customizeChromeApp.$.categoriesPage.dispatchEvent(new Event('back-click'));
// Current page should now be overview.
assertTrue(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ suite('AppearanceTest', () => {
mock, new CustomizeChromePageCallbackRouter()));
callbackRouterRemote = CustomizeChromeApiProxy.getInstance()
.callbackRouter.$.bindNewPipeAndPassRemote();
// Set result for getChromeColors for the mock handler. Otherwise, it
// crashes since colors is a child of appearance and needs a Promise
// from getChromeColors.
handler.setResultFor('getChromeColors', Promise.resolve([]));
// Set result for getOverviewChromeColors for the mock handler. Otherwise,
// it crashes since colors is a child of appearance and needs a Promise from
// getOverviewChromeColors.
handler.setResultFor('getOverviewChromeColors', Promise.resolve({}));
appearanceElement = document.createElement('customize-chrome-appearance');
document.body.appendChild(appearanceElement);
await handler.whenCalled('getChromeColors');
await handler.whenCalled('getOverviewChromeColors');
});

test('appearance edit button creates event', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,12 @@ suite('CategoriesTest', () => {
categoriesElement.$.chromeWebStoreTile.click();
assertEquals(1, handler.getCallCount('openChromeWebStore'));
});

test('clicking chrome colors sends event', async () => {
const eventPromise =
eventToPromise('chrome-colors-select', categoriesElement);
categoriesElement.$.chromeColorsTile.click();
const event = await eventPromise;
assertTrue(!!event);
});
});

0 comments on commit 8a30271

Please sign in to comment.