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));
   AddChildViewAt(separator_, 2);
 
+  ChromeLayoutProvider* provider = ChromeLayoutProvider::Get();
+
+  find_next_button_->SetProperty(views::kMarginsKey, new gfx::Insets(0,

[Peter Kasting, 2017/07/13 04:52:47]

For insets, vertical comes first, then horizontal (lots of places)

[ananta, 2017/07/13 05:22:10]

Yeah. I looked at it after I sent out the patch. Sorry about that.

+      provider->GetDistanceMetric(DISTANCE_TOAST_BUTTON_VERTICAL)));
+  close_button_->SetProperty(views::kMarginsKey, new gfx::Insets(0,
+      provider->GetDistanceMetric(DISTANCE_TOAST_BUTTON_VERTICAL)));
+  find_text_->SetProperty(views::kMarginsKey, new gfx::Insets(0,
+      provider->GetDistanceMetric(DISTANCE_TOAST_TEXT_VERTICAL)));
+  match_count_text_->SetProperty(views::kMarginsKey, new gfx::Insets(0,
+      provider->GetDistanceMetric(DISTANCE_TOAST_LABEL_VERTICAL)));
+
   find_text_->SetBorder(views::NullBorder());
 
   views::BoxLayout* manager =
       new views::BoxLayout(views::BoxLayout::kHorizontal,
-                           gfx::Insets(kInteriorPadding), kInterChildSpacing);
+                           provider->GetInsetsMetric(

[Peter Kasting, 2017/07/13 04:52:47]

This shouldn't have dialog contents insets, as that puts vertical insets atop
the dialog.

It should just have insets of 16 horizontal and 0 vertical.

[ananta, 2017/07/13 05:22:10]

Thanks. I used views::INSETS_DIALOG_CONTENTS).left(). Could not find a constant
matching 16 there.

[Peter Kasting, 2017/07/13 05:31:37]

I would probably just add an INSETS_TOAST in the layout provider that is (0,
16).

[ananta, 2017/07/13 05:43:48]

Done.

+                              views::INSETS_DIALOG_CONTENTS),
+                           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 233 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);
 }