From 6fea42a4cf31f791f468d5e9e683976e643450ed Mon Sep 17 00:00:00 2001 From: Alex Newcomer Date: Mon, 18 Sep 2017 23:54:59 +0000 Subject: [PATCH] cros: Fix for small AppList interaction problems Two small fixes: 1. The fling velocity threshold needs to be lowered to 6. 2. Dragging up on the launcher sometimes transitions to peeking, this is because we check that abs(drag_offset) > drag_threshold, but don't check the sign of the offset. Bug: 765845 Change-Id: I22270e4caea5b4aab22b347b1ce28e98a5df03fe Reviewed-on: https://chromium-review.googlesource.com/669944 Commit-Queue: Alex Newcomer Reviewed-by: Xiyuan Xia Cr-Commit-Position: refs/heads/master@{#502720} --- .../app_list_presenter_delegate_unittest.cc | 54 ++++++++++++++++--- ash/shelf/shelf_layout_manager.h | 2 +- ui/app_list/views/app_list_view.cc | 4 +- 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/ash/app_list/app_list_presenter_delegate_unittest.cc b/ash/app_list/app_list_presenter_delegate_unittest.cc index f06a301b3b741..f43a320899c7a 100644 --- a/ash/app_list/app_list_presenter_delegate_unittest.cc +++ b/ash/app_list/app_list_presenter_delegate_unittest.cc @@ -465,14 +465,22 @@ TEST_F(FullscreenAppListPresenterDelegateTest, AppListViewDragHandler) { app_list::AppListView::FULLSCREEN_ALL_APPS); // Execute a short downward drag, this should fail to transition the app list. - generator.GestureScrollSequence(gfx::Point(10, 10), gfx::Point(10, 100), - base::TimeDelta::FromMilliseconds(100), 10); + gfx::Point start(10, 10); + gfx::Point end(10, 100); + generator.GestureScrollSequence( + start, end, + generator.CalculateScrollDurationForFlingVelocity(start, end, 1, 100), + 100); EXPECT_EQ(app_list->app_list_state(), app_list::AppListView::FULLSCREEN_ALL_APPS); // Execute a long and slow downward drag to switch to peeking. - generator.GestureScrollSequence(gfx::Point(10, 200), gfx::Point(10, 650), - base::TimeDelta::FromMilliseconds(1500), 100); + start = gfx::Point(10, 200); + end = gfx::Point(10, 650); + generator.GestureScrollSequence( + start, end, + generator.CalculateScrollDurationForFlingVelocity(start, end, 1, 100), + 100); EXPECT_EQ(app_list->app_list_state(), app_list::AppListView::PEEKING); // Transition to fullscreen. @@ -488,8 +496,12 @@ TEST_F(FullscreenAppListPresenterDelegateTest, AppListViewDragHandler) { app_list::AppListView::FULLSCREEN_SEARCH); // Execute a short downward drag, this should fail to close the app list. - generator.GestureScrollSequence(gfx::Point(10, 10), gfx::Point(10, 100), - base::TimeDelta::FromMilliseconds(100), 10); + start = gfx::Point(10, 10); + end = gfx::Point(10, 100); + generator.GestureScrollSequence( + start, end, + generator.CalculateScrollDurationForFlingVelocity(start, end, 1, 100), + 100); EXPECT_EQ(app_list->app_list_state(), app_list::AppListView::FULLSCREEN_SEARCH); @@ -874,8 +886,12 @@ TEST_P(FullscreenAppListPresenterDelegateTest, EXPECT_EQ(view->app_list_state(), app_list::AppListView::FULLSCREEN_ALL_APPS); // Swipe down slowly, the app list should return to peeking mode. - generator.GestureScrollSequence(gfx::Point(0, 0), gfx::Point(0, 650), - base::TimeDelta::FromMilliseconds(1500), 100); + gfx::Point start(0, 0); + gfx::Point end(0, 500); + generator.GestureScrollSequence( + start, end, + generator.CalculateScrollDurationForFlingVelocity(start, end, 1, 100), + 100); EXPECT_EQ(view->app_list_state(), app_list::AppListView::PEEKING); // Move mouse away from the searchbox, mousewheel scroll up. @@ -1004,4 +1020,26 @@ TEST_F(FullscreenAppListPresenterDelegateTest, ASSERT_EQ(app_list::AppListView::FULLSCREEN_ALL_APPS, view->app_list_state()); } +TEST_F(FullscreenAppListPresenterDelegateTest, + LongUpwardDragInFullscreenShouldNotClose) { + app_list_presenter_impl()->Show(GetPrimaryDisplayId()); + app_list::AppListView* view = app_list_presenter_impl()->GetView(); + FlingUpOrDown(GetEventGenerator(), view, true); + EXPECT_EQ(app_list::AppListView::FULLSCREEN_ALL_APPS, view->app_list_state()); + + // Drag from the center of the applist to the top of the screen very slowly. + // This should not trigger a state transition. + gfx::Point drag_start = view->GetBoundsInScreen().CenterPoint(); + drag_start.set_x(15); + gfx::Point drag_end = view->GetBoundsInScreen().top_right(); + drag_end.set_x(15); + GetEventGenerator().GestureScrollSequence( + drag_start, drag_end, + GetEventGenerator().CalculateScrollDurationForFlingVelocity( + drag_start, drag_end, 1, 1000), + 1000); + + EXPECT_EQ(app_list::AppListView::FULLSCREEN_ALL_APPS, view->app_list_state()); +} + } // namespace ash diff --git a/ash/shelf/shelf_layout_manager.h b/ash/shelf/shelf_layout_manager.h index 18262f83f49f9..d79e1934218c9 100644 --- a/ash/shelf/shelf_layout_manager.h +++ b/ash/shelf/shelf_layout_manager.h @@ -68,7 +68,7 @@ class ASH_EXPORT ShelfLayoutManager // The velocity the app list must be dragged in order to change the state of // the app list for fling event, measured in DIPs/event. - static constexpr int kAppListDragVelocityThreshold = 100; + static constexpr int kAppListDragVelocityThreshold = 6; ShelfLayoutManager(ShelfWidget* shelf_widget, Shelf* shelf); ~ShelfLayoutManager() override; diff --git a/ui/app_list/views/app_list_view.cc b/ui/app_list/views/app_list_view.cc index 994a798a5ecdd..f2dc678899b31 100644 --- a/ui/app_list/views/app_list_view.cc +++ b/ui/app_list/views/app_list_view.cc @@ -71,7 +71,7 @@ constexpr int kAppListThresholdDenominator = 3; // The velocity the app list must be dragged in order to transition to the next // state, measured in DIPs/event. -constexpr int kAppListDragVelocityThreshold = 25; +constexpr int kAppListDragVelocityThreshold = 6; // The scroll offset in order to transition from PEEKING to FULLSCREEN constexpr int kAppListMinScrollToSwitchStates = 20; @@ -671,7 +671,7 @@ void AppListView::EndDrag(const gfx::Point& location) { } switch (app_list_state_) { case FULLSCREEN_ALL_APPS: - if (std::abs(drag_delta) > app_list_threshold) { + if (drag_delta < -app_list_threshold) { if (is_tablet_mode_ || is_side_shelf_) Dismiss(); else