MacViews: support backgrounds for selected rows

ui/views/controls/tree/tree_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 "ui/views/controls/tree/tree_view.h"
 
 #include <algorithm>
 
 #include "base/i18n/rtl.h"
 #include "base/message_loop/message_loop.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/base/ime/input_method.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/events/event.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/rect_conversions.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/skia_util.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/resources/grit/ui_resources.h"
 #include "ui/views/controls/prefix_selector.h"
 #include "ui/views/controls/scroll_view.h"
 #include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/controls/tree/tree_view_controller.h"
 #include "ui/views/resources/grit/views_resources.h"
+#include "ui/views/style/platform_style.h"
 
 using ui::TreeModel;
 using ui::TreeModelNode;
 
 namespace views {
 
 // Insets around the view.
 static const int kHorizontalInset = 2;
 static const int kVerticalInset = 2;
 // Padding before/after the image.
@@ skipping 23 matching lines @@
 // and |is_selected| is true if the item is selected.
 ui::NativeTheme::ColorId text_color_id(bool has_focus, bool is_selected) {
   if (is_selected) {
     if (has_focus)
       return ui::NativeTheme::kColorId_TreeSelectedText;
     return ui::NativeTheme::kColorId_TreeSelectedTextUnfocused;
   }
   return ui::NativeTheme::kColorId_TreeText;
 }
 
+bool EventIsDoubleTapOrClick(const ui::LocatedEvent& event) {
+  if (event.type() == ui::ET_GESTURE_TAP) {
+    const auto& gesture = static_cast<const ui::GestureEvent&>(event);
+    return gesture.details().tap_count() == 2;

[tapted, 2016/06/13 07:03:47]

I think we can skip the `auto` and do

    return event.AsGestureEvent().details().tap_count() == 2;

[Elly Fong-Jones, 2016/06/14 17:21:27]

Done.

+  } else {

[tapted, 2016/06/13 07:03:48]

nit: "no else after return" - last item at
https://www.chromium.org/developers/coding-style#TOC-Code-formatting

[Elly Fong-Jones, 2016/06/14 17:21:27]

Done.

+    return event.flags() & ui::EF_IS_DOUBLE_CLICK;
+  }
+}
+
 }  // namespace
 
 TreeView::TreeView()
     : model_(NULL),
       selected_node_(NULL),
       editing_(false),
       editor_(NULL),
       focus_manager_(NULL),
       auto_expand_children_(false),
       editable_(true),
