From 6dd7191be6ff5a5b2a57229697bdafd339a3a428 Mon Sep 17 00:00:00 2001 From: Jay Harris Date: Tue, 29 Nov 2022 09:45:31 +1300 Subject: [PATCH] Cleanup of MdTextButton --- .../views/controls/button/md_text_button.cc | 180 +++++++++--------- .../ui/views/controls/button/md_text_button.h | 42 ++-- ...ws-controls-button-md_text_button.cc.patch | 12 -- 3 files changed, 115 insertions(+), 119 deletions(-) delete mode 100644 patches/ui-views-controls-button-md_text_button.cc.patch diff --git a/chromium_src/ui/views/controls/button/md_text_button.cc b/chromium_src/ui/views/controls/button/md_text_button.cc index d3efb4ec1456..bc18e7a7ff38 100644 --- a/chromium_src/ui/views/controls/button/md_text_button.cc +++ b/chromium_src/ui/views/controls/button/md_text_button.cc @@ -24,11 +24,6 @@ #include "ui/views/controls/highlight_path_generator.h" #include "ui/views/view_class_properties.h" -// To be called from MdTextButtonBase::UpdateColors(). -#define BRAVE_MD_TEXT_BUTTON_UPDATE_COLORS \ - UpdateColorsForBrave(); \ - UpdateIconForBrave(); - #define MdTextButton MdTextButtonBase #include "src/ui/views/controls/button/md_text_button.cc" #undef MdTextButton @@ -207,55 +202,105 @@ void MdTextButton::SetLoading(bool loading) { UpdateColors(); } -void MdTextButton::UpdateBackgroundColor() { - // Handled via |UpdateColorsForBrave|. - if (kind_ != kOld) { +void MdTextButton::UpdateTextColor() { + MdTextButtonBase::UpdateTextColor(); + + // Once we update the buttons across Brave to use the new style, we can remove + // this branch. + if (kind_ == kOld) { + if (GetProminent()) { + return; + } + const ui::NativeTheme* theme = GetNativeTheme(); + // Override different text hover color + if (theme->GetPlatformHighContrastColorScheme() != + ui::NativeTheme::PlatformHighContrastColorScheme::kDark) { + SetTextColor(ButtonState::STATE_HOVERED, kBraveBrandColor); + SetTextColor(ButtonState::STATE_PRESSED, kBraveBrandColor); + } return; } - MdTextButtonBase::UpdateBackgroundColor(); + auto colors = GetButtonColors(); + SetTextColor(GetVisualState(), colors.text_color); } -void MdTextButton::UpdateOldColorsForBrave() { - if (GetProminent()) { +void MdTextButton::UpdateBackgroundColor() { + // Once we update the buttons across Brave to use the new style, we can remove + // this branch. + if (kind_ == kOld) { + MdTextButtonBase::UpdateBackgroundColor(); + + // We don't modify the Prominent button at all. + if (GetProminent()) { + return; + } + + // Override border color for hover on non-prominent + if (GetState() == ButtonState::STATE_PRESSED || + GetState() == ButtonState::STATE_HOVERED) { + // First, get the same background fill color that MdTextButtonBase does. + // It is unfortunate to copy these lines almost as-is. Consider otherwise + // patching it in via a #define. + SkColor bg_color = + GetColorProvider()->GetColor(ui::kColorDialogBackground); + if (GetBgColorOverride()) { + bg_color = *GetBgColorOverride(); + } + if (GetState() == STATE_PRESSED) { + bg_color = GetNativeTheme()->GetSystemButtonPressedColor(bg_color); + } + // The only thing that differs for Brave is the stroke color + SkColor stroke_color = kBraveBrandColor; + SetBackground(CreateBackgroundFromPainter( + Painter::CreateRoundRectWith1PxBorderPainter(bg_color, stroke_color, + GetCornerRadius()))); + } return; } - const ui::NativeTheme* theme = GetNativeTheme(); - // Override different text hover color - if (theme->GetPlatformHighContrastColorScheme() != - ui::NativeTheme::PlatformHighContrastColorScheme::kDark) { - SetTextColor(ButtonState::STATE_HOVERED, kBraveBrandColor); - SetTextColor(ButtonState::STATE_PRESSED, kBraveBrandColor); - } - // Override border color for hover on non-prominent - if (GetState() == ButtonState::STATE_PRESSED || - GetState() == ButtonState::STATE_HOVERED) { - // First, get the same background fill color that MdTextButtonBase does. - // It is undfortunate to copy these lines almost as-is. Consider otherwise - // patching it in via a #define. - SkColor bg_color = GetColorProvider()->GetColor(ui::kColorDialogBackground); - if (GetBgColorOverride()) { - bg_color = *GetBgColorOverride(); - } - if (GetState() == STATE_PRESSED) { - bg_color = GetNativeTheme()->GetSystemButtonPressedColor(bg_color); - } - // The only thing that differs for Brave is the stroke color - SkColor stroke_color = kBraveBrandColor; - SetBackground(CreateBackgroundFromPainter( - Painter::CreateRoundRectWith1PxBorderPainter(bg_color, stroke_color, - GetCornerRadius()))); + auto colors = GetButtonColors(); + + // SubPixelRendering doesn't work if we have any background opacity. + SetTextSubpixelRenderingEnabled(SkColorGetA(colors.background_color) == 0xFF); + + SetBackground( + CreateBackgroundFromPainter(Painter::CreateRoundRectWith1PxBorderPainter( + colors.background_color, colors.stroke_color, GetCornerRadius()))); +} + +void MdTextButton::UpdateColors() { + MdTextButtonBase::UpdateColors(); + + // Update the icon color. + if (icon_) { + SetImageModel( + ButtonState::STATE_NORMAL, + ui::ImageModel::FromVectorIcon(*icon_, GetCurrentTextColor())); } } -// To be called from MdTextButtonBase::UpdateColors(). -void MdTextButton::UpdateColorsForBrave() { - if (GetKind() == Kind::kOld) { - UpdateOldColorsForBrave(); - return; +void MdTextButton::OnPaintBackground(gfx::Canvas* canvas) { + // Set brave-style hover colors + MdTextButtonBase::OnPaintBackground(canvas); + if (GetProminent() && + (hover_animation().is_animating() || GetState() == STATE_HOVERED)) { + constexpr SkColor normal_color = kBraveBrandColor; + constexpr SkColor hover_color = SkColorSetRGB(0xff, 0x97, 0x7d); + const SkAlpha alpha = + static_cast(hover_animation().CurrentValueBetween(0x00, 0xff)); + const SkColor current_color = + color_utils::AlphaBlend(hover_color, normal_color, alpha); + cc::PaintFlags flags; + flags.setColor(current_color); + flags.setStyle(cc::PaintFlags::kFill_Style); + flags.setAntiAlias(true); + canvas->DrawRoundRect(gfx::RectF(GetLocalBounds()), GetCornerRadius(), + flags); } +} +MdTextButton::ButtonColors MdTextButton::GetButtonColors() { // Leo buttons only have a light and dark mode. auto color_scheme = GetNativeTheme()->GetPreferredColorScheme() == ColorScheme::kDark @@ -277,7 +322,7 @@ void MdTextButton::UpdateColorsForBrave() { // The enabled style is the normal button style with more opacity. if (!GetEnabled() || state == STATE_DISABLED) { - state = ButtonState::STATE_DISABLED; + state = ButtonState::STATE_NORMAL; opacity = kDisabledOpacity; } @@ -290,50 +335,13 @@ void MdTextButton::UpdateColorsForBrave() { << ", ButtonState: " << state; } const auto& style = it->second; - - SetTextColor(GetVisualState(), AddOpacity(style.text_color, opacity)); - - // Prefer the BgColorOverride, if there is one. Fallback to what's in our - // style. - SkColor bg_color = GetBgColorOverride().value_or( - style.background_color.value_or(SK_ColorTRANSPARENT)); - SkColor stroke_color = style.border_color.value_or(SK_ColorTRANSPARENT); - - // SubPixelRendering doesn't work if we have any background opacity. - SetTextSubpixelRenderingEnabled(opacity == 1 && - SkColorGetA(bg_color) == 0xFF); - SetBackground( - CreateBackgroundFromPainter(Painter::CreateRoundRectWith1PxBorderPainter( - AddOpacity(bg_color, opacity), AddOpacity(stroke_color, opacity), - GetCornerRadius()))); -} - -void MdTextButton::UpdateIconForBrave() { - if (icon_) { - SetImageModel( - ButtonState::STATE_NORMAL, - ui::ImageModel::FromVectorIcon(*icon_, GetCurrentTextColor())); - } -} - -void MdTextButton::OnPaintBackground(gfx::Canvas* canvas) { - // Set brave-style hover colors - MdTextButtonBase::OnPaintBackground(canvas); - if (GetProminent() && - (hover_animation().is_animating() || GetState() == STATE_HOVERED)) { - constexpr SkColor normal_color = kBraveBrandColor; - constexpr SkColor hover_color = SkColorSetRGB(0xff, 0x97, 0x7d); - const SkAlpha alpha = - static_cast(hover_animation().CurrentValueBetween(0x00, 0xff)); - const SkColor current_color = - color_utils::AlphaBlend(hover_color, normal_color, alpha); - cc::PaintFlags flags; - flags.setColor(current_color); - flags.setStyle(cc::PaintFlags::kFill_Style); - flags.setAntiAlias(true); - canvas->DrawRoundRect(gfx::RectF(GetLocalBounds()), GetCornerRadius(), - flags); - } + return {.background_color = AddOpacity( + GetBgColorOverride().value_or( + style.background_color.value_or(SK_ColorTRANSPARENT)), + opacity), + .stroke_color = AddOpacity( + style.border_color.value_or(SK_ColorTRANSPARENT), opacity), + .text_color = AddOpacity(style.text_color, opacity)}; } } // namespace views diff --git a/chromium_src/ui/views/controls/button/md_text_button.h b/chromium_src/ui/views/controls/button/md_text_button.h index fdb44fd1e5a0..db91c7d98df0 100644 --- a/chromium_src/ui/views/controls/button/md_text_button.h +++ b/chromium_src/ui/views/controls/button/md_text_button.h @@ -7,33 +7,29 @@ #define BRAVE_CHROMIUM_SRC_UI_VIEWS_CONTROLS_BUTTON_MD_TEXT_BUTTON_H_ #include "third_party/abseil-cpp/absl/types/optional.h" +#include "third_party/skia/include/core/SkColor.h" #include "ui/gfx/vector_icon_types.h" #include "ui/views/controls/button/label_button.h" // Rename MdTextButton to MdTextButtonBase #define MdTextButton MdTextButtonBase -// Define a Brave-specific method to be called from UpdateColors() to extend its -// functionality instead of defining UpdateColors() "virtual" and overriding it -// in our version of the MdTextButton class because there are some subclasses -// that define their own UpdateColors() method (OmniboxChipButton) now, which -// would not work with the virtual + override approach. -// Note: We redefine UpdateBackgroundColor because we want it to be protected. -#define UpdateBackgroundColor \ - UpdateBackgroundColor_Unused(); \ - \ - protected: \ - virtual void UpdateColorsForBrave() = 0; \ - virtual void UpdateIconForBrave() = 0; \ - void UpdateBackgroundColor +// Redefine UpdateTextColor as protected virtual so we can override it. +#define UpdateTextColor \ + UpdateTextColor_Unused(); \ + \ + protected: \ + virtual void UpdateTextColor + +#define UpdateColors virtual UpdateColors #include "src/ui/views/controls/button/md_text_button.h" // IWYU pragma: export -#undef UpdateBackgroundColor +#undef UpdateColors +#undef UpdateTextColor #undef MdTextButton namespace views { - // Make visual changes to MdTextButton in line with Brave visual style: // - More rounded rectangle (for regular border, focus ring and ink drop) // - Different hover text and boder color for non-prominent button @@ -41,6 +37,12 @@ namespace views { // - No shadow for prominent background class VIEWS_EXPORT MdTextButton : public MdTextButtonBase { public: + struct ButtonColors { + SkColor background_color; + SkColor stroke_color; + SkColor text_color; + }; + enum Kind { kOld, kPrimary, kSecondary, kTertiary }; explicit MdTextButton(PressedCallback callback = PressedCallback(), @@ -60,20 +62,18 @@ class VIEWS_EXPORT MdTextButton : public MdTextButtonBase { bool GetLoading() const; void SetLoading(bool loading); - // Until we decide to update the whole UI to use the new Leo colors, we - // need to keep this logic around. Currently the new colors are opt-in only. - void UpdateOldColorsForBrave(); - // MdTextButtonBase: + void UpdateTextColor() override; void UpdateBackgroundColor() override; - void UpdateColorsForBrave() override; - void UpdateIconForBrave() override; + void UpdateColors() override; protected: // views::Views void OnPaintBackground(gfx::Canvas* canvas) override; private: + ButtonColors GetButtonColors(); + Kind kind_ = kOld; bool loading_ = false; raw_ptr icon_ = nullptr; diff --git a/patches/ui-views-controls-button-md_text_button.cc.patch b/patches/ui-views-controls-button-md_text_button.cc.patch deleted file mode 100644 index 5dee608c2e59..000000000000 --- a/patches/ui-views-controls-button-md_text_button.cc.patch +++ /dev/null @@ -1,12 +0,0 @@ -diff --git a/ui/views/controls/button/md_text_button.cc b/ui/views/controls/button/md_text_button.cc -index f4d8b90bf65ed999bbff54eee293823247585fd5..5a837245cf7759d91e4144d74bf8ce78579b4837 100644 ---- a/ui/views/controls/button/md_text_button.cc -+++ b/ui/views/controls/button/md_text_button.cc -@@ -280,6 +280,7 @@ void MdTextButton::UpdateColors() { - UpdateTextColor(); - UpdateBackgroundColor(); - SchedulePaint(); -+ BRAVE_MD_TEXT_BUTTON_UPDATE_COLORS - } - } -