New issue
Advanced search Search tips

Issue 905770 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 18
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 497851



Sign in to add a comment

Should snap backwards if there is no snap position on the direction forward and the type is mandatory.

Project Member Reported by sunyunjia@chromium.org, Nov 15

Issue description

Current behavior: Given a scroll that starts on a non-snap position and has a specified direction, if there's no snap position on the direction, the snap behavior won't be performed and the scroll will end at the default offset.

Problem: However, if the scroll-snap-type is mandatory and there's a valid snap position on the other direction, at the scroll end, another snap behavior will still be performed, breaking the scroll into two parts(original scroll and snap scroll).

Use cases: This problem is prevalent in the fling gestures. The fling gesture consists of: a GSB, several non-inertial GSU when the finger is on the screen, several inertial GSU after the finger is lifted, and a GSE. Even if the gesture is performed from a snap position, at the time when the finger is lifted and we decide to snap for the fling, it's already away from the snap position, and won't snap back until the fling has ended.

We should allow it to snap backwards if there is no snap position on the direction forward and the type is mandatory.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8acec0a3ba9c5cffa8f5aa8fc4cf96c9aca7039e

commit 8acec0a3ba9c5cffa8f5aa8fc4cf96c9aca7039e
Author: Sandra Sun <sunyunjia@chromium.org>
Date: Tue Dec 18 17:05:14 2018

Allowing snap backwards with type: mandatory.

Current behavior: Given a scroll that starts on a non-snap position and
has a specified direction, if there's no snap position on the direction,
the snap behavior won't be performed and the scroll will end at the
default offset.

Problem: However, if the scroll-snap-type is mandatory and there's a
valid snap position on the other direction, at the scroll end, another
snap behavior will still be performed, breaking the scroll into two
parts(original scroll and snap scroll).

Example: This problem is prevalent in the fling gestures. The fling
gesture consists of: a GSB, several non-inertial GSU when the finger is
on the screen, several inertial GSU after the finger is lifted, and a
GSE. Even if the gesture is performed from a snap position, at the time
when the finger is lifted and we decide to snap for the fling, it's
already away from the snap position, and won't snap back until the
fling has ended.

We should allow it to snap backwards if there is no snap position on
the direction forward and the type is mandatory.

This patch renames FindClosestValidArea() to
FindClosestValidAreaInternal(). And use FindClosestValidArea() as a
wrapper of FCVAI(). In cases where we need to snap back, after the
first call of FCVAI() fails, we create a relaxed strategy that ignores
the direction, and call FCVAI() with the relaxed strategy again to see
if there is any valid snap position backwards.

Previous logic adds a special check for current position for mandatory
snapping. The new patch merges this with new logic as this case is
included in the second search with relaxed strategy. This patch also
considers snap positions outside the intended direction as valid
prev/next positions for large areas check, as they shouldn't be affected
by directions.

Bug:  905770 
Change-Id: Ib99484e94b77bc3731b7efd9c31bbce40195bf36
Reviewed-on: https://chromium-review.googlesource.com/c/1338200
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617537}
[modify] https://crrev.com/8acec0a3ba9c5cffa8f5aa8fc4cf96c9aca7039e/cc/input/scroll_snap_data.cc
[modify] https://crrev.com/8acec0a3ba9c5cffa8f5aa8fc4cf96c9aca7039e/cc/input/scroll_snap_data.h
[modify] https://crrev.com/8acec0a3ba9c5cffa8f5aa8fc4cf96c9aca7039e/cc/input/scroll_snap_data_unittest.cc
[modify] https://crrev.com/8acec0a3ba9c5cffa8f5aa8fc4cf96c9aca7039e/cc/input/snap_selection_strategy.cc
[modify] https://crrev.com/8acec0a3ba9c5cffa8f5aa8fc4cf96c9aca7039e/cc/input/snap_selection_strategy.h

Blocking: 497851
Status: Fixed (was: Started)

Sign in to add a comment