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..c784647e71a70188e054bf66ec5fd7da4bdc8e08 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,20 +10,45 @@
 #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(
+    const DesktopMediaSourceViewStyle& style) = default;
+
+DesktopMediaSourceViewStyle::DesktopMediaSourceViewStyle(
+    int columns,
+    const gfx::Size& item_size,
+    const gfx::Rect& label_rect,
+    gfx::HorizontalAlignment text_alignment,
+    const gfx::Rect& image_rect,
+    int selection_border_thickness,
+    int focus_rectangle_inset)
+    : columns(columns),
+      item_size(item_size),
+      label_rect(label_rect),
+      text_alignment(text_alignment),
+      image_rect(image_rect),
+      selection_border_thickness(selection_border_thickness),
+      focus_rectangle_inset(focus_rectangle_inset) {}
+
+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);
 }
 
@@ -59,11 +84,28 @@ void DesktopMediaSourceView::SetSelected(bool selected) {
       }
     }
 
+    const SkColor border_color = GetNativeTheme()->GetSystemColor(
+        ui::NativeTheme::kColorId_ButtonHighlightColor);

[msw, 2016/05/25 17:22:13]

Use consistent colors (menu item vs button). Use
kColorId_FocusedMenuItemBackgroundColor (like in Layout) or similar?

[qiangchen, 2016/05/25 20:27:30]

Done.

+    image_view_->SetBorder(views::Border::CreateSolidBorder(
+        style_.selection_border_thickness, border_color));
+    label_->SetFontList(label_->font_list().Derive(0, gfx::Font::BOLD));
+    parent_->OnSelectionChanged();
+  } else {
+    image_view_->SetBorder(views::Border::NullBorder());
+    label_->SetFontList(label_->font_list().Derive(0, gfx::Font::NORMAL));
+  }
+
+  SchedulePaint();
+}
+
+void DesktopMediaSourceView::SetHovered(bool hovered) {
+  hovered_ = hovered;
+
+  if (hovered) {
+    // Use background color to show mouse hover.
     const SkColor bg_color = GetNativeTheme()->GetSystemColor(
-        ui::NativeTheme::kColorId_FocusedMenuItemBackgroundColor);
+        ui::NativeTheme::kColorId_HoverMenuItemBackgroundColor);
     set_background(views::Background::CreateSolidBackground(bg_color));
-
-    parent_->OnSelectionChanged();
   } else {
     set_background(NULL);
   }
@@ -76,15 +118,16 @@ 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_rect);
+  if (selected_) {

[msw, 2016/05/25 17:22:13]

Remove this block; SetSelected already applies an |image_view_| border.

[qiangchen, 2016/05/25 20:27:30]

We need this. When the single/multiple screen style changes, SetSelected() will
not be called, and the thickness of the border will follow old style.

Or do you think it is better to move this part of code in SetStyle()?

[msw, 2016/05/26 20:50:33]

Yeah, change DesktopMediaSourceView::set_style into a full-fledged function
DesktopMediaSourceView::SetStyle. Make SetStyle do everything that is currently
in Layout(), and remove the Layout override.

[qiangchen, 2016/05/26 21:38:37]

Done.

+    const SkColor border_color = GetNativeTheme()->GetSystemColor(
+        ui::NativeTheme::kColorId_FocusedMenuItemBackgroundColor);
+    image_view_->SetBorder(views::Border::CreateSolidBorder(
+        style_.selection_border_thickness, border_color));
+  }
+

[msw, 2016/05/25 17:22:13]

nit: remove blank line

[qiangchen, 2016/05/25 20:27:30]

Done.

+  label_->SetBoundsRect(style_.label_rect);
+  label_->SetHorizontalAlignment(style_.text_alignment);
 }
 
 views::View* DesktopMediaSourceView::GetSelectedViewForGroup(int group) {
@@ -112,8 +155,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_.focus_rectangle_inset, style_.focus_rectangle_inset);
     canvas->DrawFocusRect(bounds);
   }
 }
@@ -142,6 +184,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) {