Adjust width of find in page bar.

chrome/browser/ui/views/find_bar_view.cc

Index: chrome/browser/ui/views/find_bar_view.cc
diff --git a/chrome/browser/ui/views/find_bar_view.cc b/chrome/browser/ui/views/find_bar_view.cc
index 002e56cae81e70d80d94a68020b51a5439e755f6..92911a8ce2ce53678b38c3900498de5c8fec33b4 100644
--- a/chrome/browser/ui/views/find_bar_view.cc
+++ b/chrome/browser/ui/views/find_bar_view.cc
@@ -89,9 +89,9 @@ const SkColor kBackgroundColorMatch = SkColorSetARGB(0, 255, 255, 255);
 // The background color of the match count label when no results are found.
 const SkColor kBackgroundColorNoMatch = SkColorSetRGB(255, 102, 102);
 
-// The default number of average characters that the text box will be. This
-// number brings the width on a "regular fonts" system to about 300px.
+// The default number of average characters that the text box will be.
 const int kDefaultCharWidth = 43;
+const int kDefaultCharWidthMd = 26;

[sky, 2016/07/18 23:56:04]

Even though this constant is smaller, is the new overall size bigger because of
the GetPreferredSize changes?

[Peter Kasting, 2016/07/18 23:58:27]

When you gave your "DesignDesignDesignDesign" example, was that alongside a "0
of 0" match count label as well?

If not, I'd probably increase this to be enough to fit at least both of those at
once, ideally on some platform like Win 7 that apparently has really wide
characters.

[Evan Stade, 2016/07/19 00:05:18]

yes indeedy

[Evan Stade, 2016/07/19 00:05:18]

yes (although I didn't verify this on Windows, just cros and linux)

 
 // The match count label is like a normal label, but can process events (which
 // makes it easier to forward events to the text input --- see
@@ -104,6 +104,15 @@ class MatchCountLabel : public views::Label {
   // views::Label overrides:
   bool CanProcessEventsWithinSubtree() const override { return true; }
 
+  gfx::Size GetPreferredSize() const override {
+    // Always reserve at least 1dp for the match count label so the box layout
+    // it's inside will allot it some space. Otherwise, it will cause the layout
+    // to jump in size when the contents switch between empty and non-empty.

[sky, 2016/07/18 23:56:04]

Why is the jump in size problematic?

[Evan Stade, 2016/07/19 00:05:18]

Because it makes the overall find bar width change when the match count label
changes (i.e. as soon as you start typing). Instead we want the find bar to
always be the same width.

[sky, 2016/07/19 02:51:50]

I'm confused. After your change the width is always 1 or the preferred width,
which varies based on number of matches. Won't that result in jumping? Also, if
the preferred width is based on the content won't the width vary?

[Evan Stade, 2016/07/19 13:44:54]

the preferred width is removed from the equation in
FindBarView::GetPreferredSize below, but it has to be non-zero to begin with for
the BoxLayout to put padding between it (match count text) and its siblings.

+    gfx::Size size = views::Label::GetPreferredSize();
+    size.set_width(std::max(1, size.width()));
+    return size;
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(MatchCountLabel);
 };
@@ -124,7 +133,9 @@ FindBarView::FindBarView(FindBarHost* host)
       close_button_(nullptr) {
   find_text_ = new views::Textfield;
   find_text_->set_id(VIEW_ID_FIND_IN_PAGE_TEXT_FIELD);
-  find_text_->set_default_width_in_chars(kDefaultCharWidth);
+  find_text_->set_default_width_in_chars(
+      ui::MaterialDesignController::IsModeMaterial() ? kDefaultCharWidthMd
+                                                     : kDefaultCharWidth);
   find_text_->set_controller(this);
   find_text_->SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_FIND));
   find_text_->SetTextInputFlags(ui::TEXT_INPUT_FLAG_AUTOCORRECT_OFF);
@@ -389,10 +400,11 @@ void FindBarView::Layout() {
 
 gfx::Size FindBarView::GetPreferredSize() const {
   if (ui::MaterialDesignController::IsModeMaterial()) {
-    // The entire bar is sized to a specific number of characters set on
-    // |find_text_|.
     gfx::Size size = views::View::GetPreferredSize();
-    size.set_width(find_text_->GetPreferredSize().width());
+    // Don't allot space for the match count text, it will take up some of the
+    // space set aside for the input.

[Peter Kasting, 2016/07/18 23:58:27]

This means "we want it to take up space set aside for the input", not "it will
take up some of this space unless we subtract that off here", right?

If so, maybe this would be slightly clearer?:

Ignore the preferred size for the match count label, and just let it take up
part of the space for the input textfield.  This prevents the overall width from
changing every time the match count text changes.

[Evan Stade, 2016/07/19 00:05:18]

Done.

+    size.set_width(size.width() -
+                   match_count_text_->GetPreferredSize().width());
     return size;

[Peter Kasting, 2016/07/18 23:44:54]

Still seems like we should be including the size of the buttons calculated below
(or whatever the MD equivalent is).

[Evan Stade, 2016/07/18 23:48:51]

we do, via View::GetPreferredSize. It's just less manual in MD (relies on
BoxLayout to calculate all that for us).

[Peter Kasting, 2016/07/18 23:58:27]

Ah.  I missed that.

   }