Skip to content
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

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

Open
dontcallmedom opened this issue Jul 1, 2022 · 10 comments

Comments

@dontcallmedom
Copy link
Member

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.

@bakura10
Copy link

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.

@zcorpan
Copy link
Member

zcorpan commented Nov 22, 2023

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
Copy link
Member

zcorpan commented Nov 22, 2023

The implemented SnapEvent interface is also not specified.

@flackr
Copy link
Contributor

flackr commented Nov 22, 2023

@DavMila is working on improving the spec for these on #9515

@zcorpan
Copy link
Member

zcorpan commented Nov 23, 2023

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

@DavMila
Copy link
Contributor

DavMila commented Nov 23, 2023

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.)

@zcorpan
Copy link
Member

zcorpan commented Nov 23, 2023

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.

@DavMila
Copy link
Contributor

DavMila commented Dec 11, 2023

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

@flackr
Copy link
Contributor

flackr commented Jun 7, 2024

Is this resolved now? I think @DavMila articulated why it's useful to have separate events #7442 (comment), we agreed on and updated the names #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.

@dontcallmedom
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants