Desktop Capture Picker New UI: Non Mac Appearance Change

chrome/browser/ui/views/desktop_capture/desktop_media_source_view.cc

Index: chrome/browser/ui/views/desktop_capture/desktop_media_source_view.cc
diff --git a/chrome/browser/ui/views/desktop_capture/desktop_media_source_view.cc b/chrome/browser/ui/views/desktop_capture/desktop_media_source_view.cc
index 02c9e68858b2b2eb353ba261867873b97aa7684c..28b3042019cbb93324ca374c7b21e0dce2feeb75 100644
--- a/chrome/browser/ui/views/desktop_capture/desktop_media_source_view.cc
+++ b/chrome/browser/ui/views/desktop_capture/desktop_media_source_view.cc
@@ -10,28 +10,76 @@
 #include "ui/gfx/canvas.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/background.h"
+#include "ui/views/border.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 
 using content::DesktopMediaID;
 
-DesktopMediaSourceView::DesktopMediaSourceView(DesktopMediaListView* parent,
-                                               DesktopMediaID source_id)
+DesktopMediaSourceViewStyle::DesktopMediaSourceViewStyle()
+    : columns(0),
+      text_alignment(gfx::HorizontalAlignment::ALIGN_CENTER),
+      selection_border_thickness(0),
+      paint_inset_horizontal(0),
+      paint_inset_vertical(0) {}
+
+DesktopMediaSourceViewStyle::DesktopMediaSourceViewStyle(
+    const DesktopMediaSourceViewStyle& style) {
+  columns = style.columns;
+  item_size = style.item_size;
+  image_position = style.image_position;
+  selection_border_thickness = style.selection_border_thickness;
+  label_position = style.label_position;
+  text_alignment = style.text_alignment;
+  paint_inset_horizontal = style.paint_inset_horizontal;
+  paint_inset_vertical = style.paint_inset_vertical;
+}
+
+DesktopMediaSourceViewStyle::DesktopMediaSourceViewStyle(
+    int columns,
+    const gfx::Size& item_size,
+    const gfx::Rect& label_position,
+    gfx::HorizontalAlignment text_alignment,
+    const gfx::Rect& image_position,
+    int selection_border_thickness,
+    int paint_inset_horizontal,
+    int paint_inset_vertical)
+    : columns(columns),
+      item_size(item_size),
+      label_position(label_position),
+      text_alignment(text_alignment),
+      image_position(image_position),
+      selection_border_thickness(selection_border_thickness),
+      paint_inset_horizontal(paint_inset_horizontal),
+      paint_inset_vertical(paint_inset_vertical) {}
+
+DesktopMediaSourceView::DesktopMediaSourceView(
+    DesktopMediaListView* parent,
+    DesktopMediaID source_id,
+    DesktopMediaSourceViewStyle style)
     : parent_(parent),
       source_id_(source_id),
+      style_(style),
       image_view_(new views::ImageView()),
       label_(new views::Label()),
       selected_(false) {
   AddChildView(image_view_);
   AddChildView(label_);
+  image_view_->set_interactive(false);
+
   SetFocusBehavior(FocusBehavior::ALWAYS);
 }
 
 DesktopMediaSourceView::~DesktopMediaSourceView() {}
 
+

[msw, 2016/05/20 23:26:37]

nit: remove

[qiangchen, 2016/05/24 00:03:37]

Done.

 const char* DesktopMediaSourceView::kDesktopMediaSourceViewClassName =
     "DesktopMediaPicker_DesktopMediaSourceView";
 
+void DesktopMediaSourceView::SetStyle(DesktopMediaSourceViewStyle style) {

[msw, 2016/05/20 23:26:37]

nit: make this a simple accessor, defined in the header:
  void set_style(const DesktopMediaSourceViewStyle& style) { style_ = style; }

[qiangchen, 2016/05/24 00:03:37]

Done.

+  style_ = style;
+}
+
 void DesktopMediaSourceView::SetName(const base::string16& name) {
   label_->SetText(name);
 }
@@ -59,12 +107,35 @@ void DesktopMediaSourceView::SetSelected(bool selected) {
       }
     }
 
