V1 of material design 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 8d786443e9736bca0aefe812e5de53abc2c83742..f8fda8dd234deb2927aeb0d88996245a0841ac55 100644
--- a/chrome/browser/ui/views/find_bar_view.cc
+++ b/chrome/browser/ui/views/find_bar_view.cc
@@ -25,14 +25,23 @@
 #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/resource/material_design/material_design_controller.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/theme_provider.h"
 #include "ui/events/event.h"
 #include "ui/gfx/canvas.h"
+#include "ui/gfx/paint_vector_icon.h"
+#include "ui/gfx/vector_icons_public2.h"
+#include "ui/native_theme/common_theme.h"
+#include "ui/native_theme/native_theme.h"
 #include "ui/resources/grit/ui_resources.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/label.h"
+#include "ui/views/controls/separator.h"
+#include "ui/views/layout/box_layout.h"
 #include "ui/views/painter.h"
 #include "ui/views/widget/widget.h"
 
@@ -69,6 +78,26 @@ const SkColor kBackgroundColorNoMatch = SkColorSetRGB(255, 102, 102);
 // number brings the width on a "regular fonts" system to about 300px.
 const int kDefaultCharWidth = 43;
 
+// Creates a label that mouse click focus to a given target.

[sky, 2015/08/05 00:06:01]

This comment is missing a few words, and I was confused that the description in
no way matches the class name. In context the name makes sense, but the comment
doesn't help.

[Evan Stade, 2015/08/05 19:04:25]

Done.

+class MatchCountLabel : public views::Label {

[sky, 2015/08/05 00:06:01]

Rather than this, and FocusForwarderView, could we make the textfield go all the
way to the right edge, but set an inset so that it never renders over the match
count?

[Evan Stade, 2015/08/05 19:04:25]

that sounds more difficult because
a) we'd probably have to do explicit layout, instead of using a LayoutManager. I
think this is more error-prone.
b) The match count label changes width depending on its contents ("1 of 1" vs "1
of 1000"), so the inset would have to change as well.

I do think I've found a more comprehensive way to do this forwarding though by
using ViewTargeter[Delegate]. Unfortunately I still have to subclass Label to
give it event handling (note that the code we're commenting on here doesn't
actually work, as it also needs the CanProcessEventsWithinSubtree override). The
other option would be to add a boolean + setter to Label. Let me know if you
prefer that.

+ public:
+  explicit MatchCountLabel(views::View* target_view)
+      : target_view_(target_view) {
+    DCHECK(target_view);
+  }
+
+  // views::Label overrides:
+  bool OnMousePressed(const ui::MouseEvent& event) override {
+    target_view_->RequestFocus();
+    return true;
+  }
+
+ private:
+  views::View* target_view_;
+
+  DISALLOW_COPY_AND_ASSIGN(MatchCountLabel);
+};
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -82,36 +111,17 @@ FindBarView::FindBarView(FindBarHost* host)
       find_previous_button_(NULL),
       find_next_button_(NULL),
       close_button_(NULL) {
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-
   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_controller(this);
   find_text_->SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_FIND));
   find_text_->SetTextInputFlags(ui::TEXT_INPUT_FLAG_AUTOCORRECT_OFF);
-  // The find bar textfield has a background image instead of a border.
-  find_text_->SetBorder(views::Border::NullBorder());
   AddChildView(find_text_);
 
-  match_count_text_ = new views::Label();
-  AddChildView(match_count_text_);
-
-  // Create a focus forwarder view which sends focus to find_text_.
-  focus_forwarder_view_ = new FocusForwarderView(find_text_);
-  AddChildView(focus_forwarder_view_);
-
   find_previous_button_ = new views::ImageButton(this);
   find_previous_button_->set_tag(FIND_PREVIOUS_TAG);
   find_previous_button_->SetFocusable(true);
