Skip to content

Commit

Permalink
[Read Anything] Add colors for dropdown colors based on figma mocks.
Browse files Browse the repository at this point in the history
Screenshot once connected with RA: https://screenshot.googleplex.com/8M2ZpSYU9QTuUeZ.png

Bug:1266555

Change-Id: Ie1e73e6430c9eafa913bd3151a9bdf14110231e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4327414
Commit-Queue: Lauren Winston <lwinston@google.com>
Reviewed-by: Abigail Klein <abigailbklein@google.com>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1115975}
  • Loading branch information
Lauren Winston authored and Chromium LUCI CQ committed Mar 10, 2023
1 parent eb9cd70 commit f9bbbfd
Show file tree
Hide file tree
Showing 11 changed files with 50 additions and 14 deletions.
5 changes: 5 additions & 0 deletions chrome/browser/ui/color/chrome_color_id.h
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,11 @@
E_CPONLY(kColorReadAnythingSeparatorDark) \
E_CPONLY(kColorReadAnythingSeparatorLight) \
E_CPONLY(kColorReadAnythingSeparatorYellow) \
E_CPONLY(kColorReadAnythingDropdownBackground) \
E_CPONLY(kColorReadAnythingDropdownBackgroundBlue) \
E_CPONLY(kColorReadAnythingDropdownBackgroundDark) \
E_CPONLY(kColorReadAnythingDropdownBackgroundLight) \
E_CPONLY(kColorReadAnythingDropdownBackgroundYellow) \

