View Transitions: use history.scrollRestoration="manual"

[justinbeaty]

Changes

• When using the morph animation, there is a scroll position jump when navigating back / forward through history.
• This is because the browser is restoring the old scroll position before the DOM is swapped.
• This PR simply changes history.scrollRestoration = "manual"; to tell the browser we'll take care of the scroll position ourselves.

Testing

• Create a morph animation with transition:name
• Navigate back / forwards in history at various scroll positions and see the smooth animation

Docs

• No doc updated needed, this was a bug.

[changeset-bot]

🦋 Changeset detected

Latest commit: 25c81e1

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

[matthewp]

Perfect, thank you!

[matthewp]

@martrapp this sound right to you?

[justinbeaty]

Even more generally, this fixes page jumping for all SPA page transitions when using back / forward navigation. The morph transitions were just the most noticeable.

[natemoo-re]

Thanks for the PR! Looks great!

[martrapp]

Hi @justinbeaty! Thanks a lot for the PR. I wasn't really able to see what scrolling is suppressed by scrollRestoration="manual" for navigating to different pages. Do you have a minimal example where I can observe the difference in behavior?

In its current form, the PR breaks the in-page history navigation (back/forward between links to fragments on the same page).

Currently, we only actively handle navigation between different pages.
We leave navigation on the same page to the browser.
Switching all history entries to "manual" does not work for intra-page navigation.

[justinbeaty]

@martrapp Here's a reproducible example: https://github.com/justinbeaty/astro-scroll-restoration-mwe

There's also the with-pr branch that uses the ViewTransition.astro file from this PR.

Here's a video before the PR. Notice how the colored box jumps around, and also you can see flashes of red (from the footer) in some cases.

bug-compressed.mp4

Here's a video after:

with-pr-compressed.mp4

I will investigate the hash fragment issue, but I'm quite certain history.scrollRestoration = "manual"; is the correct setting.

[martrapp]

Quite a difference, thank you for the examples.
Maybe we should set "manual" just before we add new entries with pushState and set it to "auto" when we fall through to the browsers default handling.

[justinbeaty]

That might be an easy solution, otherwise we have to pushState on fragment changes, but avoid fetching the page. But that's a pretty significant refactor it seems like.

[martrapp]

@justinbeaty Hi Justin, do you see a chance to update your PR?
Do you want to give "manual before pushState" a try?

[justinbeaty]

@martrapp Hi Martin, I just tried this on my minimal reproducible repo, which seems like it might in fact resolve the issue. I've only done light testing so I could be missing something (much like I missed the issues with the hash fragments in the first place.)

justinbeaty/astro-scroll-restoration-mwe@844e635 (Note: if you clone this project, there's a /test page with anchors to hash fragments on it for testing)

I can update this PR with that change as well if there's not a cleaner way to do this. I'm also not opposed to a replacement PR or commit by the Astro team if the solution is more involved. I haven't fully grokked the ViewTransitions.astro file yet.

[martrapp]

@justinbeaty I'm sure that resetting scrollRestoration to "auto" at the beginning of the click handler and adding "manual" scroll restoration just before the two pushStates is a real improvement! In fact, this might also be necessary to solve another scrolling issue. Currently I don't see any chance that your change would make anything worse ;-)

Thanks to your discovery, the issue with the hash fragments is now covered by the e2e tests.

You found the bug and I would be happy to see your updated PR in the code base ;-)

[justinbeaty]

@martrapp Done, I'm glad to know that it might fix another bug too.

[matthewp]

lgtm