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

Scroll restoration happens too early before the page gets rendered after hitting browser back button #3303

Closed
1 task done
liweinan0423 opened this issue Nov 17, 2017 · 75 comments
Labels
locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.

Comments

@liweinan0423
Copy link
Contributor

liweinan0423 commented Nov 17, 2017

  • I have searched the issues of this repository and believe that this is not a duplicate.

After transiting from one page to another page via next <Link />, if user clicks the back button and the previous page has getInitialProps method that takes some time to finish, the scroll position of the previous will be restored before the previous pages gets rendered.

Demo

source code can be found here
out

After clicking back button, the "go back" text should still be visible (not scroll down to previous position) until the previous page gets rendered

@liweinan0423 liweinan0423 changed the title Scroll restoration happens too early before the page gets rendered when hitting browser back button Scroll restoration happens too early before the page gets rendered after hitting browser back button Nov 17, 2017
@pablopunk
Copy link

This seems to happen because next's router (and <Link />) uses window.history.pushState and then window.history.back restores the scroll. This doesn't happen with common <a /> because it doesn't use pushState.

This is an interesting article about how scroll restoration works on each browser. Basically you can disable it and manage the scroll on your own:

history.scrollRestoration = 'manual'

@liweinan0423
Copy link
Contributor Author

liweinan0423 commented Dec 28, 2017 via email

@pablopunk
Copy link

Yeah, I understand. I'm not sure what the solution would be from within next.

@pablopunk
Copy link

@liweinan0423 I'm not sure but this could be a workaround

@raffij
Copy link

raffij commented Feb 15, 2018

We've come across this too and it feels like something next should be doing internally. Otherwise everyone needs to write their own code to store scrollpositions and then scroll on completion of getInitialProps.

@mgiraldo
Copy link

mgiraldo commented Mar 6, 2018

we're having a related issue where, upon clicking a link from a scrolled-down position (say, result 20 in a list) the new page shows up halfway down so the user needs to scroll up to see the item itself.

@mgiraldo
Copy link

mgiraldo commented Mar 7, 2018

here's an example screen capture of the issue we're having:

scroll

@jlei523
Copy link

jlei523 commented Mar 13, 2018

We're also running into this issue and it makes the user experience very frustrating to use. @arunoda Any thoughts or way to fix this?

@spencersmb
Copy link
Contributor

I was having this same issue as @mgiraldo awhile back and I originally was just using router.push(), but as soon as I switched to using the tag, my problem resolved itself with out needing to implement a hack like scroll-to-top in component did mount or something ghetto like that. Curious if you are using a Link tag at all?

@mgiraldo
Copy link

it is a regular <Link /> see: https://github.com/dpla/dpla-frontend/blob/master/components/ExhibitionsComponents/AllExhibitions/components/ExhibitionsList/index.js#L8-L18

but we ended up using componentDidMount with window.scrollTo in the destination page: https://github.com/dpla/dpla-frontend/blob/master/pages/exhibitions/exhibition/index.js#L25-L27

feels very hacky, though... and we have the Back button issue mentioned in the issue in other parts of the site.

you can see the problem in action here: https://dp.la/exhibitions (scroll down and click)... the patch will be up tomorrow

@mgiraldo
Copy link

you can see the patched version here: https://beta-staging.dp.la/exhibitions

@jlei523
Copy link

jlei523 commented Mar 13, 2018

@mgiraldo I see a slight flash when using the back button. We're running into the same issue.

@spencersmb
Copy link
Contributor

spencersmb commented Mar 13, 2018

@mgiraldo In another part of my site I forgot I did this:

Router.push("/account")
window.scrollTo(0, 0)
document.body.focus()

which works when navigating TO a page, but like you said when you hit the back button, its at the top of the window because I think the window recorded that in its history....
What are you doing to mitigate that back button issue? Its like I can get one to work but not the other.

@jlei523
Copy link

jlei523 commented Mar 28, 2018

@spencersmb What do you mean switch to using the tag? What tag is that?

@arunoda
Copy link
Contributor

arunoda commented Mar 28, 2018

We've a similar issues reported here: https://spectrum.chat/thread/3d65b436-b085-4eef-ad84-0941c473e09d

Next.js doesn't hijack scrolling. But this is a real issue. Here's what's happening.
This is because of browser's default scrolling behavior. See: #3303 (comment)

When you go back, browser tries to go to the previous scroll position for better UX. But in the ^^ case, the data is not loaded yet. But the data will be there in a bit with the latency of getInitialProps(). When the data rendered, it's not the correct position as before.

So, in order fix this you can do a few things:

  • Cache the data locally and render the page from that cache. You can even update the data in behind the scene. In this case data will be there as the user hit the back button.
  • Control scrolling manually via Router.push(). (This will return a promise and it'll get resolved when the page is rendered)

We've no plan to manage scrolling inside Next.js yet. But you can do it in the userland and publish a new Link component like LinkBetterScrolling.

@arunoda arunoda closed this as completed Mar 28, 2018
@developer239
Copy link

developer239 commented Mar 28, 2018

I mean every time a javascript library gets popular and people start using it in production ... some majors issues show up and the official response is: "Fix it yourself." 😢

I fail to understand how does Router.push() fix scrolling issue that happens when you press button back in a browser.

@mgiraldo Posted a link to a patched version of his website.

  1. Go here: https://beta-staging.dp.la/exhibitions
  2. Scroll to the bottom
  3. Click on activism in the US
  4. Click on button back in your browser or just swipe back

There are two major issues:

  1. Before the list gets rendered. Item detail scrolls to the bottom for no reason.
  2. When the list gets rendered you are not at the bottom of the list but somewhere in the middle.

I believe it would be nice to at least consider fixing this issue in the future.

Other than that thanks for all the hard work that you put in nextjs. It might take a little longer but we are getting closer to stable libraries in javascript community.

@jlei523
Copy link

jlei523 commented Mar 28, 2018

@developer239 We have the exact same issue with our Next.js app as documented here: https://spectrum.chat/thread/3d65b436-b085-4eef-ad84-0941c473e09d

It completely kills the user experience.

Basically, the current page scrolls to the bottom of the page because it's trying to restore scroll position before the previous page is rendered.

@arunoda Could you explain why the Next team isn't considering an official fix to this issue? It seems like it's a breaking bug that completely ruins the user experience.

@mgiraldo
Copy link

mgiraldo commented Mar 29, 2018

if i understand correctly the back button/scroll position functionality is a native browser behavior that assumes a standard page refresh (user goes from url x to url y via a standard http request). what client-side javascript-based frameworks do is implement a page transition without making use of the native browser behavior (via pushState or similar). the browser is counting on itself having a cached version of the page to return you to, but it doesn't (it has an incomplete version that is completed by the framework after the jump is made). however, if your app is not fast enough to produce page transitions/content using the framework (in either direction, backwards or forward), you will get this jarring, second-rate experience.

what i think the nextjs team is saying is the effort of implementing a cross-browser solution to this problem is beyond the scope they are willing to take on.

@rus-yurchenko
Copy link

I think if a next.js team provides its own routing management they should care about similar critical issues as well. For example react-router doesn't have this issue and they don't think that is should be implemented by another developer themselves.
Definitely this should be fixed because it makes the user experience very frustrating and confusing.

@Emiliano-Bucci
Copy link

Emiliano-Bucci commented Nov 26, 2018

Hi to all! I think i solve this issue by following the advice of @pablopunk
history.scrollRestoration = 'manual' using it inside componentDidMount() inside the _app.js file.
However, after some minutes i removed it and the problem seems that has gone.

Strange thing, by using and removing it, it seems that it also fix the issue in a build that is deployed with now. Maybe is only a browser issue?

@vitaliy-webmaster
Copy link

vitaliy-webmaster commented Apr 13, 2019

I came across mentioned issue while building my portfolio site and I think this is really a big breaking thing in user experience overall.
I can confirm that problem occurs when the page to go back has more height than the current page. And (IMHO) this has nothing to do with getInitialProps as was stated in official next team response. I get similar bad result with or without using getInitialProps . More likely it happens because scroll restoration occurs before route change but not after it. Try to set simple page transitions and you'll see that scroll height jumps instantly despite the content of current page is still there.
In my case I found temporary solution. This involves setting the height of "html" tag to a very high value (like 15000px) on onRouteChangeStart event, and restoring its original height in 2 seconds after onRouteChangeComplete. This works for me and hope it will help someone until we get official fix. Good luck.

@hardwit
Copy link

hardwit commented Jul 1, 2019

My solution for restoring scroll position when the browser back button clicked:

_app.js

componentDidMount() {
    const cachedPageHeight = []
    const html = document.querySelector('html')

    Router.events.on('routeChangeStart', () => {
      cachedPageHeight.push(document.documentElement.offsetHeight)
    })

    Router.events.on('routeChangeComplete', () => {
      html.style.height = 'initial'
    })

    Router.beforePopState(() => {
      html.style.height = `${cachedPageHeight.pop()}px`

      return true
    })
}

@vitaliy-webmaster
Copy link

@hardwit well done!
however it works for me only if I restore height asynchronously like this:

Router.events.on("routeChangeComplete", () => { setTimeout(() => { html.style.height = 'initial' }, 1500); });

@malthemilthers
Copy link

My solution for restoring scroll position when the browser back button clicked:

_app.js

componentDidMount() {
    const cachedPageHeight = []
    const html = document.querySelector('html')

    Router.events.on('routeChangeStart', () => {
      cachedPageHeight.push(document.documentElement.offsetHeight)
    })

    Router.events.on('routeChangeComplete', () => {
      html.style.height = 'initial'
    })

    Router.beforePopState(() => {
      html.style.height = `${cachedPageHeight.pop()}px`

      return true
    })
}

Where does Router come from?

@ghost
Copy link

ghost commented Sep 7, 2019

This worked for me:

_app.js

  componentDidMount() {
    window.history.scrollRestoration = "manual";

    const cachedScroll = [];

    Router.events.on("routeChangeStart", () => {
      cachedScroll.push([window.scrollX, window.scrollY]);
    });

    Router.beforePopState(() => {
      const [x, y] = cachedScroll.pop();
      setTimeout(() => {
        window.scrollTo(x, y);
      }, 100);

      return true;
    });
  }

@lmf-git
Copy link

lmf-git commented Oct 3, 2021

I'm my project, sometimes I can't even scroll down after changing page in development... if it was a production issue I'd switch framework.

@styfle styfle modified the milestones: 11.x.x, 12.0.4 Nov 5, 2021
@timneutkens timneutkens added the Navigation Related to Next.js linking (e.g., <Link>) and navigation. label Nov 17, 2021
@timneutkens timneutkens removed this from the 12.0.5 milestone Nov 17, 2021
@veasna-ung
Copy link

Hi ! It seems to still not work on nextjs.org with Chrome iOS when I click on footer links.
I also tried with another projects based on Next 12 and got same issues.
Have you any solutions ?

Tested with Chrome IOS (v.96.0.4664.101) on 15.1 and 14.7.

Image.from.iOS.MP4

@dejesus2010
Copy link

Welp, I just ran into this issue 2 days before I'm scheduled to ship my project... None of the solutions work. Nextjs has been awesome, but this bug really sucks. Any chance this has been looked at recently?

@abhimanyuPatil
Copy link

Welp, I just ran into this issue 2 days before I'm scheduled to ship my project... None of the solutions work. Nextjs has been awesome, but this bug really sucks. Any chance this has been looked at recently?

@dejesus2010 Trying to get this solved since last month but still didn't get any solution.
Any help ?

@Kipitup
Copy link

Kipitup commented Aug 3, 2022

I'm having the same issue. I've added a setTimeout to delay the scroll, but this is obviously not a sustainable solution.

@reza-akbari
Copy link

reza-akbari commented Sep 7, 2022

next: 12.2.2
chrome: 105

module.exports = {
  experimental: {
    scrollRestoration: true
  }
}

this flag triggers replaceState on every scroll, it BROKE the browser back/forward buttons for me.
I ended up doing something like this: (in the _app.tsx)

  const router = useRouter();
  const scrollCache = useRef<Record<string, [number, number]>>({});
  const activeRestorePath = useRef<string>();
  useEffect(() => {
    if (history.scrollRestoration !== "manual") {
      history.scrollRestoration = "manual";
    }
    const getCurrentPath = () => location.pathname + location.search;
    router.beforePopState(() => {
      activeRestorePath.current = getCurrentPath();
      return true;
    });
    const onComplete = () => {
      const scrollPath = activeRestorePath.current;
      if (!scrollPath || !(scrollPath in scrollCache.current)) {
        return;
      }
      activeRestorePath.current = undefined;
      const [scrollX, scrollY] = scrollCache.current[scrollPath];
      const delays = [0, 10, 20, 40, 80, 100, 100, 100, 100];
      const checkAndScroll = () => {
        if (scrollPath !== getCurrentPath()) {
          return;
        }
        const docEl = document.documentElement;
        const maxScrollY = docEl.scrollHeight - docEl.clientHeight;
        const maxScrollX = docEl.scrollWidth - docEl.clientWidth;
        if (maxScrollX >= scrollX && maxScrollY >= scrollY) {
          window.scrollTo(scrollX, scrollY);
        } else {
          const delay = delays.shift();
          if (delay) {
            setTimeout(checkAndScroll, delay);
          }
        }
      };
      setTimeout(checkAndScroll, delays.shift());
    };
    const onScroll = () => {
      scrollCache.current[getCurrentPath()] = [window.scrollX, window.scrollY];
    };
    router.events.on("routeChangeComplete", onComplete);
    window.addEventListener("scroll", onScroll);
    return () => {
      router.events.off("routeChangeComplete", onComplete);
      window.removeEventListener("scroll", onScroll);
    };
  }, []);

hope it helps

@jsvelte
Copy link

jsvelte commented Sep 9, 2022

next: 12.2.2 chrome: 105

module.exports = {
  experimental: {
    scrollRestoration: true
  }
}

this flag triggers replaceState on every scroll, it broke the browser back/forward buttons for me. I ended up doing something like this: (in the _app.tsx)

  const router = useRouter();
  const scrollCache = useRef<Record<string, [number, number]>>({});
  const activeRestorePath = useRef<string>();
  useEffect(() => {
    if (history.scrollRestoration !== "manual") {
      history.scrollRestoration = "manual";
    }
    const getCurrentPath = () => location.pathname + location.search;
    router.beforePopState(() => {
      activeRestorePath.current = getCurrentPath();
      return true;
    });
    const onComplete = () => {
      const scrollPath = activeRestorePath.current;
      if (!scrollPath || !(scrollPath in scrollCache.current)) {
        return;
      }
      activeRestorePath.current = undefined;
      const [scrollX, scrollY] = scrollCache.current[scrollPath];
      window.scrollTo(scrollX, scrollY);
      // sometimes rendering the page can take a bit longer
      const delays = [10, 20, 40, 80, 160];
      const checkAndScroll = () => {
        if (
          (window.scrollX === scrollX && window.scrollY === scrollY) ||
          scrollPath !== getCurrentPath()
        ) {
          return;
        }
        window.scrollTo(scrollX, scrollY);
        const delay = delays.shift();
        if (delay) {
          setTimeout(checkAndScroll, delay);
        }
      };
      setTimeout(checkAndScroll, delays.shift());
    };
    const onScroll = () => {
      scrollCache.current[getCurrentPath()] = [window.scrollX, window.scrollY];
    };
    router.events.on("routeChangeComplete", onComplete);
    window.addEventListener("scroll", onScroll);
    return () => {
      router.events.off("routeChangeComplete", onComplete);
      window.removeEventListener("scroll", onScroll);
    };
  }, []);

hope it helps

This does not work for me bro!

@reza-akbari
Copy link

@jsvelte I updated code. after using it for a few days I noticed it doesn't work on IOS safari. I noticed calling window.scrollTo with any numbers was setting the window.scrollX and window.scrollY to exactly those numbers for a short time on safari.
now it's working fine so far. but using setTimeout isn't ideal anyway.

@MikeDrewitt
Copy link

MikeDrewitt commented Jan 25, 2023

I'm kinda at my wits end with this one. I'm not sure if I'm just having a configuration issue or what's going on, but surely the intended behavior is not to maintain scroll position when loading a new page?

Currently clicking any <Link /> or using router.push() results in the scroll positioned being maintained across pages regardless of whether or not that page had been loaded prior. (Next 13.0.6)

Is the intended behavior out of the box for <Link /> to maintain scroll position when loading a new page when clicked with href as its only prop?

From what I've been reading the answer seems like this is not the intended default behavior (which makes sense to me).

So my question then becomes, is this a bug? Or is this some configuration issue on my end? If it is a configuration issue, where can I look to either confirm/ fix that issue.

@simonhrogers
Copy link

Absolutely mental that this hasn’t been solved in four years! But there you go...

@zgreen
Copy link

zgreen commented Mar 21, 2023

Just noting that this also seems to be an issue with Next's own router.back.

@genikineg
Copy link

genikineg commented Mar 21, 2023

I settled on this version of the scroll

next.config.js

change scrollRestoration to false

const nextConfig = {
  experimental: {
    scrollRestoration: false
  },
  reactStrictMode: true,
}

_app.tsx

add to App function


  const router = useRouter()

  const scrollCache = useRef<Record<string, [number, number]>>({})
  const activeRestorePath = useRef<string>()

  useEffect(() => {

    if (history.scrollRestoration !== "manual") {
      history.scrollRestoration = "manual";
    }
    const getCurrentPath = () => location.pathname + location.search;
    router.beforePopState(() => {
      activeRestorePath.current = getCurrentPath();
      return true;
    });
    const onComplete = () => {

      const scrollPath = activeRestorePath.current;
      if (!scrollPath || !(scrollPath in scrollCache.current)) {

        window.scrollTo(0, 0)
        return;
      }

      activeRestorePath.current = undefined;
      const [scrollX, scrollY] = scrollCache.current[scrollPath];
      window.scrollTo(scrollX, scrollY);
      // sometimes rendering the page can take a bit longer
      const delays = [10, 20, 40, 80, 160];
      const checkAndScroll = () => {
        if (
          (window.scrollX === scrollX && window.scrollY === scrollY) ||
          scrollPath !== getCurrentPath()
        ) {
          return;
        }
        window.scrollTo(scrollX, scrollY);
        const delay = delays.shift();
        if (delay) {
          setTimeout(checkAndScroll, delay);
        }
      };
      setTimeout(checkAndScroll, delays.shift());
    };
    const onScroll = () => {
      scrollCache.current[getCurrentPath()] = [window.scrollX, window.scrollY];
    };
    router.events.on("routeChangeComplete", onComplete);
    window.addEventListener("scroll", onScroll);
    return () => {
      router.events.off("routeChangeComplete", onComplete);
      window.removeEventListener("scroll", onScroll);
    };
  }, [])

Link needs to disable scrolling

scroll={false}

@James-Lam
Copy link

It seems that the official documentation has not handled scrolling restoration well.

Screen.Recording.2023-06-21.at.14.06.00.mov
RPReplay_Final1687327053.mp4

@simonhrogers
Copy link

For anyone working with or without Framer – hopefully this helps as an easy solution. If you aren’t using Framer, you can follow Jakob Chill’s post here: https://jak-ch-ll.medium.com/next-js-preserve-scroll-history-334cf699802a

import { useEffect, useRef } from 'react'
import { AppProps } from 'next/app'
import { WindowInfoProvider } from '@faceless-ui/window-info'
import { ScrollInfoProvider } from '@faceless-ui/scroll-info'
import { LazyMotion, m, AnimatePresence } from 'framer-motion'
const framerFeatures = () => import("utils/framerFeatures").then(res => res.default)
import { SplashProvider, useSplashContext } from "context/splash"
import { PreviousRouteProvider } from "context/previousRoute"
import Layout from 'components/global/Layout'

import useFouc from 'hooks/useFoucFix'

import 'styles/sanity.css'
import 'styles/scss/main.scss'
import { useRouter } from 'next/router'


export default function App({ Component, pageProps }: AppProps) {

  // Scroll restoration
  // Next.js scroll restoration is not working with framer-motion
  // So we have to do it manually
  // I’ve adapted the kind work of Jakob Chill in order for it to work with framer-motion
  // The original article is here:
  // https://jak-ch-ll.medium.com/next-js-preserve-scroll-history-334cf699802a
  // if you weren’t using framer-motion, you could just use it as-is as a convenient hook
  // if I ever do a blog I’ll write a basic post about this incase anyone has better ideas

  useFouc()

  // Stop the browser from scrolling the exiting page to top before transition 
  useEffect(() => {
    if (history.scrollRestoration !== "manual") {
      history.scrollRestoration = "manual";
    }
  }, [])

  const router = useRouter()
  const scrollPositions = useRef<{ [url: string]: number }>({})
  const isBack = useRef(false)
  const currentRoute = router.pathname
  const baseRoute = currentRoute.split('/')[1]

  const onRouteChangeComplete = (url: any) => {
    if (isBack.current && scrollPositions.current[url]) {
      setTimeout(() => {
        window.scroll({
          top: scrollPositions.current[url],
          behavior: "auto",
        })
      }, 1)
    } else {
      setTimeout(() => {
        window.scroll({
          top: 0,
          behavior: "auto",
        })
      }, 1)
    }
    isBack.current = false
  }

  useEffect(() => {
    router.beforePopState(() => {
      isBack.current = true
      return true
    })
    const onRouteChangeStart = () => {
      const url = router.asPath
      scrollPositions.current[url] = window.scrollY
    }
    router.events.on("routeChangeStart", onRouteChangeStart)
    return () => {
      router.events.off("routeChangeStart", onRouteChangeStart)
    }
  }, [router])

  // delQuery removes the query string from the url
  // this is necessary to prevent framer motion from animating on query string changes

  function delQuery(asPath) {
    return asPath.split('?')[0]
  }

  return (
    <>
      {baseRoute && baseRoute === 'studio' ? (
        <Component {...pageProps} />
      ) : (
        <WindowInfoProvider
          breakpoints={{
            phone: '(max-width: 767px)',
            tablet: '(min-width: 768px)',
            laptop: '(min-width: 1224px)',
            desktop: '(min-width: 1748px)',
          }}
        >
          <ScrollInfoProvider>
            <PreviousRouteProvider>
              <SplashProvider>
                {/* <Layout {...pageProps}> */}
                  <LazyMotion features={framerFeatures}>
                    <AnimatePresence 
                      mode="wait" 
                      initial={false}
                      onExitComplete={() => onRouteChangeComplete(router.asPath)} 
                    >
                      <m.div
                        key={delQuery(router.asPath)} 
                        initial="pageInitial" 
                        animate="pageAnimate"
                        exit="pageInitial"
                        variants={{
                          pageInitial: {
                            opacity: 0
                          },
                          pageAnimate: {
                            opacity: 1
                          },
                        }}
                      >
                        <Component {...pageProps} key={router.asPath} />
                      </m.div>
                    </AnimatePresence>
                  </LazyMotion>
                {/* </Layout> */}
              </SplashProvider>
            </PreviousRouteProvider>
          </ScrollInfoProvider>
        </WindowInfoProvider>
      )}
    </>
  )
}

@claus
Copy link

claus commented Jun 22, 2023

It seems that the official documentation has not handled scrolling restoration well.

It's an infinite scroller, getting scroll restoration right with those is not trivial

@timneutkens
Copy link
Member

Good news: This issue has been solved in App Router, we've kept it in mind from the start when building the new router.
Unfortunate news: due to limitations of how the Pages Router and React work you can't reliably restore scroll as the client-side router did not have a cache that is reused when navigating back/forward, which means that e.g. getInitialProps / getStaticProps / getServerSideProps has to be re-executed to get the props for the page component. Theoretically this could be changed but it's very likely that would break existing applications.

For App Router we've changed this, the router is built around a client-side cache that is reused when navigating back/forward. In addition to that we've made changes in React to support React Transitions being synchronous when triggered in the popstate event. This ensures React blocks the main thread while rendering on popstate as browsers have a behavior where there's a time window before it commits on popstate in order for JS-based navigation to apply.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot added the locked label Aug 2, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked Navigation Related to Next.js linking (e.g., <Link>) and navigation.
Projects
None yet
Development

No branches or pull requests