#if BUILDFLAG(IS_CHROMEOS)
#define CHROME_PLATFORM_SPECIFIC_COLOR_IDS \
Expand Down
7 changes: 7 additions & 0 deletions chrome/browser/ui/color/chrome_color_mixer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,13 @@ void AddChromeColorMixer(ui::ColorProvider* provider,
mixer[kColorReadAnythingSeparatorYellow] = ui::PickGoogleColor(
kColorReadAnythingForegroundLight, kColorReadAnythingBackgroundYellow,
color_utils::kMinimumVisibleContrastRatio);
mixer[kColorReadAnythingDropdownBackground] = {
dark_mode ? kColorReadAnythingDropdownBackgroundDark
: kColorReadAnythingDropdownBackgroundLight};
mixer[kColorReadAnythingDropdownBackgroundBlue] = {gfx::kGoogleBlue100};
mixer[kColorReadAnythingDropdownBackgroundDark] = {gfx::kGoogleGrey900};
mixer[kColorReadAnythingDropdownBackgroundLight] = {SK_ColorWHITE};
mixer[kColorReadAnythingDropdownBackgroundYellow] = {gfx::kGoogleYellow050};

// Apply high contrast recipes if necessary.
if (!ShouldApplyHighContrastColors(key)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ void ReadAnythingContainerView::OnReadAnythingThemeChanged(
ui::ColorId foreground_color_id,
ui::ColorId background_color_id,
ui::ColorId separator_color_id,
ui::ColorId dropdown_color_id,
read_anything::mojom::LineSpacing line_spacing,
read_anything::mojom::LetterSpacing letter_spacing) {
separator_->SetColorId(separator_color_id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class ReadAnythingContainerView : public views::View,
ui::ColorId foreground_color_id,
ui::ColorId background_color_id,
ui::ColorId separator_color_id,
ui::ColorId dropdown_color_id,
read_anything::mojom::LineSpacing line_spacing,
read_anything::mojom::LetterSpacing letter_spacing) override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ void ReadAnythingModel::Init(const std::string& font_name,
foreground_color_id_ = initial_colors.foreground_color_id;
background_color_id_ = initial_colors.background_color_id;
separator_color_id_ = initial_colors.separator_color_id;
dropdown_color_id_ = initial_colors.dropdown_color_id;

line_spacing_ = line_spacing_model_->GetLineSpacingAt(
line_spacing_model_->GetSelectedIndex().value());
Expand Down Expand Up @@ -113,6 +114,7 @@ void ReadAnythingModel::SetSelectedColorsByIndex(size_t new_index) {
foreground_color_id_ = new_colors.foreground_color_id;
background_color_id_ = new_colors.background_color_id;
separator_color_id_ = new_colors.separator_color_id;
dropdown_color_id_ = new_colors.dropdown_color_id;

NotifyThemeChanged();
}
Expand Down Expand Up @@ -189,9 +191,10 @@ void ReadAnythingModel::IncreaseTextSize() {

void ReadAnythingModel::NotifyThemeChanged() {
for (Observer& obs : observers_) {
obs.OnReadAnythingThemeChanged(
font_name_, font_scale_, foreground_color_id_, background_color_id_,
separator_color_id_, line_spacing_, letter_spacing_);
obs.OnReadAnythingThemeChanged(font_name_, font_scale_,
foreground_color_id_, background_color_id_,
separator_color_id_, dropdown_color_id_,
line_spacing_, letter_spacing_);
}
}

Expand Down Expand Up @@ -283,6 +286,7 @@ ReadAnythingColorsModel::ReadAnythingColorsModel() {
kColorReadAnythingForeground,
kColorReadAnythingBackground,
kColorReadAnythingSeparator,
kColorReadAnythingDropdownBackground,
ReadAnythingColor::kDefault};

ColorInfo kLightColors = {
Expand All @@ -291,6 +295,7 @@ ReadAnythingColorsModel::ReadAnythingColorsModel() {
kColorReadAnythingForegroundLight,
kColorReadAnythingBackgroundLight,
kColorReadAnythingSeparatorLight,
kColorReadAnythingDropdownBackgroundLight,
ReadAnythingColor::kLight};

ColorInfo kDarkColors = {
Expand All @@ -299,6 +304,7 @@ ReadAnythingColorsModel::ReadAnythingColorsModel() {
kColorReadAnythingForegroundDark,
kColorReadAnythingBackgroundDark,
kColorReadAnythingSeparatorDark,
kColorReadAnythingDropdownBackgroundDark,
ReadAnythingColor::kDark};

ColorInfo kYellowColors = {
Expand All @@ -307,6 +313,7 @@ ReadAnythingColorsModel::ReadAnythingColorsModel() {
kColorReadAnythingForegroundYellow,
kColorReadAnythingBackgroundYellow,
kColorReadAnythingSeparatorYellow,
kColorReadAnythingDropdownBackgroundYellow,
ReadAnythingColor::kYellow};

ColorInfo kBlueColors = {
Expand All @@ -315,6 +322,7 @@ ReadAnythingColorsModel::ReadAnythingColorsModel() {
kColorReadAnythingForegroundBlue,
kColorReadAnythingBackgroundBlue,
kColorReadAnythingSeparatorBlue,
kColorReadAnythingDropdownBackgroundBlue,
ReadAnythingColor::kBlue};

colors_choices_.emplace_back(kDefaultColors);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ class ReadAnythingColorsModel : public ReadAnythingMenuModel {
// toolbar.
ui::ColorId separator_color_id;

// The color of the dropdown menu, used for the combobox menu model.
ui::ColorId dropdown_color_id;

// The enum value used to log this theme.
ReadAnythingColorsModel::ReadAnythingColor logging_value;
};
Expand Down Expand Up @@ -217,6 +220,7 @@ class ReadAnythingModel {
ui::ColorId foreground_color_id,
ui::ColorId background_color_id,
ui::ColorId separator_color_id,
ui::ColorId dropdown_color_id,
read_anything::mojom::LineSpacing line_spacing,
read_anything::mojom::LetterSpacing letter_spacing) = 0;
#if BUILDFLAG(ENABLE_SCREEN_AI_SERVICE)
Expand Down Expand Up @@ -281,7 +285,10 @@ class ReadAnythingModel {
std::string font_name_ = string_constants::kReadAnythingDefaultFontName;
ui::ColorId foreground_color_id_ = kColorReadAnythingForeground;
ui::ColorId background_color_id_ = kColorReadAnythingBackground;

// Additional theme colors.
ui::ColorId separator_color_id_ = kColorReadAnythingSeparator;
ui::ColorId dropdown_color_id_ = kColorReadAnythingDropdownBackground;

// A scale multiplier for font size (internal use only, not shown to user).
float font_scale_ = kReadAnythingDefaultFontScale;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class MockReadAnythingModelObserver : public ReadAnythingModel::Observer {
ui::ColorId foreground_color_id,
ui::ColorId background_color_id,
ui::ColorId separator_color_id,
ui::ColorId dropdown_color_id,
read_anything::mojom::LineSpacing line_spacing,
read_anything::mojom::LetterSpacing letter_spacing),
(override));
Expand Down Expand Up @@ -77,13 +78,13 @@ TEST_F(ReadAnythingModelTest, AddingModelObserverNotifiesAllObservers) {
EXPECT_CALL(model_observer_1_, AccessibilityEventReceived(_)).Times(0);
EXPECT_CALL(model_observer_1_, OnActiveAXTreeIDChanged(_, _)).Times(0);
EXPECT_CALL(model_observer_1_,
OnReadAnythingThemeChanged(_, _, _, _, _, _, _))
OnReadAnythingThemeChanged(_, _, _, _, _, _, _, _))
.Times(1);

EXPECT_CALL(model_observer_2_, AccessibilityEventReceived(_)).Times(0);
EXPECT_CALL(model_observer_2_, OnActiveAXTreeIDChanged(_, _)).Times(0);
EXPECT_CALL(model_observer_2_,
OnReadAnythingThemeChanged(_, _, _, _, _, _, _))
OnReadAnythingThemeChanged(_, _, _, _, _, _, _, _))
.Times(1);

model_->AddObserver(&model_observer_2_);
Expand All @@ -96,19 +97,19 @@ TEST_F(ReadAnythingModelTest, RemovedModelObserversDoNotReceiveNotifications) {
EXPECT_CALL(model_observer_1_, AccessibilityEventReceived(_)).Times(0);
EXPECT_CALL(model_observer_1_, OnActiveAXTreeIDChanged(_, _)).Times(0);
EXPECT_CALL(model_observer_1_,
OnReadAnythingThemeChanged(_, _, _, _, _, _, _))
OnReadAnythingThemeChanged(_, _, _, _, _, _, _, _))
.Times(1);

EXPECT_CALL(model_observer_2_, AccessibilityEventReceived(_)).Times(0);
EXPECT_CALL(model_observer_2_, OnActiveAXTreeIDChanged(_, _)).Times(0);
EXPECT_CALL(model_observer_2_,
OnReadAnythingThemeChanged(_, _, _, _, _, _, _))
OnReadAnythingThemeChanged(_, _, _, _, _, _, _, _))
.Times(0);

EXPECT_CALL(model_observer_3_, AccessibilityEventReceived(_)).Times(0);
EXPECT_CALL(model_observer_3_, OnActiveAXTreeIDChanged(_, _)).Times(0);
EXPECT_CALL(model_observer_3_,
OnReadAnythingThemeChanged(_, _, _, _, _, _, _))
OnReadAnythingThemeChanged(_, _, _, _, _, _, _, _))
.Times(1);

model_->RemoveObserver(&model_observer_2_);
Expand All @@ -119,7 +120,7 @@ TEST_F(ReadAnythingModelTest, NotificationsOnSetSelectedFontIndex) {
model_->AddObserver(&model_observer_1_);

EXPECT_CALL(model_observer_1_,
OnReadAnythingThemeChanged(_, _, _, _, _, _, _))
OnReadAnythingThemeChanged(_, _, _, _, _, _, _, _))
.Times(1);

model_->SetSelectedFontByIndex(2);
Expand Down Expand Up @@ -156,7 +157,7 @@ TEST_F(ReadAnythingModelTest, NotificationsOnDecreasedFontSize) {
model_->AddObserver(&model_observer_1_);

EXPECT_CALL(model_observer_1_,
OnReadAnythingThemeChanged(_, _, _, _, _, _, _))
OnReadAnythingThemeChanged(_, _, _, _, _, _, _, _))
.Times(1);

model_->DecreaseTextSize();
Expand All @@ -168,7 +169,7 @@ TEST_F(ReadAnythingModelTest, NotificationsOnIncreasedFontSize) {
model_->AddObserver(&model_observer_1_);

EXPECT_CALL(model_observer_1_,
OnReadAnythingThemeChanged(_, _, _, _, _, _, _))
OnReadAnythingThemeChanged(_, _, _, _, _, _, _, _))
.Times(1);

model_->IncreaseTextSize();
Expand All @@ -180,7 +181,7 @@ TEST_F(ReadAnythingModelTest, NotificationsOnSetSelectedColorsIndex) {
model_->AddObserver(&model_observer_1_);

EXPECT_CALL(model_observer_1_,
OnReadAnythingThemeChanged(_, _, _, _, _, _, _))
OnReadAnythingThemeChanged(_, _, _, _, _, _, _, _))
.Times(1);

model_->SetSelectedColorsByIndex(2);
Expand All @@ -190,7 +191,7 @@ TEST_F(ReadAnythingModelTest, NotificationsOnSetSelectedLineSpacingIndex) {
model_->AddObserver(&model_observer_1_);

EXPECT_CALL(model_observer_1_,
OnReadAnythingThemeChanged(_, _, _, _, _, _, _))
OnReadAnythingThemeChanged(_, _, _, _, _, _, _, _))
.Times(1);

model_->SetSelectedLineSpacingByIndex(2);
Expand All @@ -200,7 +201,7 @@ TEST_F(ReadAnythingModelTest, NotificationsOnSetSelectedLetterSpacingIndex) {
model_->AddObserver(&model_observer_1_);

EXPECT_CALL(model_observer_1_,
OnReadAnythingThemeChanged(_, _, _, _, _, _, _))
OnReadAnythingThemeChanged(_, _, _, _, _, _, _, _))
.Times(1);

model_->SetSelectedLetterSpacingByIndex(2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ void ReadAnythingToolbarView::OnReadAnythingThemeChanged(
ui::ColorId foreground_color_id,
ui::ColorId background_color_id,
ui::ColorId separator_color_id,
ui::ColorId dropdown_color_id,
read_anything::mojom::LineSpacing line_spacing,
read_anything::mojom::LetterSpacing letter_spacing) {
if (!GetColorProvider())
Expand Down Expand Up @@ -188,6 +189,8 @@ void ReadAnythingToolbarView::OnReadAnythingThemeChanged(
letter_spacing_button_->SetIcon(kLetterSpacingIcon, kIconSize,
foreground_skcolor);

// TODO(1266555): Pass the dropdown color to the combobox and menu models.

for (views::Separator* separator : separators_) {
separator->SetColorId(separator_color_id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class ReadAnythingToolbarView : public views::View,
ui::ColorId foreground_color_id,
ui::ColorId background_color_id,
ui::ColorId separator_color_id,
ui::ColorId dropdown_color_id,
read_anything::mojom::LineSpacing line_spacing,
read_anything::mojom::LetterSpacing letter_spacing) override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ void ReadAnythingPageHandler::OnReadAnythingThemeChanged(
ui::ColorId foreground_color_id,
ui::ColorId background_color_id,
ui::ColorId separator_color_id,
ui::ColorId dropdown_color_id,
read_anything::mojom::LineSpacing line_spacing,
read_anything::mojom::LetterSpacing letter_spacing) {
content::WebContents* web_contents = web_ui_->GetWebContents();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class ReadAnythingPageHandler : public read_anything::mojom::PageHandler,
ui::ColorId foreground_color_id,
ui::ColorId background_color_id,
ui::ColorId separator_color_id,
ui::ColorId dropdown_color_id,
read_anything::mojom::LineSpacing line_spacing,
read_anything::mojom::LetterSpacing letter_spacing) override;
#if BUILDFLAG(ENABLE_SCREEN_AI_SERVICE)
Expand Down

0 comments on commit f9bbbfd

Please sign in to comment.