-    const SkColor bg_color = GetNativeTheme()->GetSystemColor(
+    const SkColor border_color = GetNativeTheme()->GetSystemColor(
         ui::NativeTheme::kColorId_FocusedMenuItemBackgroundColor);
-    set_background(views::Background::CreateSolidBackground(bg_color));
-
+    image_view_->SetBorder(views::Border::CreateSolidBorder(
+        style_.selection_border_thickness, border_color));
+    gfx::Font font =
+        label_->font_list().GetPrimaryFont().Derive(0, gfx::Font::BOLD);
+    label_->SetFontList(gfx::FontList(font));

[msw, 2016/05/20 23:26:37]

nit: label_->SetFontList(label_->font_list().Derive(0, gfx::Font::BOLD));

[qiangchen, 2016/05/24 00:03:37]

Done.

     parent_->OnSelectionChanged();
   } else {
+    image_view_->SetBorder(views::Border::NullBorder());
+    gfx::Font font =
+        label_->font_list().GetPrimaryFont().Derive(0, gfx::Font::NORMAL);
+    label_->SetFontList(gfx::FontList(font));

[msw, 2016/05/20 23:26:37]

nit: label_->SetFontList(label_->font_list().Derive(0, gfx::Font::NORMAL));

[qiangchen, 2016/05/24 00:03:37]

Done.

+  }
+
+  SchedulePaint();
+}
+
+void DesktopMediaSourceView::SetHovered(bool hovered) {
+  if (hovered == hovered_)

[msw, 2016/05/20 23:26:37]

nit: tracking hovered_ to early return here probably isn't necessary.

[qiangchen, 2016/05/24 00:03:37]

Done.

+    return;
+
+  hovered_ = hovered;
+
+  if (hovered) {
+    // Make the background a little darker.
+    const SkColor bg_color = 0x44000000;

[msw, 2016/05/20 23:26:37]

Use kColorId_HoverMenuItemBackgroundColor

[qiangchen, 2016/05/24 00:03:37]

Done.

But I found HoverMenuButtonColor looks better.

[msw, 2016/05/24 18:54:25]

Please post pictures of each, it seems odd to mix menu and button styles here.

[qiangchen, 2016/05/25 16:24:08]

I'll set color for the scroll_view's background to menubackground, which will
make things consistent.

+    set_background(views::Background::CreateSolidBackground(bg_color));
+  } else {
     set_background(NULL);
   }
 
@@ -76,15 +147,9 @@ const char* DesktopMediaSourceView::GetClassName() const {
 }
 
 void DesktopMediaSourceView::Layout() {
-  image_view_->SetBounds(DesktopMediaPickerDialogView::kThumbnailMargin,
-                         DesktopMediaPickerDialogView::kThumbnailMargin,
-                         DesktopMediaPickerDialogView::kThumbnailWidth,
-                         DesktopMediaPickerDialogView::kThumbnailHeight);
-  label_->SetBounds(DesktopMediaPickerDialogView::kThumbnailMargin,
-                    DesktopMediaPickerDialogView::kThumbnailHeight +
-                        DesktopMediaPickerDialogView::kThumbnailMargin,
-                    DesktopMediaPickerDialogView::kThumbnailWidth,
-                    DesktopMediaPickerDialogView::kLabelHeight);
+  image_view_->SetBoundsRect(style_.image_position);
+  label_->SetBoundsRect(style_.label_position);
+  label_->SetHorizontalAlignment(style_.text_alignment);
 }
 
 views::View* DesktopMediaSourceView::GetSelectedViewForGroup(int group) {
@@ -112,8 +177,7 @@ void DesktopMediaSourceView::OnPaint(gfx::Canvas* canvas) {
   View::OnPaint(canvas);
   if (HasFocus()) {
     gfx::Rect bounds(GetLocalBounds());
-    bounds.Inset(DesktopMediaPickerDialogView::kThumbnailMargin / 2,
-                 DesktopMediaPickerDialogView::kThumbnailMargin / 2);
+    bounds.Inset(style_.paint_inset_horizontal, style_.paint_inset_vertical);
     canvas->DrawFocusRect(bounds);
   }
 }
@@ -142,6 +206,14 @@ bool DesktopMediaSourceView::OnMousePressed(const ui::MouseEvent& event) {
   return true;
 }
 
+void DesktopMediaSourceView::OnMouseEntered(const ui::MouseEvent& event) {
+  SetHovered(true);
+}
+
+void DesktopMediaSourceView::OnMouseExited(const ui::MouseEvent& event) {
+  SetHovered(false);
+}
+
 void DesktopMediaSourceView::OnGestureEvent(ui::GestureEvent* event) {
   if (event->type() == ui::ET_GESTURE_TAP &&
       event->details().tap_count() == 2) {