-  find_previous_button_->SetImage(views::CustomButton::STATE_NORMAL,
-      rb.GetImageSkiaNamed(IDR_FINDINPAGE_PREV));
-  find_previous_button_->SetImage(views::CustomButton::STATE_HOVERED,
-      rb.GetImageSkiaNamed(IDR_FINDINPAGE_PREV_H));
-  find_previous_button_->SetImage(views::CustomButton::STATE_PRESSED,
-      rb.GetImageSkiaNamed(IDR_FINDINPAGE_PREV_P));
-  find_previous_button_->SetImage(views::CustomButton::STATE_DISABLED,
-      rb.GetImageSkiaNamed(IDR_FINDINPAGE_PREV_D));
   find_previous_button_->SetTooltipText(
       l10n_util::GetStringUTF16(IDS_FIND_IN_PAGE_PREVIOUS_TOOLTIP));
   find_previous_button_->SetAccessibleName(
@@ -121,14 +131,6 @@ FindBarView::FindBarView(FindBarHost* host)
   find_next_button_ = new views::ImageButton(this);
   find_next_button_->set_tag(FIND_NEXT_TAG);
   find_next_button_->SetFocusable(true);
-  find_next_button_->SetImage(views::CustomButton::STATE_NORMAL,
-      rb.GetImageSkiaNamed(IDR_FINDINPAGE_NEXT));
-  find_next_button_->SetImage(views::CustomButton::STATE_HOVERED,
-      rb.GetImageSkiaNamed(IDR_FINDINPAGE_NEXT_H));
-  find_next_button_->SetImage(views::CustomButton::STATE_PRESSED,
-      rb.GetImageSkiaNamed(IDR_FINDINPAGE_NEXT_P));
-  find_next_button_->SetImage(views::CustomButton::STATE_DISABLED,
-      rb.GetImageSkiaNamed(IDR_FINDINPAGE_NEXT_D));
   find_next_button_->SetTooltipText(
       l10n_util::GetStringUTF16(IDS_FIND_IN_PAGE_NEXT_TOOLTIP));
   find_next_button_->SetAccessibleName(
@@ -138,12 +140,6 @@ FindBarView::FindBarView(FindBarHost* host)
   close_button_ = new views::ImageButton(this);
   close_button_->set_tag(CLOSE_TAG);
   close_button_->SetFocusable(true);
-  close_button_->SetImage(views::CustomButton::STATE_NORMAL,
-                          rb.GetImageSkiaNamed(IDR_CLOSE_1));
-  close_button_->SetImage(views::CustomButton::STATE_HOVERED,
-                          rb.GetImageSkiaNamed(IDR_CLOSE_1_H));
-  close_button_->SetImage(views::CustomButton::STATE_PRESSED,
-                          rb.GetImageSkiaNamed(IDR_CLOSE_1_P));
   close_button_->SetTooltipText(
       l10n_util::GetStringUTF16(IDS_FIND_IN_PAGE_CLOSE_TOOLTIP));
   close_button_->SetAccessibleName(
@@ -151,18 +147,12 @@ FindBarView::FindBarView(FindBarHost* host)
   close_button_->SetAnimationDuration(0);
   AddChildView(close_button_);
 
-  SetBackground(rb.GetImageSkiaNamed(IDR_FIND_DLG_LEFT_BACKGROUND),
-                rb.GetImageSkiaNamed(IDR_FIND_DLG_RIGHT_BACKGROUND));
-
-  SetBorderFromIds(
-      IDR_FIND_DIALOG_LEFT, IDR_FIND_DIALOG_MIDDLE, IDR_FIND_DIALOG_RIGHT);
-
-  preferred_height_ = rb.GetImageSkiaNamed(IDR_FIND_DIALOG_MIDDLE)->height();
-
-  static const int kImages[] = IMAGE_GRID(IDR_TEXTFIELD);
-  find_text_border_.reset(views::Painter::CreateImageGridPainter(kImages));
-
   EnableCanvasFlippingForRTLUI(true);
+
+  if (ui::MaterialDesignController::IsModeMaterial())
+    InitViewsForMaterial();
+  else
+    InitViewsForNonMaterial();
 }
 
 FindBarView::~FindBarView() {
@@ -243,6 +233,9 @@ void FindBarView::SetFocusAndSelection(bool select_all) {
 // FindBarView, views::View overrides:
 
 void FindBarView::OnPaint(gfx::Canvas* canvas) {
+  if (ui::MaterialDesignController::IsModeMaterial())
+    return views::View::OnPaint(canvas);
+
   // Paint drop down bar border and background.
   DropdownBarView::OnPaint(canvas);
 
@@ -278,7 +271,23 @@ void FindBarView::OnPaint(gfx::Canvas* canvas) {
                    match_count_text_->background_color());
 }
 
+void FindBarView::OnPaintBackground(gfx::Canvas* canvas) {
+  if (!ui::MaterialDesignController::IsModeMaterial())
+    return views::View::OnPaintBackground(canvas);
+
+  // Draw within the lines.
+  canvas->Save();
+  gfx::Rect bounds = GetLocalBounds();
+  bounds.Inset(border()->GetInsets());
+  canvas->ClipRect(bounds);
+  views::View::OnPaintBackground(canvas);
+  canvas->Restore();
+}
+
 void FindBarView::Layout() {
+  if (ui::MaterialDesignController::IsModeMaterial())
+    return views::View::Layout();
+
   int panel_width = GetPreferredSize().width();
 
   // Stay within view bounds.
@@ -347,6 +356,12 @@ void FindBarView::Layout() {
 }
 
 gfx::Size FindBarView::GetPreferredSize() const {
+  if (ui::MaterialDesignController::IsModeMaterial()) {
+    gfx::Size size = views::View::GetPreferredSize();
+    size.set_width(find_text_->GetPreferredSize().width());

[sky, 2015/08/05 00:06:01]

It's worth a comment as to why the preferred size doesn't include space for
anything else (maybe a pointer to the comment that sets the
set_default_width_in_chars).

[Evan Stade, 2015/08/05 19:04:25]

Done.

+    return size;
+  }
+
   gfx::Size prefsize = find_text_->GetPreferredSize();
   prefsize.set_height(preferred_height_);
 
@@ -440,6 +455,114 @@ void FindBarView::OnAfterPaste() {
   last_searched_text_.clear();
 }
 
+void FindBarView::InitViewsForNonMaterial() {
+  match_count_text_ = new views::Label();
+  AddChildView(match_count_text_);
+
+  // Create a focus forwarder view which sends focus to find_text_.
+  focus_forwarder_view_ = new FocusForwarderView(find_text_);
+  AddChildView(focus_forwarder_view_);
+
+  // The find bar textfield has a background image instead of a border.
+  find_text_->SetBorder(views::Border::NullBorder());
+
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  find_previous_button_->SetImage(views::CustomButton::STATE_NORMAL,
+                                  rb.GetImageSkiaNamed(IDR_FINDINPAGE_PREV));
+  find_previous_button_->SetImage(views::CustomButton::STATE_HOVERED,
+                                  rb.GetImageSkiaNamed(IDR_FINDINPAGE_PREV_H));
+  find_previous_button_->SetImage(views::CustomButton::STATE_PRESSED,
+                                  rb.GetImageSkiaNamed(IDR_FINDINPAGE_PREV_P));
+  find_previous_button_->SetImage(views::CustomButton::STATE_DISABLED,
+                                  rb.GetImageSkiaNamed(IDR_FINDINPAGE_PREV_D));
+
+  find_next_button_->SetImage(views::CustomButton::STATE_NORMAL,
+                              rb.GetImageSkiaNamed(IDR_FINDINPAGE_NEXT));
+  find_next_button_->SetImage(views::CustomButton::STATE_HOVERED,
+                              rb.GetImageSkiaNamed(IDR_FINDINPAGE_NEXT_H));
+  find_next_button_->SetImage(views::CustomButton::STATE_PRESSED,
+                              rb.GetImageSkiaNamed(IDR_FINDINPAGE_NEXT_P));
+  find_next_button_->SetImage(views::CustomButton::STATE_DISABLED,
+                              rb.GetImageSkiaNamed(IDR_FINDINPAGE_NEXT_D));
+
+  close_button_->SetImage(views::CustomButton::STATE_NORMAL,
+                          rb.GetImageSkiaNamed(IDR_CLOSE_1));
+  close_button_->SetImage(views::CustomButton::STATE_HOVERED,
+                          rb.GetImageSkiaNamed(IDR_CLOSE_1_H));
+  close_button_->SetImage(views::CustomButton::STATE_PRESSED,
+                          rb.GetImageSkiaNamed(IDR_CLOSE_1_P));
+
+  SetBackground(rb.GetImageSkiaNamed(IDR_FIND_DLG_LEFT_BACKGROUND),
+                rb.GetImageSkiaNamed(IDR_FIND_DLG_RIGHT_BACKGROUND));
+  SetBorderFromIds(IDR_FIND_DIALOG_LEFT, IDR_FIND_DIALOG_MIDDLE,
+                   IDR_FIND_DIALOG_RIGHT);
+
+  preferred_height_ = rb.GetImageSkiaNamed(IDR_FIND_DIALOG_MIDDLE)->height();
+
+  static const int kImages[] = IMAGE_GRID(IDR_TEXTFIELD);
+  find_text_border_.reset(views::Painter::CreateImageGridPainter(kImages));
+}
+
+void FindBarView::InitViewsForMaterial() {
+  // The background color is not used since there's no arrow.
+  // TODO(estade): this shadow is not quite the same as in the mocks.
+  SetBorder(make_scoped_ptr(new views::BubbleBorder(
+      views::BubbleBorder::NONE, views::BubbleBorder::SMALL_SHADOW,
+      SK_ColorGREEN)));
+
+  match_count_text_ = new MatchCountLabel(find_text_);
+  AddChildViewAt(match_count_text_, 1);
+
+  views::Separator* separator =
+      new views::Separator(views::Separator::VERTICAL);
+  // TODO(estade): refine these constants.
+  separator->SetColor(SkColorSetRGB(0xD5, 0xD5, 0xD5));
+  separator->SetBorder(views::Border::CreateEmptyBorder(4, 10, 4, 10));
+  AddChildViewAt(separator, 2);
+
+  find_text_->SetBorder(views::Border::NullBorder());
+  // Reserve horizontal space. Note that |find_text_| will only be allotted
+  // what space remains after other elements have been laid out, but this
+  // setting is still used when calculating the overall find bar's width.
+  find_text_->set_default_width_in_chars(30);
+
+  struct {
+    views::ImageButton* button;
+    gfx::VectorIconId id;
+  } button_images[] = {
+      {find_previous_button_, gfx::VectorIconId::FIND_PREV},
+      {find_next_button_, gfx::VectorIconId::FIND_NEXT},
+      {close_button_, gfx::VectorIconId::FIND_CLOSE},
+  };
+
+  SkColor grey;
+  ui::CommonThemeGetSystemColor(ui::NativeTheme::kColorId_ChromeIconGrey,
+                                &grey);
+  SkColor blue;
+  ui::CommonThemeGetSystemColor(ui::NativeTheme::kColorId_GoogleBlue, &blue);
+  for (size_t i = 0; i < arraysize(button_images); ++i) {
+    views::ImageButton* button = button_images[i].button;
+    button->SetImageAlignment(views::ImageButton::ALIGN_CENTER,
+                              views::ImageButton::ALIGN_MIDDLE);
+
+    gfx::ImageSkia image = gfx::CreateVectorIcon(button_images[i].id, 16, grey);
+    button->SetImage(views::CustomButton::STATE_NORMAL, &image);
+    image = gfx::CreateVectorIcon(button_images[i].id, 16, blue);
+    button->SetImage(views::CustomButton::STATE_PRESSED, &image);
+    // TODO(estade): I don't think the buttons are ever disabled, so this image
+    // is never visible. Consider removing this image or setting the buttons to
+    // disabled at sensible times.
+    image = gfx::CreateVectorIcon(button_images[i].id, 16,
+                                  SkColorSetA(grey, 0xff / 2));
+    button->SetImage(views::CustomButton::STATE_DISABLED, &image);
+  }
+
+  views::BoxLayout* manager =
+      new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0);
+  SetLayoutManager(manager);
+  manager->SetFlexForView(find_text_, 1);
+}
+
 void FindBarView::Find(const base::string16& search_text) {
   FindBarController* controller = find_bar_host()->GetFindBarController();
   DCHECK(controller);
@@ -502,6 +625,9 @@ const char* FindBarView::GetClassName() const {
 }
 
 void FindBarView::OnThemeChanged() {
+  if (ui::MaterialDesignController::IsModeMaterial())
+    return;
+
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   if (GetThemeProvider()) {
     close_button_->SetBackground(
@@ -510,3 +636,13 @@ void FindBarView::OnThemeChanged() {
         rb.GetImageSkiaNamed(IDR_CLOSE_1_MASK));
   }
 }
+
+void FindBarView::OnNativeThemeChanged(const ui::NativeTheme* theme) {
+  if (!ui::MaterialDesignController::IsModeMaterial())
+    return;
+
+  SkColor color =
+      theme->GetSystemColor(ui::NativeTheme::kColorId_DialogBackground);
+  set_background(views::Background::CreateSolidBackground(color));
+  match_count_text_->SetBackgroundColor(color);
+}