Cleanup of MdTextButton

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<SkAlpha>(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<SkAlpha>(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

chromium_src/ui/views/controls/button/md_text_button.h

@@ -7,40 +7,42 @@
 #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
 //  - Differenet hover bg color for prominent background
 //  - 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<const gfx::VectorIcon> icon_ = nullptr;