Handle bubble title resizing (growth) by reworking title layout

ui/views/bubble/bubble_frame_view.cc

 // Copyright (c) 2012 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/bubble/bubble_frame_view.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "build/build_config.h"
@@ skipping 17 matching lines @@
 #include "ui/views/layout/layout_constants.h"
 #include "ui/views/resources/grit/views_resources.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
 #include "ui/views/window/client_view.h"
 
 namespace views {
 
 namespace {
 
-// The horizontal padding between the title and the icon.
-const int kTitleHorizontalPadding = 5;
-
 // Background color of the footnote view.
 const SkColor kFootnoteBackgroundColor = SkColorSetRGB(245, 245, 245);
 
 // Color of the top border of the footnote.
 const SkColor kFootnoteBorderColor = SkColorSetRGB(229, 229, 229);
 
 // Get the |vertical| or horizontal amount that |available_bounds| overflows
 // |window_bounds|.
 int GetOffScreenLength(const gfx::Rect& available_bounds,
                        const gfx::Rect& window_bounds,
@@ skipping 244 matching lines @@
   gfx::Rect bounds(GetContentsBounds());
   bounds.Inset(title_margins_);
   if (bounds.IsEmpty())
     return;
 
   // The close button is positioned somewhat closer to the edge of the bubble.
   gfx::Point close_position = GetContentsBounds().top_right();
   close_position += gfx::Vector2d(-close_->width() - 7, 6);
   close_->SetPosition(close_position);
 
-  gfx::Size title_icon_size(title_icon_->GetPreferredSize());
-  gfx::Size title_label_size(title_->GetPreferredSize());
+  gfx::Size title_icon_pref_size(title_icon_->GetPreferredSize());
   int padding = 0;
-  if (title_icon_size.width() > 0 && title_label_size.width() > 0)
-    padding = kTitleHorizontalPadding;
-  const int title_height = std::max(title_icon_size.height(),
-                                    title_label_size.height());
 
-  const int title_icon_width = std::max(0, close_->x() - bounds.x());
-  title_icon_size.SetToMin(gfx::Size(title_icon_width, title_height));
-  gfx::Rect title_icon_bounds(
-      bounds.x(), bounds.y(), title_icon_size.width(), title_height);
-  title_icon_->SetBoundsRect(title_icon_bounds);
+  if (!title_->text().empty()) {

[msw, 2016/03/31 19:18:31]

Some WidgetDelegate may return a non-empty GetWindowTitle, but false for
ShouldShowWindowTitle, which was previously handled via
set_collapse_when_hidden(true) and pref size checking. Checking for a non-empty
string might regress layout in those cases; maybe check title_->visible() too?

[msw, 2016/03/31 21:28:27]

Ping

[Evan Stade, 2016/03/31 22:07:06]

Done. Good catch.

+    if (title_icon_pref_size.width() > 0)
+      padding = title_margins_.left();

[msw, 2016/03/31 19:18:31]

Hmm, re-using the title margins (for outside the icon and label) as the
inter-view spacing might be okay, but what is the actual px difference?
Isn't BubbleDialogDelegateView's use of kPanelHorizMargin (13) here much bigger
than the current value (5)?
The (current?) global error bubble is ugly, but how does this change make it
look? https://bugs.chromium.org/p/chromium/issues/detail?id=449737#c19

[Evan Stade, 2016/03/31 21:26:13]

This screenshot captures both visual improvements/simplifications:
https://screenshot.googleplex.com/yv3MVAqxAHB.png

 
-  const int title_label_x = title_icon_->bounds().right() + padding;
-  const int title_label_width = std::max(0, close_->x() - title_label_x);
-  title_label_size.SetToMin(gfx::Size(title_label_width,
-                                      title_label_size.height()));
-  gfx::Rect title_label_bounds(
-      title_label_x, bounds.y(), title_label_size.width(), title_height);
-  title_->SetBoundsRect(title_label_bounds);
+    const int title_label_x =
+        bounds.x() + title_icon_pref_size.width() + padding;
+    title_->SizeToFit(std::max(1, close_->x() - title_label_x));

[msw, 2016/03/31 19:18:31]

q: why 1 over 0? (for my enlightenment)

[Evan Stade, 2016/03/31 21:26:13]

because I'm afraid of crazy things like infinite loops when you try to do the
impossible (squeeze text into zero horizontal space)

+    title_->SetPosition(gfx::Point(title_label_x, bounds.y()));
+  }
 
-  bounds.set_width(
-      title_icon_size.width() + title_label_size.width() + padding);
+  const int title_height =
+      std::max(title_icon_pref_size.height(), title_->height());
+  title_icon_->SetBounds(bounds.x(), bounds.y(), title_icon_pref_size.width(),
+                         title_height);
+  bounds.set_width(title_->bounds().right() - title_icon_->x());

[msw, 2016/03/31 19:18:31]

nit: use bounds.x() instead of title_icon_->x()?

[Evan Stade, 2016/03/31 21:26:13]

Done.

   bounds.set_height(title_height);
 
   if (footnote_container_) {
     gfx::Rect local_bounds = GetContentsBounds();
     int height = footnote_container_->GetHeightForWidth(local_bounds.width());
     footnote_container_->SetBounds(local_bounds.x(),
                                    local_bounds.bottom() - height,
                                    local_bounds.width(), height);
   }
 }
@@ skipping 162 matching lines @@
     SchedulePaint();
 }
 
 gfx::Size BubbleFrameView::GetSizeForClientSize(
     const gfx::Size& client_size) const {
   // Accommodate the width of the title bar elements.
   int title_bar_width = title_margins_.width() + border()->GetInsets().width();
   gfx::Size title_icon_size = title_icon_->GetPreferredSize();
   gfx::Size title_label_size = title_->GetPreferredSize();
   if (title_icon_size.width() > 0 && title_label_size.width() > 0)
-    title_bar_width += kTitleHorizontalPadding;
+    title_bar_width += title_margins_.left();
   title_bar_width += title_icon_size.width();
   if (close_->visible())
     title_bar_width += close_->width() + 1;
 
   gfx::Size size(client_size);
   gfx::Insets client_insets = GetInsets();
   size.Enlarge(client_insets.width(), client_insets.height());
   size.SetToMax(gfx::Size(title_bar_width, 0));
 
   if (footnote_container_)
     size.Enlarge(0, footnote_container_->GetHeightForWidth(size.width()));
 
   return size;
 }
 
 }  // namespace views