feat(ViewTransitions): use scrollend instead of scroll where supported
#8156
Conversation
…orted The [scrollend](https://developer.chrome.com/blog/scrollend-a-new-javascript-event/#event-details) mechanism seems like a better way to record the scroll position compared to throttling, so we could use it whenever a browser supports it. Additionally, I've removed the `{passive}` flag from the `scroll` event, as it does nothing ([source](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener?retiredLocale=de#improving_scrolling_performance_with_passive_listeners:~:text=You%20don%27t%20need%20to%20worry%20about%20the%20value%20of%20passive%20for%20the%20basic%20scroll%20event.%20Since%20it%20can%27t%20be%20canceled%2C%20event%20listeners%20can%27t%20block%20page%20rendering%20anyway.)).
🦋 Changeset detectedLatest commit: ceb4253 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
pkg: astro
|
sgtm. Need a changeset and to not be in draft status. |
|
Ok, want to talk through this just a little more. Does cc @martrapp |
The However, if you scroll around, keep your finger at some position and e.g. some random non-Astro JS triggers a navigation after a specific timeout, |
|
I like the idea to ignore a bunch of scroll events in favor of a single scroll end event. you could try to keep scrolling the mouse wheel while you press one handed Position tracking before clicking is handled by the click handler. We only need the scroll handler for history navigation, because we can only access the "navigated to state" in the popstate handler and then it is too late to update the "navigated from state". |
|
@kurtextrem Yeah good point. If I'm correct, you're saying that it's probably not possible to navigate; either by clicking a link or doing a back swipe gesture, without the scrollend occurring. That sounds right to me, and removes my fear for this. @martrapp marked you as a reviewer if you don't mind approving this. |
|
Oh! Sorry about that. I'll take this comment as an approval. I guess access is only for maintainers (which you are on track to become imo). |
Changes
The scrollend mechanism seems like a better way to record the scroll position compared to throttling, so we could use it whenever a browser supports it.
Additionally, I've removed the
{passive}flag from thescrollevent, as it does nothing (source).pnpm exec changesetTesting
I've tested it locally, but maybe I'm missing out how you are testing changes to this component.
Docs
Open question: Does it make sense to document we're using
scrollendin modern browsers? Personally, I don't think so (implementation details), but maybe it's interesting for some techy folks.