Skip to content

Commit

Permalink
cros: Fix for small AppList interaction problems
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Xiyuan Xia <[email protected]>
Cr-Commit-Position: refs/heads/master@{#502720}
  • Loading branch information
Alex Newcomer authored and Commit Bot committed Sep 18, 2017
1 parent 04a3b37 commit 6fea42a
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 11 deletions.
54 changes: 46 additions & 8 deletions ash/app_list/app_list_presenter_delegate_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);

Expand Down Expand Up @@ -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.
Expand Down Expand 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
2 changes: 1 addition & 1 deletion ash/shelf/shelf_layout_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions ui/app_list/views/app_list_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 6fea42a

Please sign in to comment.