Cros: Increase tap area of scrollable arrow button

The tapping area for the arrow button in the scrollable shelf is really
small, which makes it hard to click on small devices. This change
increases it enopugh for it to be easier to hit the target.

Bug: 1004998
Change-Id: I74c9c3083e496f1e248eb00383be3ae1ba35bcf0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1815359
Commit-Queue: Ana Salazar <anasalazar@chromium.org>
Reviewed-by: Manu Cornet <manucornet@chromium.org>
Reviewed-by: Andrew Xu <andrewxu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698976}

ash/shelf/scrollable_shelf_view.cc

@@ -18,6 +18,7 @@
 #include "ui/gfx/geometry/vector2d_conversions.h"
 #include "ui/gfx/skia_paint_util.h"
 #include "ui/views/focus/focus_search.h"
+#include "ui/views/view_targeter_delegate.h"
 
 namespace ash {
 
@@ -42,6 +43,9 @@ constexpr int kArrowButtonGroupWidth =
 // neglected.
 constexpr int kFlingVelocityThreshold = 1000;
 
+// Horizontal size of the tap areafor the overflow arrow button.
+constexpr int kArrowButtonTapAreaHorizontal = 32;
+
 // Sum of the shelf button size and the gap between shelf buttons.
 int GetUnit() {
   return ShelfConfig::Get()->button_size() +
@@ -154,6 +158,41 @@ class ScrollableShelfView::GradientLayerDelegate : public ui::LayerDelegate {
   DISALLOW_COPY_AND_ASSIGN(GradientLayerDelegate);
 };
 
+////////////////////////////////////////////////////////////////////////////////
+// ScrollableShelfArrowView
+
+class ScrollableShelfView::ScrollableShelfArrowView
+    : public ash::ScrollArrowView,
+      public views::ViewTargeterDelegate {
+ public:
+  explicit ScrollableShelfArrowView(ArrowType arrow_type,
+                                    bool is_horizontal_alignment,
+                                    Shelf* shelf,
+                                    ShelfButtonDelegate* shelf_button_delegate)
+      : ScrollArrowView(arrow_type,
+                        is_horizontal_alignment,
+                        shelf,
+                        shelf_button_delegate) {
+    SetEventTargeter(std::make_unique<views::ViewTargeter>(this));
+  }
+  ~ScrollableShelfArrowView() override = default;
+
+  // views::ViewTargeterDelegate:
+  bool DoesIntersectRect(const views::View* target,
+                         const gfx::Rect& rect) const override {
+    DCHECK_EQ(target, this);
+    gfx::Rect bounds = gfx::Rect(size());
+
+    // Calculate padding for the tap area. (Should be 32 x shelfButtonSize)
+    constexpr int horizontalPadding =
+        (kArrowButtonTapAreaHorizontal - kArrowButtonSize) / 2;
+    const int verticalPadding =
+        (ShelfConfig::Get()->button_size() - kArrowButtonSize) / 2;
+    bounds.Inset(gfx::Insets(-verticalPadding, -horizontalPadding));
+    return bounds.Intersects(rect);
+  }
+};
+
 ////////////////////////////////////////////////////////////////////////////////
 // ScrollableShelfFocusSearch
 
@@ -242,12 +281,12 @@ void ScrollableShelfView::Init() {
   layer()->SetFillsBoundsOpaquely(false);
 
   // Initialize the left arrow button.
-  left_arrow_ = AddChildView(std::make_unique<ScrollArrowView>(
+  left_arrow_ = AddChildView(std::make_unique<ScrollableShelfArrowView>(
       ScrollArrowView::kLeft, GetShelf()->IsHorizontalAlignment(), GetShelf(),
       this));
 
   // Initialize the right arrow button.
-  right_arrow_ = AddChildView(std::make_unique<ScrollArrowView>(
+  right_arrow_ = AddChildView(std::make_unique<ScrollableShelfArrowView>(
       ScrollArrowView::kRight, GetShelf()->IsHorizontalAlignment(), GetShelf(),
       this));
 

ash/shelf/scrollable_shelf_view.h

@@ -88,6 +88,7 @@ class ASH_EXPORT ScrollableShelfView : public views::AccessiblePaneView,
 
  private:
   class GradientLayerDelegate;
+  class ScrollableShelfArrowView;
 
   enum ScrollStatus {
     // Indicates whether the gesture scrolling is across the main axis.

ash/shelf/scrollable_shelf_view_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "ash/shelf/scrollable_shelf_view.h"
 
+#include "ash/public/cpp/shelf_config.h"
 #include "ash/shelf/shelf_test_util.h"
 #include "ash/shelf/shelf_tooltip_manager.h"
 #include "ash/shelf/shelf_view_test_api.h"
@@ -166,4 +167,46 @@ TEST_F(ScrollableShelfViewTest, NotShowTooltipForHiddenIcons) {
   EXPECT_FALSE(tooltip_manager->IsVisible());
 }
 
+// Test that tapping near the scroll arrow button triggers scrolling. (see
+// https://crbug.com/1004998)
+TEST_F(ScrollableShelfViewTest, ScrollAfterTappingNearScrollArrow) {
+  AddAppShortcutUntilOverflow();
+
+  ASSERT_EQ(ScrollableShelfView::kShowRightArrowButton,
+            scrollable_shelf_view_->layout_strategy_for_test());
+
+  // Tap right arrow and check that the scrollable shelf now shows the left
+  // arrow only. Then do the same for the left arrow.
+  const gfx::Rect right_arrow =
+      scrollable_shelf_view_->right_arrow()->GetBoundsInScreen();
+  GetEventGenerator()->GestureTapAt(right_arrow.CenterPoint());
+  ASSERT_EQ(ScrollableShelfView::kShowLeftArrowButton,
+            scrollable_shelf_view_->layout_strategy_for_test());
+
+  const gfx::Rect left_arrow =
+      scrollable_shelf_view_->left_arrow()->GetBoundsInScreen();
+  GetEventGenerator()->GestureTapAt(left_arrow.CenterPoint());
+  ASSERT_EQ(ScrollableShelfView::kShowRightArrowButton,
+            scrollable_shelf_view_->layout_strategy_for_test());
+
+  // Recalculate the right arrow  bounds considering the padding for the tap
+  // area.
+  const int horizontalPadding = (32 - right_arrow.width()) / 2;
+  const int verticalPadding =
+      (ShelfConfig::Get()->button_size() - right_arrow.height()) / 2;
+
+  // Tap near the right arrow and check that the scrollable shelf now shows the
+  // left arrow only. Then do the same for the left arrow.
+  GetEventGenerator()->GestureTapAt(
+      gfx::Point(right_arrow.top_right().x() - horizontalPadding,
+                 right_arrow.top_right().y() + verticalPadding));
+  ASSERT_EQ(ScrollableShelfView::kShowLeftArrowButton,
+            scrollable_shelf_view_->layout_strategy_for_test());
+
+  GetEventGenerator()->GestureTapAt(
+      gfx::Point(left_arrow.top_right().x(), left_arrow.top_right().y()));
+  EXPECT_EQ(ScrollableShelfView::kShowRightArrowButton,
+            scrollable_shelf_view_->layout_strategy_for_test());
+}
+
 }  // namespace ash