-
Notifications
You must be signed in to change notification settings - Fork 707
[css-scroll-snap] Multiple nested scrollers and a "default" scrollIntoView()? #2593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
The proposed solution seems reasonable though as pointed out it adds complexity. This means that we are now allowing snap and align to be simultaneously honored depending on the container. This is fine but it is odd that the only possible alignment option is the default alignment. I think this is not very user friendly and provides for an odd API. This issue is actually caused by the original resolution which makes snap application depend on presence of alignment. I also have a second concern with original proposal. The way it works if a user specifies an alignment we Alternative proposalSince the spec has not been updated and no one has implemented then I suppose we have a chance to reconsider the original resolution. Here is an alternative idea: Introduce a dictionary ScrollIntoViewOptions : ScrollOptions {
ScrollLogicalPosition block = "start";
ScrollLogicalPosition inline = "nearest";
boolean snap = true;
}; I think giving authors an explicit way to disable snapping provides a better API. Also it is better suited if in future we decide to provide a similar option for other scrolling API (e.g., So in this particular case:
|
If you want to disable snapping, we already have an API for that - just set Overall I'm kinda confused tho - you introduce the proposal with the problem that an aligned sIV() won't honor the snap, but the next scroll will; but then your proposed addition just addresses the original problem in my first post, that of what to do about intermediate snap containers during an unaligned sIV(). (Also, boolean options should default to falsey values, by convention. So if we did do this, we'd want to reverse the naming/meaning of the flag.) |
I should have been more clear. This is only disabling snapping for the scrollIntoView() operation only allowing the author to explicitly ask for their specified alignment to win when it is in conflict with snapping. It should not permanently disable snapping for those reason you pointed out.
The general problem that escaping snapping is going to be reverted on next scroll is unavoidable IMHO if we were to allow any form of scroll snapping. The proposed solution at least helps by making it so that the default behavior does not escape snapping. The author has to explicitly ask for it. Original resolution meant that by default any scrollIntoView() with alignment will trigger the above whether it actually wanted to escape snapping or not.
Good to know 😃. So perhaps |
Ah, okay, then yeah, |
I do want to point out that one of the design constraints we agreed on was that script cannot put a scroller into a scroll position that the user cannot then return to. |
That is directly contradicting our other resolution that an explicit alignment directive in a scrolling method should align accordingly, overriding any scroll-snapping. Script shouldn't put the scroller into a user-unreachable position by default, but when you explicitly request it, we will do so. |
No, the other resolution was that the explicit alignment directive would set the scroll target, but it would then be subject to any snapping constraints. If there's none that conflict, then it's fine, you get the alignment you asked for. But if you've got mandatory snapping in a way that conflicts, then it wins, just as if you manually scrolled to the same alignment the script specified and let go. |
I also read the original resolution differently as it meant that snap only wins when there is no alignment. I am fine with fantasai's explanation above. It basically amounts to snap always be in effect which is consistent and easy to reason about. I don't think we need to talk about wining at all rather snapping operates after all other scrolling operations so it always applies. In the case of sIV() if the alignment brings the scroll offset to So effectively:
|
I'm down with that, it's just not how I interpreted the previous resolutions. ^_^ So yeah, that makes this stuff easier, then. Just scroll as normal, with all the nested scrollers aligned as normal, then snap some of them as needed. |
The Working Group just discussed
The full IRC log of that discussion<dael> Topic: Multiple nested scrollers and a "default" scrollIntoView()?<dael> github: https://github.com//issues/2593 <dael> TabAtkins: I suspect it's resolved at this point, but making sure. majidvp and our other impl that are getting our scroll snap up to date found an interaction. Spec defines and interaction between scroll methods alignment and scroll snap. You try to align via howt he scroll method says and if you need to snap after you do. <dael> TabAtkins: Nested scrollers when you have to go multi deep to get hte element in view. Conclusion is you first align all the scrollers as you go down according to scroll method alignment. If any scrollers have scroll snap you honor that to bring them into alignment. And that's about it. <dael> TabAtkins: Unless anyone disagrees that's what we concluded in the thread which means no edits to the spec except we might want to clarify the wording. <dael> Rossen_: Sounds reasonable. Any other opinions or objections? <dael> prop: Have the default scrolling behavior for nested scrollers to have scroll snap <dael> TabAtkins: Yeah, sure. <dael> RESOLVED: Have the default scrolling behavior for nested scrollers to have scroll snap |
Marking needs edits for applying clarifications. |
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should always snap regardless of whether it has alignment specified or not. And all the affected scrollers should also land on a snap position if one exists. This patch adds top_alignment to the WebScrollIntoViewParams to specify the target element's snap alignment. We always retrieve the alignment from top_alignment during scrollIntoView. Once a scroller has been scrolled, the previous top_alignment is popped out and the variable is updated with the alignment specified from scrollIntoView. If the element doesn't have scroll_snap_align, the top_alignment is always the same with the alignment from scrollIntoView. Whenever a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should always snap regardless of whether it has alignment specified or not. And all the affected scrollers should also land on a snap position if one exists. This patch adds top_alignment to the WebScrollIntoViewParams to specify the target element's snap alignment. We always retrieve the alignment from top_alignment during scrollIntoView. Once a scroller has been scrolled, the previous top_alignment is popped out and the variable is updated with the alignment specified from scrollIntoView. If the element doesn't have scroll_snap_align, the top_alignment is always the same with the alignment from scrollIntoView. Whenever a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on asnap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on asnap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d Reviewed-on: https://chromium-review.googlesource.com/c/1188746 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@{#607019}
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d Reviewed-on: https://chromium-review.googlesource.com/c/1188746 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@{#607019}
According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d Reviewed-on: https://chromium-review.googlesource.com/c/1188746 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@{#607019}
Automatic update from web-platform-testsSnap at ScrollIntoView. According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d Reviewed-on: https://chromium-review.googlesource.com/c/1188746 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@{#607019} -- wpt-commits: a0610cf468e6208e74b2cfda77a565ab90fbd74e wpt-pr: 13038
Automatic update from web-platform-testsSnap at ScrollIntoView. According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d Reviewed-on: https://chromium-review.googlesource.com/c/1188746 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@{#607019} -- wpt-commits: a0610cf468e6208e74b2cfda77a565ab90fbd74e wpt-pr: 13038
Automatic update from web-platform-testsSnap at ScrollIntoView. According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d Reviewed-on: https://chromium-review.googlesource.com/c/1188746 Reviewed-by: David Bokan <bokanchromium.org> Reviewed-by: Majid Valipour <majidvpchromium.org> Commit-Queue: Sandra Sun <sunyunjiachromium.org> Cr-Commit-Position: refs/heads/master{#607019} -- wpt-commits: a0610cf468e6208e74b2cfda77a565ab90fbd74e wpt-pr: 13038 UltraBlame original commit: e910ba99ad29164d923592600f408c23c2ddcfa2
Automatic update from web-platform-testsSnap at ScrollIntoView. According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d Reviewed-on: https://chromium-review.googlesource.com/c/1188746 Reviewed-by: David Bokan <bokanchromium.org> Reviewed-by: Majid Valipour <majidvpchromium.org> Commit-Queue: Sandra Sun <sunyunjiachromium.org> Cr-Commit-Position: refs/heads/master{#607019} -- wpt-commits: a0610cf468e6208e74b2cfda77a565ab90fbd74e wpt-pr: 13038 UltraBlame original commit: e910ba99ad29164d923592600f408c23c2ddcfa2
Automatic update from web-platform-testsSnap at ScrollIntoView. According to the spec, w3c/csswg-drafts#2593 (comment) scrollIntoView should 1) Always snap to the target element's snap alignment, and 2) All the affected scrollers should also land on a snap position if one exists. This patch does part 2). Before a scrollable_area is added to the smoothScrollSequencer with its new scroll_offset, we will check if it has a valid snap offset around, and update the final offset accordingly. We'll implement part 1) in a separate patch. Bug: 842317 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ica22134ed0b1f9d36f5a017c8dd0be54e907dd3d Reviewed-on: https://chromium-review.googlesource.com/c/1188746 Reviewed-by: David Bokan <bokanchromium.org> Reviewed-by: Majid Valipour <majidvpchromium.org> Commit-Queue: Sandra Sun <sunyunjiachromium.org> Cr-Commit-Position: refs/heads/master{#607019} -- wpt-commits: a0610cf468e6208e74b2cfda77a565ab90fbd74e wpt-pr: 13038 UltraBlame original commit: e910ba99ad29164d923592600f408c23c2ddcfa2
Our intention in #1708 is that scrollIntoView()'s alignment arg is honored if specified, but if it's unspecified we honor scroll-snap; only when both are unspecified do we use the "default" sIV() alignment.
What do we do when the targetted element is nested into multiple scrollable elements, tho? Each one of them needs to be scrolled to bring the element into view. When sIV() doesn't specify a particular alignment, how should we treat all the nested scrollers? In particular, what if some of them have scroll-snap specified, while others don't?
Honoring scroll-snap on the scrollers that have it, and default-aligning the rest, seems like probably the best behavior, but it's also significantly more complicated for implementations, as they have to collect and track more data as they execute the scroll operation. What should we do?
The text was updated successfully, but these errors were encountered: