dillonkearns / elm-pages

Hybrid Elm framework with full-stack and static routes.
https://elm-pages.com
BSD 3-Clause "New" or "Revised" License
650 stars 96 forks source link

Scroll position is not restored when navigating back in browser #294

Open Strepto opened 2 years ago

Strepto commented 2 years ago

Hi, great stuff!

I have tried some of the demos, and notice that the vertical scroll position on a page is not kept when using the "Back" button in my browser. This makes apps such as the Hackernews clone (https://hacker-news-elm-pages.netlify.app) and the Elm-Pages website (https://elm-pages.com) harder to navigate.

Compare:

https://astro-solid-hn.netlify.app to https://hacker-news-elm-pages.netlify.app

Repro:

  1. Scroll down on a page.
  2. Press view comments of a random story.
  3. Wait for it to load.
  4. Press the "Back button" in your browser to go back.
  5. Observe the vertical scroll position

The astro-solid-hn keeps the scroll position from before you navigated away, while elm-pages-hn jumps to the top of the page again.

Browser: Vivaldi (Chromium based)

Feel free to close the issue if this is a duplicate, or a known problem :)

dillonkearns commented 2 years ago

Thanks for that feedback!

I think it's worth noting that the Astro example you linked to is an MPA (multi-page app), not an SPA (single-page app). So the astro app is using the default browser scroll restoration on back navigation.

I think it would be great to understand what other SPA frameworks, like Remix, do in these cases.

I found something that the Remix team wrote in an earlier version of React Router (a tool they maintain which has an API for scroll restoration): https://v5.reactrouter.com/web/guides/scroll-restoration. One of the things it talks about is the idea of restoring scroll position on back/forward navigation, but not on link clicks.

They also talk about not implementing a generic solution, at least in that release. I'm still trying to understand if that is any different with the latest React Router release, or with Remix, or if you have to opt in to scroll restoration.

I think laying out how these things are solved in Remix would be a great starting point. In particular, I'd be curious to understand 1) what is the default behavior, and 2) what is the additional behavior that you can opt into or configure.

dillonkearns commented 2 years ago

Here's the same hackernews demo built with Remix: https://remix-hackernews.ryansolid.workers.dev. It looks like it also has scroll restoration.

Looking through the code (https://github.com/ryansolid/remix-hackernews), the only reference I see to scrolling is here:

https://github.com/ryansolid/remix-hackernews/blob/d254c3ffff585d9ed6d32cfe032d7267ab2a797d/app/root.tsx#L109

So it looks like the default Remix template includes that behavior, but they allow you to opt out of it by not using that ScrollRestoration component.

Here are the Remix docs on ScrollRestoration: https://remix.run/docs/en/v1/api/remix#scrollrestoration.

It also looks like they try to run the scroll restoration behavior before hydrating the React app:

In order to avoid (usually) the client-side routing "scroll flash" on refresh or clicking back into the app from a different domain, this component attempts to restore scroll before React hydration. If you render the script anywhere other than the bottom of the document the window will not be tall enough to restore to the correct position.

Strepto commented 2 years ago

Just out of curiosity I looked a bit at this and found this (see link below). Might it actively "work against" the browser default scroll state restoring?

https://github.com/dillonkearns/elm-pages/blob/d51b9f0cc15281c815c608f367b45b23d7910cbe/generator/src/Pages/Internal/Platform.elm#L493

I did not look at the code long enough so see if its triggered both on "link presses" and on "browser navigation", but it kinda feels like it does.

Maybe disabling this on browser history navigation could be a solution?

Strepto commented 2 years ago

When running the hackernews demo locally (from branch serverless-latest 723f0b22a1901ece38fd9cb177664a5037454e56 ) I get the expected scroll position when navigating back. Not sure why its different, might be that it loads fast enough for the browser keep the scroll position while running from localhost?

Edit: now I'm confused as I cannot reproduce the localhost behaviour after doing some experimental changes and reverting them....

But adding a conditional ScrollToTop only if the navigation was from a link seems to work okish in my veeery limited testing

-- In Platform.elm  UpdateCacheAndUrlNew
-- snip
  , if fromLinkClick then
      ScrollToTop

    else
      NoEffect
-- snip

makes it somewhat more like what I expect in my browser. But I have not tested for compability elsewhere, or with any other app than the hackernews demo.

j-maas commented 8 months ago

My two cents from #437:

I noticed a few issues with how back navigation is handled with respect to scrolling.

  1. When I scroll down on a page, open a link, and using the browser's back button go back to the previous page, the scroll position is reset to the top of the page. Normal browser behavior is to remember the scroll position when navigation back.
  2. When I click the back button, there is a short flash where it seems that the page is scrolled to the top and then the content is changed. Ideally, the content change and scrolling would happen simultaneously.

For handling the scroll restauration on back navigation, react-router seems to follow this strategy:

  1. On each navigation, store the scroll position in session storage.
    • This probably is not necessary for fragment changes, because to my knowledge the browser can handle those even in single page application, since they usually happen on the same (logical) page.
  2. When navigating back, the scroll position is retrieved from the session storage and scrolled to.