@@ skipping 304 matching lines @@
 
 void TreeView::ShowContextMenu(const gfx::Point& p,
                                ui::MenuSourceType source_type) {
   if (!model_)
     return;
   if (source_type == ui::MENU_SOURCE_MOUSE) {
     // Only invoke View's implementation (which notifies the
     // ContextMenuController) if over a node.
     gfx::Point local_point(p);
     ConvertPointFromScreen(this, &local_point);
-    int row = (local_point.y() - kVerticalInset) / row_height_;
-    int depth = 0;
-    InternalNode* node = GetNodeByRow(row, &depth);
-    if (!node)
-      return;
-    gfx::Rect bounds(GetBoundsForNodeImpl(node, row, depth));
-    if (!bounds.Contains(local_point))
+    if (!NodeAtPoint(local_point))

[tapted, 2016/06/13 07:03:47]

I think it would improve NodeAtPoint if it required a non-null depth - then we
just end up with an extra line here like `int ignored_depth;` rather than the
extra stuff in NodeAtPoint

[Elly Fong-Jones, 2016/06/14 17:21:27]

Done.

       return;
   }
   View::ShowContextMenu(p, source_type);
 }
 
 void TreeView::GetAccessibleState(ui::AXViewState* state) {
   state->role = ui::AX_ROLE_TREE;
   state->AddStateFlag(ui::AX_STATE_READ_ONLY);
   if (!selected_node_)
     return;
@@ skipping 221 matching lines @@
     GetInputMethod()->DetachTextInputClient(GetPrefixSelector());
   SchedulePaintForNode(selected_node_);
   if (selector_)
     selector_->OnViewBlur();
 }
 
 bool TreeView::OnClickOrTap(const ui::LocatedEvent& event) {
   CommitEdit();
   RequestFocus();
 
-  int row = (event.y() - kVerticalInset) / row_height_;
-  int depth = 0;
-  InternalNode* node = GetNodeByRow(row, &depth);
-  if (node) {
-    gfx::Rect bounds(GetBoundsForNodeImpl(node, row, depth));
-    if (bounds.Contains(event.location())) {
-      int relative_x = event.x() - bounds.x();
-      if (base::i18n::IsRTL())
-        relative_x = bounds.width() - relative_x;
-      if (relative_x < kArrowRegionSize &&
-          model_->GetChildCount(node->model_node())) {
-        if (node->is_expanded())
-          Collapse(node->model_node());
-        else
-          Expand(node->model_node());
-      } else if (relative_x > kArrowRegionSize) {
-        SetSelectedNode(node->model_node());
-        bool should_toggle = false;
-        if (event.type() == ui::ET_GESTURE_TAP) {
-          const ui::GestureEvent& gesture =
-              static_cast<const ui::GestureEvent&>(event);
-          should_toggle = gesture.details().tap_count() == 2;
-        } else {
-          should_toggle = (event.flags() & ui::EF_IS_DOUBLE_CLICK) != 0;
-        }
-        if (should_toggle) {
-          if (node->is_expanded())
-            Collapse(node->model_node());
-          else
-            Expand(node->model_node());
-        }
-      }
-    }
+  int depth;
+  InternalNode* node = NodeAtPoint(event.location(), &depth);
+  if (!node)
+    return true;
+
+  bool hits_arrow = HitsNodeArrow(node, depth, event.location());
+  if (!hits_arrow)
+    SetSelectedNode(node->model_node());
+
+  if (hits_arrow || EventIsDoubleTapOrClick(event)) {
+    if (node->is_expanded())
+      Collapse(node->model_node());
+    else
+      Expand(node->model_node());
   }
   return true;
 }
 
 void TreeView::LoadChildren(InternalNode* node) {
   DCHECK_EQ(0, node->child_count());
   DCHECK(!node->loaded_children());
   node->set_loaded_children(true);
   for (int i = 0, child_count = model_->GetChildCount(node->model_node());
        i < child_count; ++i) {
@@ skipping 49 matching lines @@
                    -(empty_editor_size_.height() - font_list_.GetHeight()) / 2);
   // Give a little extra space for editing.
   row_bounds.set_width(row_bounds.width() + 50);
   editor_->SetBoundsRect(row_bounds);
   editor_->Layout();
 }
 
 void TreeView::SchedulePaintForNode(InternalNode* node) {
   if (!node)
     return;  // Explicitly allow NULL to be passed in.
-  SchedulePaintInRect(GetBoundsForNode(node));
+  SchedulePaintInRect(GetBackgroundBoundsForNode(node));
 }
 
 void TreeView::PaintRows(gfx::Canvas* canvas,
                          int min_row,
                          int max_row,
                          InternalNode* node,
                          int depth,
                          int* row) {
   if (*row >= max_row)
     return;
 
   if (*row >= min_row && *row < max_row)
     PaintRow(canvas, node, *row, depth);
   (*row)++;
   if (!node->is_expanded())
     return;
   depth++;
   for (int i = 0; i < node->child_count() && *row < max_row; ++i)
     PaintRows(canvas, min_row, max_row, node->GetChild(i), depth, row);
 }
 
 void TreeView::PaintRow(gfx::Canvas* canvas,
                         InternalNode* node,
                         int row,
                         int depth) {
   gfx::Rect bounds(GetBoundsForNodeImpl(node, row, depth));
+  const SkColor selected_row_bg_color = GetNativeTheme()->GetSystemColor(
+      text_background_color_id(HasFocus() || editing_));
+
+  // Paint the row background.
+  if (PlatformStyle::kTreeViewSelectsEntireRow && selected_node_ == node)
+    canvas->FillRect(GetBackgroundBoundsForNode(node), selected_row_bg_color);
 
   if (model_->GetChildCount(node->model_node()))
     PaintExpandControl(canvas, bounds, node->is_expanded());

[tapted, 2016/06/13 07:03:47]

We probably need to convert this to a vector icon so we can control the color
it's painted, but that's good for a follow-up.

[Elly Fong-Jones, 2016/06/14 17:21:27]

Acknowledged.

 
   // Paint the icon.
   gfx::ImageSkia icon;
   int icon_index = model_->GetIconIndex(node->model_node());
   if (icon_index != -1)
     icon = icons_[icon_index];
   else if (node == selected_node_)
     icon = open_icon_;
   else
     icon = closed_icon_;
   int icon_x = kArrowRegionSize + kImagePadding +
                (open_icon_.width() - icon.width()) / 2;
   if (base::i18n::IsRTL())
     icon_x = bounds.right() - icon_x - open_icon_.width();
   else
     icon_x += bounds.x();
   canvas->DrawImageInt(
       icon, icon_x,
       bounds.y() + (bounds.height() - icon.height()) / 2);
 
-  if (!editing_ || node != selected_node_) {
-    gfx::Rect text_bounds(bounds.x() + text_offset_, bounds.y(),
-                          bounds.width() - text_offset_, bounds.height());
-    if (base::i18n::IsRTL())
-      text_bounds.set_x(bounds.x());
-    if (node == selected_node_) {
-      const SkColor bg_color = GetNativeTheme()->GetSystemColor(
-          text_background_color_id(HasFocus()));
-      canvas->FillRect(text_bounds, bg_color);
-      if (HasFocus())
-        canvas->DrawFocusRect(text_bounds);
-    }
-    const ui::NativeTheme::ColorId color_id =
-        text_color_id(HasFocus(), node == selected_node_);
-    const gfx::Rect internal_bounds(
-        text_bounds.x() + kTextHorizontalPadding,
-        text_bounds.y() + kTextVerticalPadding,
-        text_bounds.width() - kTextHorizontalPadding * 2,
-        text_bounds.height() - kTextVerticalPadding * 2);
-    canvas->DrawStringRect(node->model_node()->GetTitle(), font_list_,
-                           GetNativeTheme()->GetSystemColor(color_id),
-                           internal_bounds);
+  // Paint the text background and text. In edit mode, the selected node is a
+  // separate editing control, so it does not need to be painted here.
+  if (editing_ && selected_node_ == node)
+    return;
+
+  gfx::Rect text_bounds(bounds.x() + text_offset_, bounds.y(),
+                        bounds.width() - text_offset_, bounds.height());
+  if (base::i18n::IsRTL())
+    text_bounds.set_x(bounds.x());
+
+  // Paint the background on the selected row.
+  if (!PlatformStyle::kTreeViewSelectsEntireRow && node == selected_node_) {
+    canvas->FillRect(text_bounds, selected_row_bg_color);
+    if (HasFocus())
+      canvas->DrawFocusRect(text_bounds);
   }
+
+  // Paint the text.
+  const ui::NativeTheme::ColorId color_id =
+      text_color_id(HasFocus(), node == selected_node_);
+  const gfx::Rect internal_bounds(
+      text_bounds.x() + kTextHorizontalPadding,
+      text_bounds.y() + kTextVerticalPadding,
+      text_bounds.width() - kTextHorizontalPadding * 2,
+      text_bounds.height() - kTextVerticalPadding * 2);
+  canvas->DrawStringRect(node->model_node()->GetTitle(), font_list_,
+                         GetNativeTheme()->GetSystemColor(color_id),
+                         internal_bounds);
 }
 
 void TreeView::PaintExpandControl(gfx::Canvas* canvas,
                                   const gfx::Rect& node_bounds,
                                   bool expanded) {
   int center_x;
   if (base::i18n::IsRTL()) {
     center_x = node_bounds.right() - kArrowRegionSize +
                (kArrowRegionSize - 4) / 2;
   } else {
@@ skipping 30 matching lines @@
     return NULL;
   if (!parent_internal_node->loaded_children()) {
     if (create_type == DONT_CREATE_IF_NOT_LOADED)
       return NULL;
     LoadChildren(parent_internal_node);
   }
   return parent_internal_node->GetChild(
       model_->GetIndexOf(parent_internal_node->model_node(), model_node));
 }
 
+gfx::Rect TreeView::GetBackgroundBoundsForNode(InternalNode* node) {
+  gfx::Rect row_bounds(GetBoundsForNode(node));
+  if (PlatformStyle::kTreeViewSelectsEntireRow) {
+    row_bounds.set_x(bounds().x());
+    row_bounds.set_width(bounds().width());
+  }
+  return row_bounds;
+}
+
 gfx::Rect TreeView::GetBoundsForNode(InternalNode* node) {

[tapted, 2016/06/13 07:03:47]

perhaps we should rename these to GetTextBoundsForNode[Impl]?

[Elly Fong-Jones, 2016/06/14 17:21:27]

Done.

   int row, depth;
   row = GetRowForInternalNode(node, &depth);
   return GetBoundsForNodeImpl(node, row, depth);
 }
 
 gfx::Rect TreeView::GetBoundsForNodeImpl(InternalNode* node,
                                          int row,
                                          int depth) {
   gfx::Rect rect(depth * kIndent + kHorizontalInset,
                  row * row_height_ + kVerticalInset,
@@ skipping 125 matching lines @@
   }
   return return_value;
 }
 
 PrefixSelector* TreeView::GetPrefixSelector() {
   if (!selector_)
     selector_.reset(new PrefixSelector(this));
   return selector_.get();
 }
 
+TreeView::InternalNode* TreeView::NodeAtPoint(const gfx::Point& point,
+                                              int* depth) {
+  int row = (point.y() - kVerticalInset) / row_height_;
+  int local_depth = 0;
+  InternalNode* node = GetNodeByRow(row, &local_depth);
+  if (!node)
+    return nullptr;
+
+  // If the entire row gets a selected background, clicking anywhere in the row
+  // serves to hit this node.
+  if (PlatformStyle::kTreeViewSelectsEntireRow) {
+    if (depth)
+      *depth = local_depth;
+    return node;
+  }
+  gfx::Rect bounds(GetBoundsForNodeImpl(node, row, local_depth));
+  if (bounds.Contains(point)) {
+    if (depth)
+      *depth = local_depth;
+    return node;
+  }
+  return nullptr;
+}
+
+bool TreeView::HitsNodeArrow(InternalNode* node,
+                             int depth,
+                             const gfx::Point& point) {
+  if (!model_->GetChildCount(node->model_node()))
+    return false;

[tapted, 2016/06/13 07:03:48]

nit: blank line after

[Elly Fong-Jones, 2016/06/14 17:21:27]

Done.

+  gfx::Rect bounds(GetBackgroundBoundsForNode(node));
+  int arrow_dx = depth * kIndent + kHorizontalInset;
+  gfx::Rect arrow_bounds(bounds.x() + arrow_dx, bounds.y(), kArrowRegionSize,

[tapted, 2016/06/13 07:03:48]

I don't think we can use bounds.x() here -- GetBackgroundBoundsForNode() returns
different a different x value for kTreeViewSelectsEntireRow, but the arrow
should still be in the same place, regardless of kTreeViewSelectsEntireRow

(it seems ok on mac, so maybe this has regressed on other platforms)

It's possible the fix is just `bounds.x()` -> 'bounds().x()` but that's a bit
too subtle. - We should rename the |bounds| local variable to row_bounds.

Or, perhaps best, just use 

  int row, depth;
  row = GetRowForInternalNode(node, &depth);
  int arrow_dx = ..
  gfx::Rect arrow_bounds(/* x calc */, row * row_height_ + kVerticalInset,
                         kArrowRegionSize, row_height_);
  

i.e. don't pass in |depth| to the function -- GetBackgroundBoundsForNode is
currently calculating it anyway.

[Elly Fong-Jones, 2016/06/14 17:21:27]

Done.

+                         bounds.height());
+  if (base::i18n::IsRTL())
+    arrow_bounds.set_x(bounds.width() - arrow_dx - kArrowRegionSize);
+  return arrow_bounds.Contains(point);
+}
+
 // InternalNode ----------------------------------------------------------------
 
 TreeView::InternalNode::InternalNode()
     : model_node_(NULL),
       loaded_children_(false),
       is_expanded_(false),
       text_width_(0) {
 }
 
 TreeView::InternalNode::~InternalNode() {
@@ skipping 20 matching lines @@
   if (!is_expanded_)
     return max_width;
   for (int i = 0; i < child_count(); ++i) {
     max_width = std::max(max_width,
                          GetChild(i)->GetMaxWidth(indent, depth + 1));
   }
   return max_width;
 }
 
 }  // namespace views