csswg-drafts icon indicating copy to clipboard operation
csswg-drafts copied to clipboard

[css-scroll-snap-2] snapChanged and snapChanging events are confusing and incomplete

Open dontcallmedom opened this issue 3 years ago • 1 comments

https://drafts.csswg.org/css-scroll-snap-2/#snap-events seems two describe two new events, snapChanged and snapChanging.

Confusingly, it describes them as "event handler" with an event type of scroll, but I'm not sure what this is supposed to mean.

This misses an IDL fragment that describes on… event handler (presumably on the HTMLElement interface, but maybe GlobalEventHandlers if the events are supposed to bubble).

The vast majority of event names are purely lowercase, so the event should probably be snapchanged and snapchanging.

dontcallmedom avatar Jul 01 '22 08:07 dontcallmedom

Adding on that, JS seems to always have used present tense (input events: change, blur), page events (pagehide), window events (resize)... would it be more sense to replace "snapchange" by "snapchanged" ? I agree snapchanged better reflect what it happens, but for consistency reason I think standard events should follow standard casing and naming logic.

bakura10 avatar Aug 15 '22 05:08 bakura10

The table seems to use the same structure as in https://html.spec.whatwg.org/multipage/webappapis.html#event-handlers-on-elements%2C-document-objects%2C-and-window-objects but confusingly defines "snapChanged" and "snapChanging" as an event handlers for scroll events (on which objects?). i.e.:

obj.snapChanged = e => console.assert(e.type === "scroll")

It looks like implementation has started in Chromium, so addressing the naming at least would be good. https://bugs.chromium.org/p/chromium/issues/detail?id=1456339 https://bugs.chromium.org/p/chromium/issues/detail?id=1494892

zcorpan avatar Nov 22 '23 15:11 zcorpan

The implemented SnapEvent interface is also not specified.

zcorpan avatar Nov 22 '23 15:11 zcorpan

@DavMila is working on improving the spec for these on https://github.com/w3c/csswg-drafts/pull/9515

flackr avatar Nov 22 '23 15:11 flackr

Why are new events needed, vs adding the snap information to the existing scroll and scrollend events?

zcorpan avatar Nov 23 '23 19:11 zcorpan

For snapchanging, I'd say I think it's better as a separate event form scroll so a developer doesn't have to have an event listener run on every scroll event just to know whether something snap-related is happening. For snapchanged, I think modifying scrollend would work except that snapchanged could be invoked by a layout change as well. (I acknowledge that spec still doesn't say much about these events.)

DavMila avatar Nov 23 '23 20:11 DavMila

OK, thanks.

Since there was a comment about the naming, I'd like to suggest event names that are more consistent with scroll and scrollend:

  • scrollsnap
  • scrollsnapend

This would make them appear next to scroll in autocomplete and docs etc.

zcorpan avatar Nov 23 '23 21:11 zcorpan

I filed this issue so the working group can come to a resolution on the naming at some point.

DavMila avatar Dec 11 '23 19:12 DavMila

Is this resolved now? I think @DavMila articulated why it's useful to have separate events https://github.com/w3c/csswg-drafts/issues/7442#issuecomment-1824863866, we agreed on and updated the names https://github.com/w3c/csswg-drafts/issues/9697, and the event handlers have been called out in the appendix https://drafts.csswg.org/css-scroll-snap-2/#event-handlers though should be added to the html spec, but it's not uncommon to start in the appendix of the related spec.

flackr avatar Jun 07 '24 16:06 flackr

re event handler, I think #10344 still needs consideration - but otherwise, my side of the issue has been addressed

dontcallmedom avatar Jun 07 '24 16:06 dontcallmedom