New issue
Advanced search Search tips

Issue 821928 link

Starred by 5 users
Status: Fixed
Owner:
sunyunjia@chromium.org
Closed: Dec 12
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 497851


Participants' hotlists:
Snap-Points-Post-Ship-Tasks


Sign in to add a comment

[css-scroll-snap] Implement scroll snapping for keyboard scrolling

Project Member Reported by majidvp@chromium.org, Mar 14 2018 
We should implement snapping for keyboard scrolling. 

The specification has some guideline on how to categorize various input and keyboard scrolling can fall in all three categories [1]

- homing operations such as the Home/End keys: intended end position
- paging operations such as the PgUp/PgDn keys: intended direction and end position
- pressing an arrow key on the keyboard: intended direction


There is also tab related focus changes but I think we should consider that
under a different issue.

All above cases go through ScrollManager::BubblingScroll with a different
granularity and direction. So that may be a good place to start.


[1] https://drafts.csswg.org/css-scroll-snap/#scroll-types

Comment 1 by sunyunjia@chromium.org, May 4 2018

Summary: [css-scroll-snap] Implement scroll snapping for keyboard scrolling (was: Implement scroll snapping for keyboard scrolling )
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 26 

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

commit e806ef73a7d39067e1bfcf451b4c16f0c56d4837
Author: Sandra Sun <sunyunjia@chromium.org>
Date: Fri Oct 26 22:32:15 2018

Snap after pressing arrow key.

This patch implements snapping after pressing arrow key. We add the
snapping logic in ScrollManager::LogicalScroll(). We also introduce an
abstract class SnapSelectionStrategy with 3 subclasses:
EndPositionStrategy, DirectionStrategy, and EndAndDirectionStrategy.
The snap point selection algorithm would query some values from one of
the classes during the search. This would keep the algorithm in one
place and we can simply change the strategy used to update the
behavior.

The refactoring for SnapSelectionStrategy has required changes in code
for other scroll types (gesture, programming) snapping. Previously all
the scroll snap uses EndPositionStrategy. With new strategies added,
we update the strategy for fling and scrollBy to
EndAndDirectionStrategy.

Bug:  821928 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Id4fbc2219c7f5dcdba415e2d79ceaa55845ba193
Reviewed-on: https://chromium-review.googlesource.com/c/1131223
Commit-Queue: Sandra Sun <sunyunjia@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603222}
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/cc/BUILD.gn
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/cc/input/scroll_snap_data.cc
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/cc/input/scroll_snap_data.h
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/cc/input/scroll_snap_data_unittest.cc
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/cc/input/snap_fling_controller.h
[add] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/cc/input/snap_selection_strategy.cc
[add] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/cc/input/snap_selection_strategy.h
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[add] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/WebKit/LayoutTests/fast/scroll-snap/resources/simple-snap.css
[add] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/WebKit/LayoutTests/fast/scroll-snap/snaps-after-keyboard-scrolling-rtl.html
[add] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/WebKit/LayoutTests/fast/scroll-snap/snaps-after-keyboard-scrolling.html
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/WebKit/LayoutTests/fast/scroll-snap/snaps-after-scrollbar-scrolling.html
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/blink/renderer/core/dom/element.cc
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/blink/renderer/core/frame/local_dom_window.cc
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/blink/renderer/core/input/scroll_manager.cc
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/blink/renderer/core/input/scroll_manager.h
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/blink/renderer/core/page/scrolling/snap_coordinator.h
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/blink/renderer/core/page/scrolling/snap_coordinator_test.cc
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/blink/renderer/core/scroll/scrollable_area.h
[modify] https://crrev.com/e806ef73a7d39067e1bfcf451b4c16f0c56d4837/third_party/blink/renderer/platform/scroll/scroll_snap_data.h
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 12 

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

commit 80dc30dc52954e0c4e559fc65089e7eb1e96abd5
Author: Sandra Sun <sunyunjia@chromium.org>
Date: Wed Dec 12 21:50:38 2018

Snap after pressing PgUp/PgDn and Home/End

This patch implements snapping after pressing PgUp/PgDn and Home/End
keys. The logic is implemented in ScrollManager::LogicalScroll().
These keys are different from arrow keys. Pressing the arrow key is
considered as scrolling with intended direction only. Pressing the
PgUp/PgDn is considered as scrolling with intended direction and end
position. Pressing the Home/End key is considered as scrolling with
intended end position only. So we need to use different strategies to
snap for these keys.

Bug:  821928 
Change-Id: If4d04ff58214a3304d09838f321e65c6e68f7d1d
Reviewed-on: https://chromium-review.googlesource.com/c/1361648
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@{#616062}
[modify] https://crrev.com/80dc30dc52954e0c4e559fc65089e7eb1e96abd5/third_party/blink/renderer/core/input/scroll_manager.cc
[modify] https://crrev.com/80dc30dc52954e0c4e559fc65089e7eb1e96abd5/third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc
[modify] https://crrev.com/80dc30dc52954e0c4e559fc65089e7eb1e96abd5/third_party/blink/renderer/core/page/scrolling/snap_coordinator.h
[modify] https://crrev.com/80dc30dc52954e0c4e559fc65089e7eb1e96abd5/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
[modify] https://crrev.com/80dc30dc52954e0c4e559fc65089e7eb1e96abd5/third_party/blink/web_tests/FlagExpectations/enable-blink-features=CompositeAfterPaint
[add] https://crrev.com/80dc30dc52954e0c4e559fc65089e7eb1e96abd5/third_party/blink/web_tests/fast/scroll-snap/snaps-for-different-key-granularity.html

Comment 4 by sunyunjia@chromium.org, Dec 12

Status: Fixed (was: Assigned)
►

Sign in to add a comment