Commit c154e149 authored by Charlene Yan's avatar Charlene Yan Committed by Commit Bot

[Tab Groups] Make selection ring color match bubble background.

The inner ring is currently painted white which is not correct in dark
mode.

Bug: 1015634
Change-Id: Id028c5ed109b885ac20eeb4804bb493eae63567f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1961007
Commit-Queue: Charlene Yan <cyan@chromium.org>
Reviewed-by: default avatarPeter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723569}
parent f8389da6
...@@ -52,10 +52,12 @@ class ColorPickerElementView : public views::Button, ...@@ -52,10 +52,12 @@ class ColorPickerElementView : public views::Button,
public: public:
ColorPickerElementView( ColorPickerElementView(
base::RepeatingCallback<void(ColorPickerElementView*)> selected_callback, base::RepeatingCallback<void(ColorPickerElementView*)> selected_callback,
SkColor background_color,
SkColor color, SkColor color,
base::string16 color_name) base::string16 color_name)
: Button(this), : Button(this),
selected_callback_(std::move(selected_callback)), selected_callback_(std::move(selected_callback)),
background_color_(background_color),
color_(color), color_(color),
color_name_(color_name) { color_name_(color_name) {
DCHECK(selected_callback_); DCHECK(selected_callback_);
...@@ -155,12 +157,11 @@ class ColorPickerElementView : public views::Button, ...@@ -155,12 +157,11 @@ class ColorPickerElementView : public views::Button,
// Visual parameters of our ring. // Visual parameters of our ring.
constexpr float kInset = 3.0f; constexpr float kInset = 3.0f;
constexpr float kThickness = 2.0f; constexpr float kThickness = 2.0f;
constexpr SkColor paint_color = SK_ColorWHITE;
cc::PaintFlags flags; cc::PaintFlags flags;
flags.setStyle(cc::PaintFlags::kStroke_Style); flags.setStyle(cc::PaintFlags::kStroke_Style);
flags.setStrokeWidth(kThickness); flags.setStrokeWidth(kThickness);
flags.setAntiAlias(true); flags.setAntiAlias(true);
flags.setColor(paint_color); flags.setColor(background_color_);
gfx::RectF indicator_bounds(GetContentsBounds()); gfx::RectF indicator_bounds(GetContentsBounds());
indicator_bounds.Inset(gfx::InsetsF(kInset)); indicator_bounds.Inset(gfx::InsetsF(kInset));
...@@ -169,14 +170,17 @@ class ColorPickerElementView : public views::Button, ...@@ -169,14 +170,17 @@ class ColorPickerElementView : public views::Button,
indicator_bounds.width() / 2.0f, flags); indicator_bounds.width() / 2.0f, flags);
} }
base::RepeatingCallback<void(ColorPickerElementView*)> selected_callback_; const base::RepeatingCallback<void(ColorPickerElementView*)>
SkColor color_; selected_callback_;
base::string16 color_name_; const SkColor background_color_;
const SkColor color_;
const base::string16 color_name_;
bool selected_ = false; bool selected_ = false;
}; };
ColorPickerView::ColorPickerView( ColorPickerView::ColorPickerView(
base::span<const std::pair<SkColor, base::string16>> colors, base::span<const std::pair<SkColor, base::string16>> colors,
SkColor background_color,
SkColor initial_color, SkColor initial_color,
ColorSelectedCallback callback) ColorSelectedCallback callback)
: callback_(std::move(callback)) { : callback_(std::move(callback)) {
...@@ -187,7 +191,7 @@ ColorPickerView::ColorPickerView( ...@@ -187,7 +191,7 @@ ColorPickerView::ColorPickerView(
// views in our destructor, ensuring we outlive them. // views in our destructor, ensuring we outlive them.
elements_.push_back(AddChildView(std::make_unique<ColorPickerElementView>( elements_.push_back(AddChildView(std::make_unique<ColorPickerElementView>(
base::Bind(&ColorPickerView::OnColorSelected, base::Unretained(this)), base::Bind(&ColorPickerView::OnColorSelected, base::Unretained(this)),
color.first, color.second))); background_color, color.first, color.second)));
if (initial_color == color.first) if (initial_color == color.first)
elements_.back()->SetSelected(true); elements_.back()->SetSelected(true);
} }
......
...@@ -31,6 +31,7 @@ class ColorPickerView : public views::View { ...@@ -31,6 +31,7 @@ class ColorPickerView : public views::View {
// not be duplicate colors. // not be duplicate colors.
explicit ColorPickerView( explicit ColorPickerView(
base::span<const std::pair<SkColor, base::string16>> colors, base::span<const std::pair<SkColor, base::string16>> colors,
SkColor background_color,
SkColor initial_color, SkColor initial_color,
ColorSelectedCallback callback); ColorSelectedCallback callback);
~ColorPickerView() override; ~ColorPickerView() override;
......
...@@ -38,8 +38,9 @@ class ColorPickerViewTest : public ChromeViewsTestBase { ...@@ -38,8 +38,9 @@ class ColorPickerViewTest : public ChromeViewsTestBase {
widget_ = std::make_unique<views::Widget>(); widget_ = std::make_unique<views::Widget>();
widget_->Init(std::move(widget_params)); widget_->Init(std::move(widget_params));
color_picker_ = new ColorPickerView(kTestColors, SK_ColorCYAN, color_picker_ =
color_selected_callback_.Get()); new ColorPickerView(kTestColors, SK_ColorWHITE, SK_ColorCYAN,
color_selected_callback_.Get());
widget_->SetContentsView(color_picker_); widget_->SetContentsView(color_picker_);
color_picker_->SizeToPreferredSize(); color_picker_->SizeToPreferredSize();
...@@ -102,8 +103,9 @@ TEST_F(ColorPickerViewTest, ColorSelectedByDefaultIfMatching) { ...@@ -102,8 +103,9 @@ TEST_F(ColorPickerViewTest, ColorSelectedByDefaultIfMatching) {
std::unique_ptr<views::Widget> widget = std::make_unique<views::Widget>(); std::unique_ptr<views::Widget> widget = std::make_unique<views::Widget>();
widget->Init(std::move(widget_params)); widget->Init(std::move(widget_params));
ColorPickerView* color_picker = new ColorPickerView( ColorPickerView* color_picker =
kTestColors, initial_color, color_selected_callback_.Get()); new ColorPickerView(kTestColors, SK_ColorWHITE, initial_color,
color_selected_callback_.Get());
widget->SetContentsView(color_picker); widget->SetContentsView(color_picker);
color_picker->SizeToPreferredSize(); color_picker->SizeToPreferredSize();
......
...@@ -127,7 +127,7 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView( ...@@ -127,7 +127,7 @@ TabGroupEditorBubbleView::TabGroupEditorBubbleView(
color_selector_ = color_selector_ =
group_modifier_container->AddChildView(std::make_unique<ColorPickerView>( group_modifier_container->AddChildView(std::make_unique<ColorPickerView>(
GetColorPickerList(), current_data->color(), GetColorPickerList(), background_color(), current_data->color(),
base::Bind(&TabGroupEditorBubbleView::UpdateGroup, base::Bind(&TabGroupEditorBubbleView::UpdateGroup,
base::Unretained(this)))); base::Unretained(this))));
color_selector_->SetBorder(views::CreateEmptyBorder( color_selector_->SetBorder(views::CreateEmptyBorder(
......
...@@ -43,6 +43,8 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView { ...@@ -43,6 +43,8 @@ class TabGroupEditorBubbleView : public views::BubbleDialogDelegateView {
void UpdateGroup(); void UpdateGroup();
SkColor background_color() const { return color(); }
TabController* const tab_controller_; TabController* const tab_controller_;
const TabGroupId group_; const TabGroupId group_;
......
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