MacViews: support backgrounds for selected rows

ui/views/controls/tree/tree_view.cc

Index: ui/views/controls/tree/tree_view.cc
diff --git a/ui/views/controls/tree/tree_view.cc b/ui/views/controls/tree/tree_view.cc
index 70501f872adda1fe37dfcd84d55e308b35aa6071..7d5c86cda8af20277bba9914edc693a65c319d05 100644
--- a/ui/views/controls/tree/tree_view.cc
+++ b/ui/views/controls/tree/tree_view.cc
@@ -25,6 +25,7 @@
 #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;
@@ -68,6 +69,12 @@ ui::NativeTheme::ColorId text_color_id(bool has_focus, bool is_selected) {
   return ui::NativeTheme::kColorId_TreeText;
 }
 
+bool EventIsDoubleTapOrClick(const ui::LocatedEvent& event) {
+  if (event.type() == ui::ET_GESTURE_TAP)
+    return event.AsGestureEvent()->details().tap_count() == 2;
+  return event.flags() & ui::EF_IS_DOUBLE_CLICK;
+}
+
 }  // namespace
 
 TreeView::TreeView()
@@ -243,7 +250,7 @@ void TreeView::SetSelectedNode(TreeModelNode* model_node) {
   }
 
   if (selected_node_)
-    ScrollRectToVisible(GetBoundsForNode(selected_node_));
+    ScrollRectToVisible(GetTextBoundsForNode(selected_node_));
 
   // Notify controller if the old selection was empty to handle the case of
   // remove explicitly resetting selected_node_ before invoking this.
@@ -392,13 +399,7 @@ void TreeView::ShowContextMenu(const gfx::Point& p,
     // 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))
       return;
   }
   View::ShowContextMenu(p, source_type);
@@ -533,7 +534,7 @@ base::string16 TreeView::GetTextForRow(int row) {
 gfx::Point TreeView::GetKeyboardContextMenuLocation() {
   int y = height() / 2;
   if (selected_node_) {
-    gfx::Rect node_bounds(GetBoundsForNode(selected_node_));
+    gfx::Rect node_bounds(GetTextBoundsForNode(selected_node_));
     gfx::Rect vis_bounds(GetVisibleBounds());
     if (node_bounds.y() >= vis_bounds.y() &&
         node_bounds.y() < vis_bounds.bottom()) {
@@ -640,39 +641,19 @@ 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());
-        }
-      }
-    }
+  InternalNode* node = NodeAtPoint(event.location());
+  if (!node)
+    return true;
+
+  bool hits_arrow = HitsNodeArrow(node, 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;
 }
@@ -725,7 +706,7 @@ void TreeView::LayoutEditor() {
 
   DCHECK(selected_node_);
   // Position the editor so that its text aligns with the text we drew.
-  gfx::Rect row_bounds = GetBoundsForNode(selected_node_);
+  gfx::Rect row_bounds = GetTextBoundsForNode(selected_node_);
   row_bounds.set_x(
       GetMirroredXWithWidthInView(row_bounds.x(), row_bounds.width()));
   row_bounds.set_x(row_bounds.x() + text_offset_);
@@ -742,7 +723,7 @@ void TreeView::LayoutEditor() {
 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,
@@ -768,7 +749,13 @@ void TreeView::PaintRow(gfx::Canvas* canvas,
                         InternalNode* node,
                         int row,
                         int depth) {
-  gfx::Rect bounds(GetBoundsForNodeImpl(node, row, depth));
+  gfx::Rect bounds(GetTextBoundsForNodeImpl(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());
@@ -792,29 +779,34 @@ void TreeView::PaintRow(gfx::Canvas* canvas,
       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(),

[tapted, 2016/06/15 03:17:49]

gah - I didn't notice this. That confuses things. i.e. having
`GetTextBoundsForNodeImpl` not include |text_offset_|. Perhaps if we state in
the comment for `GetTextBoundsForNodeImpl` that it includes |text_offset_|, and
here call |text_bounds| -> |drawn_text_bounds| or something.

Otherwise we need to come up with a better name for GetTextBoundsForNodeImpl,
but everything I can think of makes things more confusing.

[Elly Fong-Jones, 2016/06/16 14:42:34]

Argh, I didn't spot that either.

I introduced GetForegroundBoundsForNode, converted most uses of GetTextBounds
over to GetForegroundBounds, then made this use GetTextBounds directly, and
everything seems to work okay.

+                        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,
@@ -865,13 +857,22 @@ TreeView::InternalNode* TreeView::GetInternalNodeForModelNode(
       model_->GetIndexOf(parent_internal_node->model_node(), model_node));
 }
 
-gfx::Rect TreeView::GetBoundsForNode(InternalNode* node) {
+gfx::Rect TreeView::GetBackgroundBoundsForNode(InternalNode* node) {
+  gfx::Rect row_bounds(GetTextBoundsForNode(node));
+  if (PlatformStyle::kTreeViewSelectsEntireRow) {
+    row_bounds.set_x(bounds().x());
+    row_bounds.set_width(bounds().width());
+  }
+  return row_bounds;
+}
+
+gfx::Rect TreeView::GetTextBoundsForNode(InternalNode* node) {
   int row, depth;
   row = GetRowForInternalNode(node, &depth);
-  return GetBoundsForNodeImpl(node, row, depth);
+  return GetTextBoundsForNodeImpl(node, row, depth);
 }
 
-gfx::Rect TreeView::GetBoundsForNodeImpl(InternalNode* node,
+gfx::Rect TreeView::GetTextBoundsForNodeImpl(InternalNode* node,
                                          int row,
                                          int depth) {
   gfx::Rect rect(depth * kIndent + kHorizontalInset,
@@ -1011,6 +1012,39 @@ PrefixSelector* TreeView::GetPrefixSelector() {
   return selector_.get();
 }
 
+TreeView::InternalNode* TreeView::NodeAtPoint(const gfx::Point& point) {
+  int row = (point.y() - kVerticalInset) / row_height_;
+  int depth = -1;
+  InternalNode* node = GetNodeByRow(row, &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)
+    return node;
+  gfx::Rect bounds(GetTextBoundsForNodeImpl(node, row, depth));
+  return bounds.Contains(point) ? node : nullptr;
+}
+
+bool TreeView::HitsNodeArrow(InternalNode* node,
+                             const gfx::Point& point) {
+  if (!model_->GetChildCount(node->model_node()))
+    return false;
+
+  int depth = -1;
+  int row = GetRowForInternalNode(node, &depth);
+
+  int arrow_dx = depth * kIndent + kHorizontalInset;
+  gfx::Rect arrow_bounds(bounds().x() + arrow_dx,
+                         row * row_height_ + kVerticalInset,
+                         kArrowRegionSize,
+                         row_height_);
+  if (base::i18n::IsRTL())
+    arrow_bounds.set_x(bounds().width() - arrow_dx - kArrowRegionSize);
+  return arrow_bounds.Contains(point);
+}
+
 // InternalNode ----------------------------------------------------------------
 
 TreeView::InternalNode::InternalNode()