Harmonize the find in page dialog.

chrome/browser/ui/views/find_bar_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 "chrome/browser/ui/views/find_bar_view.h"
 
 #include <algorithm>
 
 #include "base/i18n/number_formatting.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
 #include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/find_bar/find_bar_controller.h"
 #include "chrome/browser/ui/find_bar/find_bar_state.h"
 #include "chrome/browser/ui/find_bar/find_bar_state_factory.h"
 #include "chrome/browser/ui/find_bar/find_notification_details.h"
 #include "chrome/browser/ui/find_bar/find_tab_helper.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/find_bar_host.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/strings/grit/components_strings.h"
 #include "ui/base/ime/input_method.h"
 #include "ui/base/ime/text_input_flags.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/theme_provider.h"
 #include "ui/events/event.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/geometry/insets.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/vector_icons/vector_icons.h"
 #include "ui/views/background.h"
 #include "ui/views/border.h"
 #include "ui/views/bubble/bubble_border.h"
 #include "ui/views/controls/button/image_button.h"
 #include "ui/views/controls/button/image_button_factory.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/separator.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/painter.h"
+#include "ui/views/view_properties.h"
 #include "ui/views/view_targeter.h"
 #include "ui/views/widget/widget.h"
 
 namespace {
 
-// These layout constants are all in dp.
-// The horizontal and vertical insets for the bar.
-const int kInteriorPadding = 8;
-// Default spacing between child views.
-const int kInterChildSpacing = 4;
-// Additional spacing around the separator.
-const int kSeparatorLeftSpacing = 12 - kInterChildSpacing;
-const int kSeparatorRightSpacing = 8 - kInterChildSpacing;
-
 // The default number of average characters that the text box will be.
 const int kDefaultCharWidth = 30;
 
 // 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
 // FindBarView::TargetForRect).
 class MatchCountLabel : public views::Label {
  public:
   MatchCountLabel() {}
   ~MatchCountLabel() override {}
@@ skipping 85 matching lines @@
   AddChildView(close_button_);
 
   AddChildView(focus_forwarder_view_);
 
   EnableCanvasFlippingForRTLUI(true);
 
   match_count_text_->SetEventTargeter(
       base::MakeUnique<views::ViewTargeter>(this));
   AddChildViewAt(match_count_text_, 1);
 
-  separator_->SetBorder(views::CreateEmptyBorder(0, kSeparatorLeftSpacing, 0,
-                                                 kSeparatorRightSpacing));
+  ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
+
+  // We use the same border as the image button for the separator to make it
+  // equidistant from the match count text on the left and the image button on
+  // the right.
+  gfx::Insets image_button_insets = provider->GetInsetsMetric(
+      views::INSETS_VECTOR_IMAGE_BUTTON);
+  separator_->SetBorder(views::CreateEmptyBorder(0, image_button_insets.left(),
+    0, image_button_insets.right()));

[Peter Kasting, 2017/07/14 00:40:12]

This won't make things equidistant (as is visible in your screenshot), since
this border is being applied to the button on the right but not the text on the
left, so to counter that you'd want to add it only on the left side.

But don't do that.  That doesn't fix the buttons being too far apart and making
the dialog too wide.  We really just need to be able to space buttons in a way
that ignores this offset.

If need be, we can hack this for this dialog, basically by putting 0 default
spacing between items, and then manually adding padding columns to the layout
manager.  It's not an automated solution, but it would serve.

[ananta, 2017/07/14 01:04:29]

ok. thanks. The patch came out wrong though. I was going to add the border to
the text and ended up adding it to the separator. 

+
   AddChildViewAt(separator_, 2);
 
+  find_previous_button_->SetProperty(views::kMarginsKey, new gfx::Insets(
+      provider->GetDistanceMetric(DISTANCE_TOAST_CONTROL_VERTICAL), 0));
+  find_next_button_->SetProperty(views::kMarginsKey, new gfx::Insets(
+      provider->GetDistanceMetric(DISTANCE_TOAST_CONTROL_VERTICAL), 0));
+  close_button_->SetProperty(views::kMarginsKey, new gfx::Insets(
+      provider->GetDistanceMetric(DISTANCE_TOAST_CONTROL_VERTICAL), 0));
+  find_text_->SetProperty(views::kMarginsKey, new gfx::Insets(
+      provider->GetDistanceMetric(DISTANCE_TOAST_CONTROL_VERTICAL), 0));
+  match_count_text_->SetProperty(views::kMarginsKey, new gfx::Insets(
+      provider->GetDistanceMetric(DISTANCE_TOAST_LABEL_VERTICAL), 0));
+  separator_->SetProperty(views::kMarginsKey, new gfx::Insets(
+      provider->GetDistanceMetric(DISTANCE_TOAST_LABEL_VERTICAL), 0));
+
   find_text_->SetBorder(views::NullBorder());
 
   views::BoxLayout* manager =
       new views::BoxLayout(views::BoxLayout::kHorizontal,
-                           gfx::Insets(kInteriorPadding), kInterChildSpacing);
+                           provider->GetInsetsMetric(views::INSETS_TOAST),
+                           provider->GetDistanceMetric(
+                              views::DISTANCE_RELATED_CONTROL_HORIZONTAL));
   SetLayoutManager(manager);
   manager->SetFlexForView(find_text_, 1);
 }
 
 FindBarView::~FindBarView() {
 }
 
 void FindBarView::SetFindTextAndSelectedRange(
     const base::string16& find_text,
     const gfx::Range& selected_range) {
@@ skipping 30 matching lines @@
     find_text_->SelectAll(true);
   }
 
   if (find_text.empty() || !have_valid_range) {
     // If there was no text entered, we don't show anything in the result count
     // area.
     ClearMatchCount();
     return;
   }
 
-  match_count_text_->SetText(l10n_util::GetStringFUTF16(
-      IDS_FIND_IN_PAGE_COUNT, base::FormatNumber(result.active_match_ordinal()),
+  match_count_text_->SetText(l10n_util::GetStringFUTF16(IDS_FIND_IN_PAGE_COUNT,
+      base::FormatNumber(result.active_match_ordinal()),
       base::FormatNumber(result.number_of_matches())));
 
   UpdateMatchCountAppearance(result.number_of_matches() == 0 &&
                              result.final_update());
 
   // The match_count label may have increased/decreased in size so we need to
   // do a layout and repaint the dialog so that the find text field doesn't
   // partially overlap the match-count label when it increases on no matches.
   Layout();
   SchedulePaint();
@@ skipping 181 matching lines @@
   separator_->SetColor(SkColorSetA(text_color, 0x26));
 
   const SkColor base_icon_color = GetNativeTheme()->GetSystemColor(
       ui::NativeTheme::kColorId_TextfieldDefaultColor);
   views::SetImageFromVectorIcon(find_previous_button_, kCaretUpIcon,
                                 base_icon_color);
   views::SetImageFromVectorIcon(find_next_button_, kCaretDownIcon,
                                 base_icon_color);
   views::SetImageFromVectorIcon(close_button_, ui::kCloseIcon, base_icon_color);
 }