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

[cssom-view] Fire scrollend consistently in cases where there is no scrolling. #8218

Open
flackr opened this issue Dec 13, 2022 · 9 comments

Comments

@flackr
Copy link
Contributor

flackr commented Dec 13, 2022

While experimenting with the implementations of scrollend in firefox and chrome I noticed that there's some inconsistency about in which cases it fires when no scrolling occurs. In particular, scrollend is fired when the user pressed an arrow key, or performs a scrolling action which results in no scroll, but is not fired when a programmatic API is used to scroll to the current location.

While I can see an ergonomic argument for strictly pairing scrollend to scroll events, I think it would be useful if we guaranteed a scrollend event will occur after scrolling API calls, enabling developers to do something after a scroll to a location without needing to detect if scroll didn't happen. e.g.

function scrollToPromise(element) {
  return new Promise((resolve) => {
    window.addEventListener('scrollend', resolve);
    element.scrollIntoViewIfNeeded();
  });
}

@argyleink

@flackr
Copy link
Contributor Author

flackr commented Dec 13, 2022

Though if we make the scrolling APIs (scrollIntoView, scrollIntoViewIfNeeded, scrollTo, etc) return promises which resolve when the scroll is complete I could see never firing scrollend if there is no scroll.

I'm not sure whether the current spec text is clear on what the behavior should be to the current text,

Whenever scrolling is completed, the user agent must run these steps

This could be interpreted either way I think.

@flackr
Copy link
Contributor Author

flackr commented Dec 13, 2022

In fact, on further reflection I strongly prefer scrollend strongly paired to scroll events, and developers relying on promises for scroll apis. So TLDR:

  1. Update / ensure definition for when scrolling is completed used in cssom-view-1 spec Scrolling section does not occur if there was no scroll.
  2. Return promises from scrolling APIs. Seems to have been resolved in issue [cssom-view] Consider making scroll methods return promises #1562 but not done yet.

@argyleink
Copy link
Contributor

I feel like keeping scrollend "simple" in that it fires after scroll when there's no more translations or user input happening. if the scroll is requested to a new location, but it happens its the current location, there was no scroll so no scrollend event. It feels much more appropriate for the scrollTo functions to provide a callback or promise that the request was fulfilled, regardless of whether scroll actually occurred.

the problem space of this issue seems to be "it's hard to know that no scrolling occurred after programmatic assignment" and I think that problem lives at a higher level than scrollend, aka it's not scrollend's responsibility to remedy it. i do agree this is a real issue though and scroll api's should offer ways to reduce this mystery.

@argyleink
Copy link
Contributor

ensure definition

what kind of updates would reduce ambiguity for you? have suggestions 🙂

@flackr
Copy link
Contributor Author

flackr commented Dec 13, 2022

So there is a note that calls this out that no scrolling produces no scrollend, but if I read the normative text from https://drafts.csswg.org/cssom-view/#scrolling it sounds as if scrollend is always emitted.

I.e. taking the instant case (because it's simpler), the following steps must be run:

  1. Abort any ongoing smooth scroll for box.
  2. perform an instant scroll of box to position. After an instant scroll emit the scrollend event.

This needs normative text to say something like if the scroll position was not originally "position" then emit a scrollend event.

@argyleink
Copy link
Contributor

argyleink commented Dec 19, 2022

I just tested out adding some of this additional copy,and it sounds repetitive while also having the note directly following the new copy about not emitting the event if no translation occurred. I'm worried it'll clutter and repeat the copy instead of make it clearer. The spec starts to sound really concerned you'll not see the note. Thoughts? maybe adding more tests for these cases is better than copy edits?

@argyleink
Copy link
Contributor

this still relevant you feel or covered via updates since this issue was created?

@flackr
Copy link
Contributor Author

flackr commented Mar 16, 2023

I still think the normative text is wrong. What if we add a step before step 2 after the abort in step 1 that says if the box is already scrolled to position, abort the remaining steps?

This would be similar to step 3 in the procedure to scroll to the beginning of the document

@dlrobertson
Copy link
Member

This seems like this would be solved by #8397

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

3 participants