RESOLVED FIXED 197744
Implement scroll-snap-stop for scroll snapping
https://bugs.webkit.org/show_bug.cgi?id=197744
Summary Implement scroll-snap-stop for scroll snapping
Majid Valipour
Reported 2019-05-09 11:08:29 PDT
This property is in scroll snap level 1 specification CR [0]. Please consider implementing this. We (at Chromium) have implemented this and plan to ship it soon [1]. We have heard from several developers (e.g., See here [2] AirBnb, AMP [3]) that they want this. A particular popular usecase is to create full-size image carousels that only snap to the next image (e.g., similar to how native apps do such things) which is the motivation behind why this feature was added to the spec in the first place. Normally a fling gesture snaps to the closest area near its "natural end point" which is not the right behavior in such usecases. We have added one wpt test for this feature so far as well [4]. [0] https://www.w3.org/TR/css-scroll-snap-1/#scroll-snap-stop [1] https://chromestatus.com/features/5439846480871424 [2] https://bugs.chromium.org/p/chromium/issues/detail?id=823998#c3 [3] https://bugs.chromium.org/p/chromium/issues/detail?id=920482#c1 [4] https://wpt.fyi/results/css/css-scroll-snap/scroll-snap-stop-always.html?label=master&product=chrome%5Bexperimental%5D&product=edge&product=firefox%5Bexperimental%5D&product=safari%5Bexperimental%5D&aligned&q=scroll-snap
Attachments
Patch (92.39 KB, patch)
2021-02-02 10:39 PST, Martin Robinson
ews-feeder: commit-queue-
Patch (96.19 KB, patch)
2021-02-02 11:02 PST, Martin Robinson
no flags
Patch (99.04 KB, patch)
2021-02-03 02:08 PST, Martin Robinson
no flags
Patch (99.93 KB, patch)
2021-02-09 00:57 PST, Martin Robinson
no flags
Patch (99.80 KB, patch)
2021-02-09 03:06 PST, Martin Robinson
no flags
Patch (99.80 KB, patch)
2021-02-09 05:53 PST, Martin Robinson
no flags
The attached testcase is a basic HTML/CSS file that demonstrates minimum viable snap requirements. (1.39 KB, text/html)
2021-03-17 16:10 PDT, Raphael Schaad
no flags
Radar WebKit Bug Importer
Comment 1 2019-05-12 15:00:33 PDT
Ben Frain
Comment 2 2019-07-17 08:54:42 PDT
Just to add a request to implement this. We have a web-app using card like interfaces and the ability to side-swipe between them one at a time with scroll-snap would be a far better option than the default.
Raphael Schaad
Comment 3 2020-10-19 08:20:29 PDT
From a developer perspective, this is a great API and works flawlessly for our use case in Chrome. There is no workaround to achieve this in Safari short of implementing custom scrolling.
Simon Fraser (smfr)
Comment 4 2020-11-09 18:52:25 PST
Martin Robinson
Comment 5 2021-02-02 10:39:14 PST
Martin Robinson
Comment 6 2021-02-02 11:02:49 PST
Martin Robinson
Comment 7 2021-02-03 00:32:51 PST
Comment on attachment 419030 [details] Patch This still needs a bit of work.
Martin Robinson
Comment 8 2021-02-03 02:08:12 PST
Martin Robinson
Comment 9 2021-02-03 03:41:36 PST
Comment on attachment 419116 [details] Patch Okay. I believe that I have sorted the failures that this patch was causing and it is now ready for review. Sorry for the noise.
Simon Fraser (smfr)
Comment 10 2021-02-08 09:13:55 PST
Comment on attachment 419116 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419116&action=review > Source/WebCore/ChangeLog:9 > + No new tests. Please add some words here, with a link to the spec, and list which WPT test the feature. > Source/WebCore/rendering/RenderLayerModelObject.cpp:204 > + bool scrollSnapStopChanged = > + oldStyle && oldStyle->scrollSnapStop() != newStyle.scrollSnapStop(); Don't wrap this.
Martin Robinson
Comment 11 2021-02-09 00:57:27 PST
Martin Robinson
Comment 12 2021-02-09 03:06:10 PST
Martin Robinson
Comment 13 2021-02-09 05:53:16 PST
Martin Robinson
Comment 14 2021-02-09 11:28:23 PST
(In reply to Simon Fraser (smfr) from comment #10) > Comment on attachment 419116 [details] > Patch Thanks for the review! > View in context: > https://bugs.webkit.org/attachment.cgi?id=419116&action=review > > > Source/WebCore/ChangeLog:9 > > + No new tests. > > Please add some words here, with a link to the spec, and list which WPT test > the feature. Sure thing. I've added the list of WPT tests here. > > > Source/WebCore/rendering/RenderLayerModelObject.cpp:204 > > + bool scrollSnapStopChanged = > > + oldStyle && oldStyle->scrollSnapStop() != newStyle.scrollSnapStop(); > > Don't wrap this. Okay. I've also unwrapped the lines around it for consistency.
Martin Robinson
Comment 15 2021-02-09 13:32:03 PST
Comment on attachment 419707 [details] Patch The assertion failure on the Mac Debug WK1 bot looks to be unrelated to this change.
EWS
Comment 16 2021-02-09 13:52:42 PST
Committed r272610: <https://commits.webkit.org/r272610> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419707 [details].
Raphael Schaad
Comment 17 2021-03-17 16:10:10 PDT
Created attachment 423535 [details] The attached testcase is a basic HTML/CSS file that demonstrates minimum viable snap requirements.
Raphael Schaad
Comment 18 2021-03-17 16:13:02 PDT
`scroll-snap-stop` doesn't work properly, tested in STP Release 122. Unlike in Chromium, it scrolls past `scroll-snap-stop: always` hard stops. The previously attached testcase is a basic HTML/CSS file that demonstrates minimum viable snap requirements: outer hard snap (paging between colors), inner soft snap (aligning to black lines), un-snapped vertical scroll. If there is a preferred way of re-opening this bug over comment here (e.g. open a new bug and reference the original bug, or comment on the metabug 218115 tracking remaining CSS scroll-snap web compatibility issues), feel free to let me know.
Simon Fraser (smfr)
Comment 19 2021-03-17 16:26:21 PDT
Please open a new bug and file your test case there.
Raphael Schaad
Comment 20 2021-03-17 18:09:58 PDT
Opened bug 223406
Note You need to log in before you can comment on or make changes to this bug.