diff options
Diffstat (limited to 'chromium/ui/views/controls/button')
14 files changed, 135 insertions, 83 deletions
diff --git a/chromium/ui/views/controls/button/button.cc b/chromium/ui/views/controls/button/button.cc index d74e05dc34c..5bb670ce30a 100644 --- a/chromium/ui/views/controls/button/button.cc +++ b/chromium/ui/views/controls/button/button.cc @@ -56,6 +56,7 @@ Button::WidgetObserverButtonBridge::WidgetObserverButtonBridge(Button* button) Button::WidgetObserverButtonBridge::~WidgetObserverButtonBridge() { if (owner_) owner_->GetWidget()->RemoveObserver(this); + CHECK(!IsInObserverList()); } void Button::WidgetObserverButtonBridge::OnWidgetPaintAsActiveChanged( @@ -240,10 +241,12 @@ void Button::SetAnimationDuration(base::TimeDelta duration) { } void Button::SetInstallFocusRingOnFocus(bool install) { - if (install) + if (focus_ring_ && !install) { + RemoveChildViewT(focus_ring_); + focus_ring_ = nullptr; + } else if (!focus_ring_ && install) { focus_ring_ = FocusRing::Install(this); - else - focus_ring_.reset(); + } } void Button::SetHotTracked(bool is_hot_tracked) { diff --git a/chromium/ui/views/controls/button/button.h b/chromium/ui/views/controls/button/button.h index 79bab1f6034..6411eb3514b 100644 --- a/chromium/ui/views/controls/button/button.h +++ b/chromium/ui/views/controls/button/button.h @@ -292,7 +292,7 @@ class VIEWS_EXPORT Button : public InkDropHostView, return hover_animation_; } - FocusRing* focus_ring() { return focus_ring_.get(); } + FocusRing* focus_ring() { return focus_ring_; } // The button's listener. Notified when clicked. ButtonListener* listener_; @@ -368,7 +368,7 @@ class VIEWS_EXPORT Button : public InkDropHostView, SkColor ink_drop_base_color_; // The focus ring for this Button. - std::unique_ptr<FocusRing> focus_ring_; + FocusRing* focus_ring_ = nullptr; std::unique_ptr<Painter> focus_painter_; diff --git a/chromium/ui/views/controls/button/button_unittest.cc b/chromium/ui/views/controls/button/button_unittest.cc index 3dfeba259fd..5850b215f25 100644 --- a/chromium/ui/views/controls/button/button_unittest.cc +++ b/chromium/ui/views/controls/button/button_unittest.cc @@ -35,6 +35,7 @@ #include "ui/views/controls/link.h" #include "ui/views/controls/textfield/textfield.h" #include "ui/views/style/platform_style.h" +#include "ui/views/test/view_metadata_test_utils.h" #include "ui/views/test/views_test_base.h" #include "ui/views/widget/widget_utils.h" @@ -178,15 +179,6 @@ TestInkDrop* AddTestInkDrop(TestButton* button) { return ink_drop; } -// TODO(tluk): remove when the appropriate ownership APIs have been added for -// Widget's SetContentsView(). -template <typename T> -T* AddContentsView(Widget* widget, std::unique_ptr<T> view) { - T* view_ptr = view.get(); - widget->SetContentsView(view.release()); - return view_ptr; -} - } // namespace class ButtonTest : public ViewsTestBase { @@ -207,7 +199,7 @@ class ButtonTest : public ViewsTestBase { widget_->Init(std::move(params)); widget_->Show(); - button_ = AddContentsView(widget(), std::make_unique<TestButton>(false)); + button_ = widget()->SetContentsView(std::make_unique<TestButton>(false)); event_generator_ = std::make_unique<ui::test::EventGenerator>(GetRootWindow(widget())); @@ -225,24 +217,21 @@ class ButtonTest : public ViewsTestBase { } TestInkDrop* CreateButtonWithInkDrop(bool has_ink_drop_action_on_click) { - button_ = AddContentsView( - widget(), std::make_unique<TestButton>(has_ink_drop_action_on_click)); - widget_->SetContentsView(button_); + button_ = widget()->SetContentsView( + std::make_unique<TestButton>(has_ink_drop_action_on_click)); return AddTestInkDrop(button_); } void CreateButtonWithRealInkDrop() { - button_ = AddContentsView(widget(), std::make_unique<TestButton>(false)); + button_ = widget()->SetContentsView(std::make_unique<TestButton>(false)); InkDropHostViewTestApi(button_).SetInkDrop( std::make_unique<InkDropImpl>(button_, button_->size())); - widget_->SetContentsView(button_); } void CreateButtonWithObserver() { - button_ = AddContentsView(widget(), std::make_unique<TestButton>(false)); + button_ = widget()->SetContentsView(std::make_unique<TestButton>(false)); button_observer_ = std::make_unique<TestButtonObserver>(); button_->AddButtonObserver(button_observer_.get()); - widget_->SetContentsView(button_); } protected: @@ -263,7 +252,12 @@ class ButtonTest : public ViewsTestBase { DISALLOW_COPY_AND_ASSIGN(ButtonTest); }; -// Tests that hover state changes correctly when visiblity/enableness changes. +// Iterate through the metadata for Button to ensure it all works. +TEST_F(ButtonTest, MetadataTest) { + test::TestViewMetadata(button()); +} + +// Tests that hover state changes correctly when visibility/enableness changes. TEST_F(ButtonTest, HoverStateOnVisibilityChange) { event_generator()->MoveMouseTo(button()->GetBoundsInScreen().CenterPoint()); event_generator()->PressLeftButton(); @@ -599,7 +593,7 @@ TEST_F(ButtonTest, InkDropAfterTryingToShowContextMenu) { } TEST_F(ButtonTest, HideInkDropHighlightWhenRemoved) { - View* contents_view = AddContentsView(widget(), std::make_unique<View>()); + View* contents_view = widget()->SetContentsView(std::make_unique<View>()); TestButton* button = contents_view->AddChildView(std::make_unique<TestButton>(false)); @@ -779,7 +773,7 @@ class VisibilityTestButton : public TestButton { // changed visibility states. TEST_F(ButtonTest, NoLayerAddedForWidgetVisibilityChanges) { VisibilityTestButton* button = - AddContentsView(widget(), std::make_unique<VisibilityTestButton>()); + widget()->SetContentsView(std::make_unique<VisibilityTestButton>()); // Ensure no layers are created during construction. EXPECT_TRUE(button->GetVisible()); diff --git a/chromium/ui/views/controls/button/checkbox_unittest.cc b/chromium/ui/views/controls/button/checkbox_unittest.cc index 3b3e5d83d76..a468b3d7212 100644 --- a/chromium/ui/views/controls/button/checkbox_unittest.cc +++ b/chromium/ui/views/controls/button/checkbox_unittest.cc @@ -32,8 +32,8 @@ class CheckboxTest : public ViewsTestBase { widget_->Init(std::move(params)); widget_->Show(); - checkbox_ = new Checkbox(base::string16()); - widget_->SetContentsView(checkbox_); + checkbox_ = + widget_->SetContentsView(std::make_unique<Checkbox>(base::string16())); } void TearDown() override { diff --git a/chromium/ui/views/controls/button/image_button_factory.cc b/chromium/ui/views/controls/button/image_button_factory.cc index 5ac49262cfe..7d7d6a9a520 100644 --- a/chromium/ui/views/controls/button/image_button_factory.cc +++ b/chromium/ui/views/controls/button/image_button_factory.cc @@ -110,21 +110,11 @@ void SetImageFromVectorIconWithColor(ImageButton* button, button->set_ink_drop_base_color(icon_color); } -void SetToggledImageFromVectorIcon(ToggleImageButton* button, - const gfx::VectorIcon& icon, - int dip_size, - SkColor related_text_color) { - const SkColor icon_color = - color_utils::DeriveDefaultIconColor(related_text_color); - SetToggledImageFromVectorIconWithColor(button, icon, dip_size, icon_color); -} - void SetToggledImageFromVectorIconWithColor(ToggleImageButton* button, const gfx::VectorIcon& icon, int dip_size, - SkColor icon_color) { - const SkColor disabled_color = - SkColorSetA(icon_color, gfx::kDisabledControlAlpha); + SkColor icon_color, + SkColor disabled_color) { const gfx::ImageSkia normal_image = gfx::CreateVectorIcon(icon, dip_size, icon_color); const gfx::ImageSkia disabled_image = diff --git a/chromium/ui/views/controls/button/image_button_factory.h b/chromium/ui/views/controls/button/image_button_factory.h index 38d7c70ad52..de2d57501b2 100644 --- a/chromium/ui/views/controls/button/image_button_factory.h +++ b/chromium/ui/views/controls/button/image_button_factory.h @@ -32,8 +32,7 @@ VIEWS_EXPORT std::unique_ptr<ImageButton> CreateVectorImageButton( ButtonListener* listener); // Creates a ToggleImageButton with an ink drop and a centered image in -// preperation for applying a vector icon from SetImageFromVectorIcon and -// SetToggledImageFromVectorIcon below. +// preparation for applying a vector icon from SetImageFromVectorIcon below. VIEWS_EXPORT std::unique_ptr<ToggleImageButton> CreateVectorToggleImageButton( ButtonListener* listener); @@ -73,7 +72,8 @@ VIEWS_EXPORT void SetToggledImageFromVectorIconWithColor( ToggleImageButton* button, const gfx::VectorIcon& icon, int dip_size, - SkColor icon_color); + SkColor icon_color, + SkColor disabled_color); } // namespace views diff --git a/chromium/ui/views/controls/button/image_button_factory_unittest.cc b/chromium/ui/views/controls/button/image_button_factory_unittest.cc index 5b8a5e4aee6..2197b35a527 100644 --- a/chromium/ui/views/controls/button/image_button_factory_unittest.cc +++ b/chromium/ui/views/controls/button/image_button_factory_unittest.cc @@ -56,24 +56,22 @@ class ImageButtonFactoryWidgetTest : public ViewsTestBase { } void TearDown() override { - button_.reset(); widget_.reset(); ViewsTestBase::TearDown(); } ImageButton* AddImageButton(std::unique_ptr<ImageButton> button) { - button_ = std::move(button); - widget_->SetContentsView(button_.get()); - return button_.get(); + button_ = widget_->SetContentsView(std::move(button)); + return button_; } protected: Widget* widget() { return widget_.get(); } - ImageButton* button() { return button_.get(); } + ImageButton* button() { return button_; } private: std::unique_ptr<Widget> widget_; - std::unique_ptr<ImageButton> button_; + ImageButton* button_ = nullptr; // owned by |widget_|. DISALLOW_COPY_AND_ASSIGN(ImageButtonFactoryWidgetTest); }; diff --git a/chromium/ui/views/controls/button/label_button.cc b/chromium/ui/views/controls/button/label_button.cc index bb9105f1a18..3aef54f32cd 100644 --- a/chromium/ui/views/controls/button/label_button.cc +++ b/chromium/ui/views/controls/button/label_button.cc @@ -281,20 +281,21 @@ int LabelButton::GetHeightForWidth(int width) const { } void LabelButton::Layout() { - gfx::Rect child_area = GetLocalBounds(); + gfx::Rect image_area = GetLocalBounds(); - ink_drop_container_->SetBoundsRect(child_area); - // The space that the label can use. Its actual bounds may be smaller if the - // label is short. - gfx::Rect label_area = child_area; + ink_drop_container_->SetBoundsRect(image_area); gfx::Insets insets = GetInsets(); - child_area.Inset(insets); - // Labels can paint over the vertical component of the border insets. - label_area.Inset(insets.left(), 0, insets.right(), 0); + // If the button have a limited space to fit in, the image and the label + // may overlap with the border, which often times contains a lot of empty + // padding. + image_area.Inset(insets.left(), 0, insets.right(), 0); + // The space that the label can use. Labels truncate horizontally, so there + // is no need to allow the label to take up the complete horizontal space. + gfx::Rect label_area = image_area; gfx::Size image_size = image_->GetPreferredSize(); - image_size.SetToMin(child_area.size()); + image_size.SetToMin(image_area.size()); const auto horizontal_alignment = GetHorizontalAlignment(); if (!image_size.IsEmpty()) { @@ -309,22 +310,28 @@ void LabelButton::Layout() { std::min(label_area.width(), label_->GetPreferredSize().width()), label_area.height()); - gfx::Point image_origin = child_area.origin(); + gfx::Point image_origin = image_area.origin(); if (label_->GetMultiLine() && !image_centered_) { - image_origin.Offset( - 0, std::max( - 0, (label_->font_list().GetHeight() - image_size.height()) / 2)); + // This code assumes the text is vertically centered. + DCHECK_EQ(gfx::ALIGN_MIDDLE, label_->GetVerticalAlignment()); + int label_height = label_->GetHeightForWidth(label_size.width()); + int first_line_y = + label_area.y() + (label_area.height() - label_height) / 2; + int image_origin_y = + first_line_y + + (label_->font_list().GetHeight() - image_size.height()) / 2; + image_origin.Offset(0, std::max(0, image_origin_y)); } else { - image_origin.Offset(0, (child_area.height() - image_size.height()) / 2); + image_origin.Offset(0, (image_area.height() - image_size.height()) / 2); } if (horizontal_alignment == gfx::ALIGN_CENTER) { const int spacing = (image_size.width() > 0 && label_size.width() > 0) ? GetImageLabelSpacing() : 0; const int total_width = image_size.width() + label_size.width() + spacing; - image_origin.Offset((child_area.width() - total_width) / 2, 0); + image_origin.Offset((image_area.width() - total_width) / 2, 0); } else if (horizontal_alignment == gfx::ALIGN_RIGHT) { - image_origin.Offset(child_area.width() - image_size.width(), 0); + image_origin.Offset(image_area.width() - image_size.width(), 0); } image_->SetBoundsRect(gfx::Rect(image_origin, image_size)); diff --git a/chromium/ui/views/controls/button/label_button_label_unittest.cc b/chromium/ui/views/controls/button/label_button_label_unittest.cc index 505c2d5ed6c..8e880de4eaf 100644 --- a/chromium/ui/views/controls/button/label_button_label_unittest.cc +++ b/chromium/ui/views/controls/button/label_button_label_unittest.cc @@ -62,12 +62,22 @@ class LabelButtonLabelTest : public ViewsTestBase { void SetUp() override { ViewsTestBase::SetUp(); - label_ = std::make_unique<TestLabel>(&last_color_); + + widget_ = CreateTestWidget(); + label_ = + widget_->SetContentsView(std::make_unique<TestLabel>(&last_color_)); + label_->SetAutoColorReadabilityEnabled(false); + } + + void TearDown() override { + widget_.reset(); + ViewsTestBase::TearDown(); } protected: - SkColor last_color_ = SK_ColorCYAN; - std::unique_ptr<TestLabel> label_; + SkColor last_color_ = gfx::kPlaceholderColor; + std::unique_ptr<views::Widget> widget_; + TestLabel* label_; TestNativeTheme theme1_; TestNativeTheme theme2_; @@ -77,16 +87,6 @@ class LabelButtonLabelTest : public ViewsTestBase { // Test that LabelButtonLabel reacts properly to themed and overridden colors. TEST_F(LabelButtonLabelTest, Colors) { - // The OnDidSchedulePaint() override won't be called while the base - // class is initialized. Not much we can do about that, so give it the first - // for free. - EXPECT_EQ(SK_ColorCYAN, last_color_); // Sanity check. - - // At the same time we can check that changing the auto color readability - // schedules a paint. Currently it does. Although it technically doesn't need - // to since the color isn't actually changing. - label_->SetAutoColorReadabilityEnabled(false); - // First one comes from the default theme. This check ensures the SK_ColorRED // placeholder initializers were replaced. SkColor default_theme_enabled_color = diff --git a/chromium/ui/views/controls/button/label_button_unittest.cc b/chromium/ui/views/controls/button/label_button_unittest.cc index cd33dbe4c0a..38443891fff 100644 --- a/chromium/ui/views/controls/button/label_button_unittest.cc +++ b/chromium/ui/views/controls/button/label_button_unittest.cc @@ -627,8 +627,8 @@ TEST_F(LabelButtonTest, HighlightedButtonStyle) { // Ensure the label resets the enabled color after LabelButton::OnThemeChanged() // is invoked. TEST_F(LabelButtonTest, OnThemeChanged) { - ASSERT_NE(button_->GetNativeTheme()->GetHighContrastColorScheme(), - ui::NativeTheme::HighContrastColorScheme::kDark); + ASSERT_NE(button_->GetNativeTheme()->GetPlatformHighContrastColorScheme(), + ui::NativeTheme::PlatformHighContrastColorScheme::kDark); ASSERT_NE(button_->label()->GetBackgroundColor(), SK_ColorBLACK); EXPECT_EQ(themed_normal_text_color_, button_->label()->GetEnabledColor()); @@ -667,6 +667,25 @@ TEST_F(LabelButtonTest, SetEnabledTextColorsResetsToThemeColors) { EXPECT_EQ(TestNativeTheme::kSystemColor, button_->label()->GetEnabledColor()); } +TEST_F(LabelButtonTest, ImageOrLabelGetClipped) { + const base::string16 text(ASCIIToUTF16("abc")); + button_->SetText(text); + + const gfx::FontList font_list = button_->label()->font_list(); + const int image_size = font_list.GetHeight(); + button_->SetImage(Button::STATE_NORMAL, + CreateTestImage(image_size, image_size)); + + button_->SetBoundsRect(gfx::Rect(button_->GetPreferredSize())); + // The border size + the content height is more than button's preferred size. + button_->SetBorder(CreateEmptyBorder(image_size / 2, 0, image_size / 2, 0)); + button_->Layout(); + + // Ensure that content (image and label) doesn't get clipped by the border. + EXPECT_GE(button_->image()->height(), image_size); + EXPECT_GE(button_->label()->height(), image_size); +} + // Test fixture for a LabelButton that has an ink drop configured. class InkDropLabelButtonTest : public ViewsTestBase { public: diff --git a/chromium/ui/views/controls/button/md_text_button.cc b/chromium/ui/views/controls/button/md_text_button.cc index b9905fc6359..79c42e7b9ac 100644 --- a/chromium/ui/views/controls/button/md_text_button.cc +++ b/chromium/ui/views/controls/button/md_text_button.cc @@ -142,6 +142,11 @@ void MdTextButton::SetEnabledTextColors(base::Optional<SkColor> color) { UpdateColors(); } +void MdTextButton::SetCustomPadding(const gfx::Insets& padding) { + custom_padding_ = padding; + UpdatePadding(); +} + void MdTextButton::SetText(const base::string16& text) { LabelButton::SetText(text); UpdatePadding(); @@ -183,6 +188,11 @@ void MdTextButton::UpdatePadding() { return; } + SetBorder( + CreateEmptyBorder(custom_padding_.value_or(CalculateDefaultPadding()))); +} + +gfx::Insets MdTextButton::CalculateDefaultPadding() const { // Text buttons default to 28dp in height on all platforms when the base font // is in use, but should grow or shrink if the font size is adjusted up or // down. When the system font size has been adjusted, the base font will be @@ -213,8 +223,8 @@ void MdTextButton::UpdatePadding() { // we apply the MD treatment to all buttons, even GTK buttons? const int horizontal_padding = LayoutProvider::Get()->GetDistanceMetric( DISTANCE_BUTTON_HORIZONTAL_PADDING); - SetBorder(CreateEmptyBorder(top_padding, horizontal_padding, bottom_padding, - horizontal_padding)); + return gfx::Insets(top_padding, horizontal_padding, bottom_padding, + horizontal_padding); } void MdTextButton::UpdateColors() { diff --git a/chromium/ui/views/controls/button/md_text_button.h b/chromium/ui/views/controls/button/md_text_button.h index 760ab2a80ae..1d293a70166 100644 --- a/chromium/ui/views/controls/button/md_text_button.h +++ b/chromium/ui/views/controls/button/md_text_button.h @@ -39,6 +39,9 @@ class VIEWS_EXPORT MdTextButton : public LabelButton { void SetCornerRadius(float radius); float GetCornerRadius() const; + // See |custom_padding_|. + void SetCustomPadding(const gfx::Insets& padding); + // LabelButton: void OnThemeChanged() override; std::unique_ptr<views::InkDropHighlight> CreateInkDropHighlight() @@ -59,6 +62,7 @@ class VIEWS_EXPORT MdTextButton : public LabelButton { private: void UpdatePadding(); void UpdateColors(); + gfx::Insets CalculateDefaultPadding() const; // True if this button uses prominent styling (blue fill, etc.). bool is_prominent_ = false; @@ -68,6 +72,9 @@ class VIEWS_EXPORT MdTextButton : public LabelButton { float corner_radius_ = 0.0f; + // Used to override default padding. + base::Optional<gfx::Insets> custom_padding_; + DISALLOW_COPY_AND_ASSIGN(MdTextButton); }; diff --git a/chromium/ui/views/controls/button/md_text_button_unittest.cc b/chromium/ui/views/controls/button/md_text_button_unittest.cc new file mode 100644 index 00000000000..5fe5d9c1bf1 --- /dev/null +++ b/chromium/ui/views/controls/button/md_text_button_unittest.cc @@ -0,0 +1,25 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "ui/views/controls/button/md_text_button.h" + +#include "ui/views/test/views_test_base.h" + +namespace views { + +using MdTextButtonTest = ViewsTestBase; + +TEST_F(MdTextButtonTest, CustomPadding) { + const base::string16 text = base::ASCIIToUTF16("abc"); + std::unique_ptr<MdTextButton> button = + MdTextButton::Create(nullptr, text, views::style::CONTEXT_BUTTON_MD); + + const gfx::Insets custom_padding(10, 20); + ASSERT_NE(button->GetInsets(), custom_padding); + + button->SetCustomPadding(custom_padding); + EXPECT_EQ(button->GetInsets(), custom_padding); +} + +} // namespace views diff --git a/chromium/ui/views/controls/button/radio_button_unittest.cc b/chromium/ui/views/controls/button/radio_button_unittest.cc index 3d25994df2a..8d22480097e 100644 --- a/chromium/ui/views/controls/button/radio_button_unittest.cc +++ b/chromium/ui/views/controls/button/radio_button_unittest.cc @@ -34,8 +34,7 @@ class RadioButtonTest : public ViewsTestBase { widget_->Init(std::move(params)); widget_->Show(); - button_container_ = new View(); - widget_->SetContentsView(button_container_); + button_container_ = widget_->SetContentsView(std::make_unique<View>()); } void TearDown() override { |