Fix image vertical alignment and multilining for LabelButton

Bug: 1028943
Change-Id: I32357a5f8673d4c12a583478525dbce682aa1e81
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1992421Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Roman Aleksandrov <raleksandrov@google.com>
Commit-Queue: Thomas Tellier <tellier@google.com>
Cr-Commit-Position: refs/heads/master@{#733698}

ui/views/controls/button/checkbox.cc

@@ -46,6 +46,7 @@ class Checkbox::FocusRingHighlightPathGenerator
 
 Checkbox::Checkbox(const base::string16& label, ButtonListener* listener)
     : LabelButton(listener, label), checked_(false), label_ax_id_(0) {
+  SetImageCentered(false);
   SetHorizontalAlignment(gfx::ALIGN_LEFT);
   SetFocusForPlatform();
 

ui/views/controls/button/label_button.cc

@@ -176,6 +176,17 @@ void LabelButton::SetImageLabelSpacing(int spacing) {
   OnPropertyChanged(&image_label_spacing_, kPropertyEffectsLayout);
 }
 
+bool LabelButton::GetImageCentered() const {
+  return image_centered_;
+}
+
+void LabelButton::SetImageCentered(bool image_centered) {
+  if (GetImageCentered() == image_centered)
+    return;
+  image_centered_ = image_centered;
+  OnPropertyChanged(&image_centered_, kPropertyEffectsLayout);
+}
+
 std::unique_ptr<LabelButtonBorder> LabelButton::CreateDefaultBorder() const {
   auto border = std::make_unique<LabelButtonBorder>();
   border->set_insets(views::LabelButtonAssetBorder::GetDefaultInsets());
@@ -292,9 +303,7 @@ void LabelButton::Layout() {
       label_area.height());
 
   gfx::Point image_origin = child_area.origin();
-  if (label_->GetMultiLine()) {
-    // Right now this code currently only works for CheckBox and RadioButton
-    // descendants that have multi-line enabled for their label.
+  if (label_->GetMultiLine() && !image_centered_) {
     image_origin.Offset(
         0, std::max(
                0, (label_->font_list().GetHeight() - image_size.height()) / 2));
@@ -559,6 +568,7 @@ ADD_PROPERTY_METADATA(LabelButton, gfx::Size, MinSize)
 ADD_PROPERTY_METADATA(LabelButton, gfx::Size, MaxSize)
 ADD_PROPERTY_METADATA(LabelButton, bool, IsDefault)
 ADD_PROPERTY_METADATA(LabelButton, int, ImageLabelSpacing)
+ADD_PROPERTY_METADATA(LabelButton, bool, ImageCentered)
 END_METADATA()
 
 }  // namespace views

ui/views/controls/button/label_button.h

@@ -91,6 +91,11 @@ class VIEWS_EXPORT LabelButton : public Button, public NativeThemeDelegate {
   int GetImageLabelSpacing() const;
   void SetImageLabelSpacing(int spacing);
 
+  // Gets or sets the option to place the image aligned with the center of the
+  // the label. The image is not centered for CheckBox and RadioButton only.
+  bool GetImageCentered() const;
+  void SetImageCentered(bool image_centered);
+
   // Creates the default border for this button. This can be overridden by
   // subclasses.
   virtual std::unique_ptr<LabelButtonBorder> CreateDefaultBorder() const;
@@ -224,6 +229,11 @@ class VIEWS_EXPORT LabelButton : public Button, public NativeThemeDelegate {
   // True if current border was set by UpdateThemedBorder.
   bool border_is_themed_border_ = true;
 
+  // A flag indicating that this button's image should be aligned with the
+  // center of the label when multiline is enabled. This shouldn't be the case
+  // for a CheckBox or a RadioButton.
+  bool image_centered_ = true;
+
   // Spacing between the image and the text.
   int image_label_spacing_ = LayoutProvider::Get()->GetDistanceMetric(
       DISTANCE_RELATED_LABEL_HORIZONTAL);

ui/views/controls/button/label_button_unittest.cc

@@ -345,6 +345,31 @@ TEST_F(LabelButtonTest, Image) {
   EXPECT_EQ(button_->GetPreferredSize(), gfx::Size(large_size, large_size));
 }
 
+TEST_F(LabelButtonTest, ImageAlignmentWithMultilineLabel) {
+  const base::string16 text(
+      ASCIIToUTF16("Some long text that would result in multiline label"));
+  button_->SetText(text);
+
+  const int max_label_width = 40;
+  button_->label()->SetMultiLine(true);
+  button_->label()->SetMaximumWidth(max_label_width);
+
+  const int image_size = 16;
+  const gfx::ImageSkia image = CreateTestImage(image_size, image_size);
+  button_->SetImage(Button::STATE_NORMAL, image);
+
+  button_->SetBoundsRect(gfx::Rect(button_->GetPreferredSize()));
+  button_->Layout();
+  int y_origin_centered = button_->image()->origin().y();
+
+  button_->SetBoundsRect(gfx::Rect(button_->GetPreferredSize()));
+  button_->SetImageCentered(false);
+  button_->Layout();
+  int y_origin_not_centered = button_->image()->origin().y();
+
+  EXPECT_LT(y_origin_not_centered, y_origin_centered);
+}
+
 TEST_F(LabelButtonTest, LabelAndImage) {
   const gfx::FontList font_list = button_->label()->font_list();
   const base::string16 text(ASCIIToUTF16("abcdefghijklm"));
@@ -422,6 +447,7 @@ TEST_F(LabelButtonTest, LabelWrapAndImageAlignment) {
   ASSERT_EQ(font_list.GetHeight(), image.width());
 
   button_->SetImage(Button::STATE_NORMAL, image);
+  button_->SetImageCentered(false);
   button_->SetMaxSize(
       gfx::Size(image.width() + image_spacing + text_wrap_width, 0));
 
@@ -436,7 +462,7 @@ TEST_F(LabelButtonTest, LabelWrapAndImageAlignment) {
   EXPECT_EQ(preferred_size.height(),
             font_list.GetHeight() * 2 + button_insets.height());
 
-  // The image should be centered on the first line of the multi-line label.
+  // The image should be centered on the first line of the multi-line label
   EXPECT_EQ(button_->image()->y(),
             (font_list.GetHeight() - button_->image()->height()) / 2 +
                 button_insets.top());

ui/views/controls/label.cc

@@ -345,7 +345,7 @@ void Label::SetMaximumWidth(int max_width) {
   if (max_width_ == max_width)
     return;
   max_width_ = max_width;
-  OnPropertyChanged(&max_width_, kPropertyEffectsPreferredSizeChanged);
+  OnPropertyChanged(&max_width_, kPropertyEffectsLayout);
 }
 
 bool Label::GetCollapseWhenHidden() const {