Commit bc565210 authored by Peter Kasting's avatar Peter Kasting Committed by Commit Bot

Separate checkbox/radio "unchecked" color from button enabled color.

This fixes a regression from http://crrev.com/716332 , which changed the enabled
button foreground color from grey to blue.  That was correct for pushbutton
text, but incorrect for checkbox/radio icons (and, incidentally, download shelf
button ink drops), which were using the same color ID.  To fix this, introduce a
distinct "unchecked button" color ID with the old (correct) color and change to
using it.

Bug: 1026176
Change-Id: I7c2b97431fd52fe85b4d560fe1737e8ff347888f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1925340
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarRobert Liao <robliao@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716941}
parent bd8dcbdf
...@@ -178,6 +178,7 @@ SkColor SkColorFromColorId(ui::NativeTheme::ColorId color_id) { ...@@ -178,6 +178,7 @@ SkColor SkColorFromColorId(ui::NativeTheme::ColorId color_id) {
// Button // Button
case ui::NativeTheme::kColorId_ButtonEnabledColor: case ui::NativeTheme::kColorId_ButtonEnabledColor:
case ui::NativeTheme::kColorId_ButtonUncheckedColor:
return GetFgColor("GtkButton#button.text-button GtkLabel"); return GetFgColor("GtkButton#button.text-button GtkLabel");
case ui::NativeTheme::kColorId_ButtonDisabledColor: case ui::NativeTheme::kColorId_ButtonDisabledColor:
return GetFgColor("GtkButton#button.text-button:disabled GtkLabel"); return GetFgColor("GtkButton#button.text-button:disabled GtkLabel");
......
...@@ -167,8 +167,10 @@ class TransparentButton : public views::Button { ...@@ -167,8 +167,10 @@ class TransparentButton : public views::Button {
// constructor but then it won't be correct after dark mode changes, and to // constructor but then it won't be correct after dark mode changes, and to
// deal with that this class would have to observe NativeTheme and so on. // deal with that this class would have to observe NativeTheme and so on.
SkColor GetInkDropBaseColor() const override { SkColor GetInkDropBaseColor() const override {
// This button will be used like a LabelButton, so use the same foreground
// base color as a label button.
return color_utils::DeriveDefaultIconColor(GetNativeTheme()->GetSystemColor( return color_utils::DeriveDefaultIconColor(GetNativeTheme()->GetSystemColor(
ui::NativeTheme::kColorId_ButtonEnabledColor)); ui::NativeTheme::kColorId_LabelEnabledColor));
} }
const char* GetClassName() const override { return "TransparentButton"; } const char* GetClassName() const override { return "TransparentButton"; }
......
...@@ -45,6 +45,7 @@ enum ColorIds : ColorId { ...@@ -45,6 +45,7 @@ enum ColorIds : ColorId {
kColorButtonProminentDisabledBackground, kColorButtonProminentDisabledBackground,
kColorButtonProminentFocusedBackground, kColorButtonProminentFocusedBackground,
kColorButtonProminentForeground, kColorButtonProminentForeground,
kColorButtonUncheckedForeground,
kColorDialogBackground, kColorDialogBackground,
kColorDialogForeground, kColorDialogForeground,
kColorFocusableBorderFocused, kColorFocusableBorderFocused,
......
...@@ -30,6 +30,7 @@ void AddUiColorMixers(ColorProvider* provider) { ...@@ -30,6 +30,7 @@ void AddUiColorMixers(ColorProvider* provider) {
BlendForMinContrastWithSelf(kColorButtonProminentBackground, 1.3f); BlendForMinContrastWithSelf(kColorButtonProminentBackground, 1.3f);
mixer[kColorButtonProminentForeground] = mixer[kColorButtonProminentForeground] =
GetColorWithMaxContrast(kColorButtonProminentBackground); GetColorWithMaxContrast(kColorButtonProminentBackground);
mixer[kColorButtonUncheckedForeground] = {kColorSecondaryForeground};
mixer[kColorDialogBackground] = {kColorPrimaryBackground}; mixer[kColorDialogBackground] = {kColorPrimaryBackground};
mixer[kColorDialogForeground] = {kColorBodyForeground}; mixer[kColorDialogForeground] = {kColorBodyForeground};
mixer[kColorFocusableBorderFocused] = SetAlpha(kColorAccent, 0x4D); mixer[kColorFocusableBorderFocused] = SetAlpha(kColorAccent, 0x4D);
......
...@@ -26,7 +26,7 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id, ...@@ -26,7 +26,7 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id,
// darker or lighter. // darker or lighter.
if (base_theme->UsesHighContrastColors()) { if (base_theme->UsesHighContrastColors()) {
switch (color_id) { switch (color_id) {
case NativeTheme::kColorId_ButtonEnabledColor: case NativeTheme::kColorId_ButtonUncheckedColor:
case NativeTheme::kColorId_MenuBorderColor: case NativeTheme::kColorId_MenuBorderColor:
case NativeTheme::kColorId_MenuSeparatorColor: case NativeTheme::kColorId_MenuSeparatorColor:
case NativeTheme::kColorId_SeparatorColor: case NativeTheme::kColorId_SeparatorColor:
...@@ -34,6 +34,7 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id, ...@@ -34,6 +34,7 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id,
case NativeTheme::kColorId_TabBottomBorder: case NativeTheme::kColorId_TabBottomBorder:
return color_scheme == NativeTheme::ColorScheme::kDark ? SK_ColorWHITE return color_scheme == NativeTheme::ColorScheme::kDark ? SK_ColorWHITE
: SK_ColorBLACK; : SK_ColorBLACK;
case NativeTheme::kColorId_ButtonEnabledColor:
case NativeTheme::kColorId_FocusedBorderColor: case NativeTheme::kColorId_FocusedBorderColor:
case NativeTheme::kColorId_ProminentButtonColor: case NativeTheme::kColorId_ProminentButtonColor:
return color_scheme == NativeTheme::ColorScheme::kDark return color_scheme == NativeTheme::ColorScheme::kDark
...@@ -65,6 +66,8 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id, ...@@ -65,6 +66,8 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id,
case NativeTheme::kColorId_ButtonEnabledColor: case NativeTheme::kColorId_ButtonEnabledColor:
case NativeTheme::kColorId_ProminentButtonColor: case NativeTheme::kColorId_ProminentButtonColor:
return gfx::kGoogleBlue300; return gfx::kGoogleBlue300;
case NativeTheme::kColorId_ButtonUncheckedColor:
return gfx::kGoogleGrey500;
case NativeTheme::kColorId_TextOnProminentButtonColor: case NativeTheme::kColorId_TextOnProminentButtonColor:
return gfx::kGoogleGrey900; return gfx::kGoogleGrey900;
...@@ -180,6 +183,8 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id, ...@@ -180,6 +183,8 @@ SkColor GetAuraColor(NativeTheme::ColorId color_id,
return SK_ColorWHITE; return SK_ColorWHITE;
case NativeTheme::kColorId_ButtonPressedShade: case NativeTheme::kColorId_ButtonPressedShade:
return SK_ColorTRANSPARENT; return SK_ColorTRANSPARENT;
case NativeTheme::kColorId_ButtonUncheckedColor:
return gfx::kGoogleGrey700;
case NativeTheme::kColorId_ButtonDisabledColor: { case NativeTheme::kColorId_ButtonDisabledColor: {
const SkColor bg = base_theme->GetSystemColor( const SkColor bg = base_theme->GetSystemColor(
NativeTheme::kColorId_DialogBackground, color_scheme); NativeTheme::kColorId_DialogBackground, color_scheme);
......
...@@ -327,6 +327,7 @@ class NATIVE_THEME_EXPORT NativeTheme { ...@@ -327,6 +327,7 @@ class NATIVE_THEME_EXPORT NativeTheme {
kColorId_ButtonEnabledColor, kColorId_ButtonEnabledColor,
kColorId_ButtonDisabledColor, kColorId_ButtonDisabledColor,
kColorId_ButtonPressedShade, kColorId_ButtonPressedShade,
kColorId_ButtonUncheckedColor,
kColorId_ProminentButtonColor, kColorId_ProminentButtonColor,
kColorId_ProminentButtonFocusedColor, kColorId_ProminentButtonFocusedColor,
kColorId_ProminentButtonDisabledColor, kColorId_ProminentButtonDisabledColor,
......
...@@ -586,6 +586,7 @@ SkColor NativeThemeWin::GetSystemColor(ColorId color_id, ...@@ -586,6 +586,7 @@ SkColor NativeThemeWin::GetSystemColor(ColorId color_id,
* overriding haphazardly like this. * overriding haphazardly like this.
case kColorId_ButtonEnabledColor: case kColorId_ButtonEnabledColor:
case kColorId_ButtonUncheckedColor:
return system_colors_[SystemThemeColor::kButtonText]; return system_colors_[SystemThemeColor::kButtonText];
case kColorId_LabelEnabledColor: case kColorId_LabelEnabledColor:
return system_colors_[SystemThemeColor::kButtonText]; return system_colors_[SystemThemeColor::kButtonText];
......
...@@ -185,8 +185,8 @@ const gfx::VectorIcon& Checkbox::GetVectorIcon() const { ...@@ -185,8 +185,8 @@ const gfx::VectorIcon& Checkbox::GetVectorIcon() const {
SkColor Checkbox::GetIconImageColor(int icon_state) const { SkColor Checkbox::GetIconImageColor(int icon_state) const {
const SkColor active_color = GetNativeTheme()->GetSystemColor( const SkColor active_color = GetNativeTheme()->GetSystemColor(
(icon_state & IconState::CHECKED) (icon_state & IconState::CHECKED)
? ui::NativeTheme::kColorId_ProminentButtonColor ? ui::NativeTheme::kColorId_ButtonEnabledColor
: ui::NativeTheme::kColorId_ButtonEnabledColor); : ui::NativeTheme::kColorId_ButtonUncheckedColor);
return (icon_state & IconState::ENABLED) return (icon_state & IconState::ENABLED)
? active_color ? active_color
: color_utils::BlendTowardMaxContrast(active_color, : color_utils::BlendTowardMaxContrast(active_color,
......
...@@ -973,8 +973,8 @@ void MenuItemView::PaintButton(gfx::Canvas* canvas, PaintButtonMode mode) { ...@@ -973,8 +973,8 @@ void MenuItemView::PaintButton(gfx::Canvas* canvas, PaintButtonMode mode) {
const gfx::VectorIcon& radio_icon = const gfx::VectorIcon& radio_icon =
toggled ? kMenuRadioSelectedIcon : kMenuRadioEmptyIcon; toggled ? kMenuRadioSelectedIcon : kMenuRadioEmptyIcon;
const SkColor radio_icon_color = GetNativeTheme()->GetSystemColor( const SkColor radio_icon_color = GetNativeTheme()->GetSystemColor(
toggled ? ui::NativeTheme::kColorId_ProminentButtonColor toggled ? ui::NativeTheme::kColorId_ButtonEnabledColor
: ui::NativeTheme::kColorId_ButtonEnabledColor); : ui::NativeTheme::kColorId_ButtonUncheckedColor);
radio_check_image_view_->SetImage( radio_check_image_view_->SetImage(
gfx::CreateVectorIcon(radio_icon, kMenuCheckSize, radio_icon_color)); gfx::CreateVectorIcon(radio_icon, kMenuCheckSize, radio_icon_color));
} }
......
...@@ -79,6 +79,7 @@ std::unique_ptr<View> CreateAllColorsView() { ...@@ -79,6 +79,7 @@ std::unique_ptr<View> CreateAllColorsView() {
InsertColorRow(layout, COLOR_LABEL_ARGS(kColorId_ButtonEnabledColor)); InsertColorRow(layout, COLOR_LABEL_ARGS(kColorId_ButtonEnabledColor));
InsertColorRow(layout, COLOR_LABEL_ARGS(kColorId_ButtonDisabledColor)); InsertColorRow(layout, COLOR_LABEL_ARGS(kColorId_ButtonDisabledColor));
InsertColorRow(layout, COLOR_LABEL_ARGS(kColorId_ButtonPressedShade)); InsertColorRow(layout, COLOR_LABEL_ARGS(kColorId_ButtonPressedShade));
InsertColorRow(layout, COLOR_LABEL_ARGS(kColorId_ButtonUncheckedColor));
InsertColorRow(layout, COLOR_LABEL_ARGS(kColorId_ProminentButtonColor)); InsertColorRow(layout, COLOR_LABEL_ARGS(kColorId_ProminentButtonColor));
InsertColorRow(layout, InsertColorRow(layout,
COLOR_LABEL_ARGS(kColorId_ProminentButtonFocusedColor)); COLOR_LABEL_ARGS(kColorId_ProminentButtonFocusedColor));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment