Commit 934024fb authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

[UI] Fix editable combobox arrow color when theme changes

Before this CL: the color of the arrow is determined once
and doesn't adapt to changes in the theme.

After this CL: the arrow color always matches the text color

Here is a screenshot for the dark mode on Linux.

Before:
https://screenshot.googleplex.com/6yK0cmkBQxJ

After:
https://screenshot.googleplex.com/ctW6pGELa1Y

Bug: 1044038
Change-Id: I62a056deb31d4ac87cfa85469fee868210519fc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2232858
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776255}
parent c84fb46e
...@@ -37,7 +37,6 @@ ...@@ -37,7 +37,6 @@
#include "ui/gfx/range/range.h" #include "ui/gfx/range/range.h"
#include "ui/gfx/render_text.h" #include "ui/gfx/render_text.h"
#include "ui/gfx/scoped_canvas.h" #include "ui/gfx/scoped_canvas.h"
#include "ui/native_theme/native_theme.h"
#include "ui/views/animation/flood_fill_ink_drop_ripple.h" #include "ui/views/animation/flood_fill_ink_drop_ripple.h"
#include "ui/views/animation/ink_drop.h" #include "ui/views/animation/ink_drop.h"
#include "ui/views/animation/ink_drop_host_view.h" #include "ui/views/animation/ink_drop_host_view.h"
...@@ -65,8 +64,7 @@ namespace { ...@@ -65,8 +64,7 @@ namespace {
class Arrow : public Button { class Arrow : public Button {
public: public:
Arrow(const SkColor color, ButtonListener* listener) explicit Arrow(ButtonListener* listener) : Button(listener) {
: Button(listener), color_(color) {
// Similar to Combobox's TransparentButton. // Similar to Combobox's TransparentButton.
SetFocusBehavior(FocusBehavior::NEVER); SetFocusBehavior(FocusBehavior::NEVER);
button_controller()->set_notify_action( button_controller()->set_notify_action(
...@@ -93,8 +91,7 @@ class Arrow : public Button { ...@@ -93,8 +91,7 @@ class Arrow : public Button {
std::unique_ptr<InkDropRipple> CreateInkDropRipple() const override { std::unique_ptr<InkDropRipple> CreateInkDropRipple() const override {
return std::make_unique<views::FloodFillInkDropRipple>( return std::make_unique<views::FloodFillInkDropRipple>(
size(), GetInkDropCenterBasedOnLastEvent(), size(), GetInkDropCenterBasedOnLastEvent(),
GetNativeTheme()->GetSystemColor( style::GetColor(*this, style::CONTEXT_TEXTFIELD, style::STYLE_PRIMARY),
ui::NativeTheme::kColorId_LabelEnabledColor),
ink_drop_visible_opacity()); ink_drop_visible_opacity());
} }
...@@ -104,7 +101,11 @@ class Arrow : public Button { ...@@ -104,7 +101,11 @@ class Arrow : public Button {
canvas->ClipRect(GetContentsBounds()); canvas->ClipRect(GetContentsBounds());
gfx::Rect arrow_bounds = GetLocalBounds(); gfx::Rect arrow_bounds = GetLocalBounds();
arrow_bounds.ClampToCenteredSize(ComboboxArrowSize()); arrow_bounds.ClampToCenteredSize(ComboboxArrowSize());
PaintComboboxArrow(color_, arrow_bounds, canvas); // Make sure the arrow use the same color as the text in the combobox.
PaintComboboxArrow(style::GetColor(*this, style::CONTEXT_TEXTFIELD,
GetEnabled() ? style::STYLE_PRIMARY
: style::STYLE_DISABLED),
arrow_bounds, canvas);
} }
void GetAccessibleNodeData(ui::AXNodeData* node_data) override { void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
...@@ -115,8 +116,6 @@ class Arrow : public Button { ...@@ -115,8 +116,6 @@ class Arrow : public Button {
node_data->SetDefaultActionVerb(ax::mojom::DefaultActionVerb::kOpen); node_data->SetDefaultActionVerb(ax::mojom::DefaultActionVerb::kOpen);
} }
const SkColor color_;
DISALLOW_COPY_AND_ASSIGN(Arrow); DISALLOW_COPY_AND_ASSIGN(Arrow);
}; };
...@@ -334,8 +333,7 @@ EditableCombobox::EditableCombobox( ...@@ -334,8 +333,7 @@ EditableCombobox::EditableCombobox(
textfield_->SetExtraInsets(gfx::Insets( textfield_->SetExtraInsets(gfx::Insets(
/*top=*/0, /*left=*/0, /*bottom=*/0, /*top=*/0, /*left=*/0, /*bottom=*/0,
/*right=*/kComboboxArrowContainerWidth - kComboboxArrowPaddingWidth)); /*right=*/kComboboxArrowContainerWidth - kComboboxArrowPaddingWidth));
arrow_ = new Arrow(textfield_->GetTextColor(), this); arrow_ = AddChildView(std::make_unique<Arrow>(this));
AddChildView(arrow_);
} }
SetLayoutManager(std::make_unique<views::FillLayout>()); SetLayoutManager(std::make_unique<views::FillLayout>());
} }
...@@ -401,11 +399,6 @@ void EditableCombobox::Layout() { ...@@ -401,11 +399,6 @@ void EditableCombobox::Layout() {
} }
} }
void EditableCombobox::OnThemeChanged() {
View::OnThemeChanged();
textfield_->OnThemeChanged();
}
void EditableCombobox::GetAccessibleNodeData(ui::AXNodeData* node_data) { void EditableCombobox::GetAccessibleNodeData(ui::AXNodeData* node_data) {
node_data->role = ax::mojom::Role::kComboBoxGrouping; node_data->role = ax::mojom::Role::kComboBoxGrouping;
......
...@@ -124,7 +124,6 @@ class VIEWS_EXPORT EditableCombobox ...@@ -124,7 +124,6 @@ class VIEWS_EXPORT EditableCombobox
// Overridden from View: // Overridden from View:
void Layout() override; void Layout() override;
void OnThemeChanged() override;
void GetAccessibleNodeData(ui::AXNodeData* node_data) override; void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
void RequestFocus() override; void RequestFocus() override;
bool GetNeedsNotificationWhenVisibleBoundsChange() const override; bool GetNeedsNotificationWhenVisibleBoundsChange() const override;
......
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