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

Link component does not persist scroll position #1309

Closed
migueloller opened this issue Feb 27, 2017 · 23 comments
Closed

Link component does not persist scroll position #1309

migueloller opened this issue Feb 27, 2017 · 23 comments

Comments

@migueloller
Copy link
Contributor

When using Link for routing and then pressing the browser's back button, the page will not remember the previous scroll position.

For example, go from page1 to page2 and then click the back button. page1 is rendered at the top of the viewport.

The browser is keeping track of the scroll position because if the page is refreshed it will refresh at the appropriate position. Is this an issue with the History API?

@rauchg
Copy link
Member

rauchg commented Feb 27, 2017

It might have to do with the latency of getting the data? I've been wanting to experiment with passing popStateEvent (the event from the popstate event) to getInitialProps as a way of letting the user optionally load the data from a cache that they maintain

@migueloller
Copy link
Contributor Author

Hmm, do you mean when using prefetch? I tried both with and without prefetch and the issue was still there.

I'm not sure how data latency might affect the scroll position of the viewport when going back a page. Do you mind explaining?

@rauchg
Copy link
Member

rauchg commented Feb 27, 2017

I was assuming that the browser has a heuristic for how long to wait for a scroll height to become available to scroll to.

@rauchg
Copy link
Member

rauchg commented Feb 27, 2017

Because, keep in mind that the popstate event is optimistic. We can't block the location from changing, it changes immediately (this is different from how push is handled).

Therefore, you press back, we start fetching data, that might take N milliseconds. I was assuming that browsers give up on preserving scroll if that N is sufficiently large.

@migueloller
Copy link
Contributor Author

migueloller commented Feb 27, 2017

Ah, I see. That sounds reasonable. What is the difference between clicking the back button or refreshing the page, though? Does clicking the back trigger the History API and you listen for that in order to fetch the page's data?

When the location changes immediately does it do another render server side, or is it all taken care of in the client? If it's done in the client then perhaps the browser might try to scroll before React mounts the component tree (when the viewport has no height). Which would make sense then that when you do a full refresh it works (the full refresh goes to the server and comes back with the entire markup).

@migueloller
Copy link
Contributor Author

migueloller commented Feb 27, 2017

I just took a look at what was going on behind the scenes and it does look that there are no network requests (other than static assets) when going back a page. This definitely sounds like it would make everything go much faster but it comes at the cost of losing the scroll position. Is there any way to avoid this?

@rauchg
Copy link
Member

rauchg commented Feb 27, 2017

At the time we always fetch data fresh. I'm thinking that if we expose said event to getInitialProps, we can write a decorator for fast back:

export default withPopstateCache(class extends React.Component {
  static async getInitialProps () {
    cont res = await fetch('api.com/this/will/be/cached/when/you/press/back')
    return res.json()
  }
})

but anyway, my rant could just be unrelated to the scroll position bug :P

@migueloller
Copy link
Contributor Author

Haha, I think that might work. Maybe also a way to simply skip the History API? So perhaps a way to not listen for the browser's back button and allow to set this as an option?

@stacygohyunsi
Copy link

Does anyone know why is it that when I added in this code the scroll position retains when I go from page1 to page2 and then click the back button:

componentDidMount () {
console.log(document.body.scrollTop);
}

@vinaypuppal
Copy link
Contributor

vinaypuppal commented May 11, 2017

@stacygohyunsi You can use scroll prop on link component for your use case like this <Link href="/path" scroll />
Ref: https://github.com/zeit/next.js/blob/master/lib/link.js#L55-L69

@coluccini
Copy link
Contributor

@vinaypuppal is there a way to achieve this while using router.push?

@vinaypuppal
Copy link
Contributor

@coluccini Yes

Router.push('/path').then(() => window.scrollTo(0, 0))

@coluccini
Copy link
Contributor

Oh, I didn't know that router return a promise :) Thanks!

@arunoda
Copy link
Contributor

arunoda commented May 24, 2017

Currently this is not implemented with Link. But if someone could send a PR I'd happy to take it.
For now, your Router.push() with your own logic.

May be someone could create a package like: next-scroll-link or something.

@arunoda arunoda closed this as completed May 24, 2017
@sojungko
Copy link

sojungko commented Nov 27, 2017

@migueloller I don't think I quite understood the solution @rauchg suggested. The premise is to cache the data on client-side to expedite rendering speed? Did you ever figure out a solution?

Also, @arunoda, you suggested someone could make a PR. Do you have any pointers as to which files or folders I should look at? I would love to make that contribution.

Currently, this is how I'm persisting scroll position with a route listener instance:

function initRouterListeners () {
  const scrollPositions = []
  let hasCompleted = false

  Router.onRouteChangeStart = () => {
    hasCompleted = false
    scrollPositions.push(window.scrollY)
  }
  Router.onRouteChangeComplete = path => {
    hasCompleted = true
  }

  window.onpopstate = popEvent

  function popEvent (e) {
    // sometimes this event fires first
    // and needs to wait for onRouteChangeComplete
    if (hasCompleted) {
      scrollTo()
    } else {
      setTimeout(popEvent, 20)
    }
  }

  function scrollTo () {
    // pop 2 and scroll to that
    scrollPositions.pop()
    const scrollPosition = scrollPositions.pop()

    if (scrollPosition) {
      window.requestAnimationFrame(() => window.scrollTo(0, scrollPosition))
    }
  }
}

This function is called when I bootstrap client. This only works sometimes because data may take too long to load and the browser will scroll to top by default.

I would like a way to persist the scroll position without having to optimizing my render function (I mean I could, but I want to know if there's a functional way of solving this issue).

@arunoda
Copy link
Contributor

arunoda commented Nov 27, 2017

@sojungko I think it's better to implement this as a user package. What you've done is correct.
Try to do the scrolling updates inside onRouteChangeComplete.
That'll be called when we completed getInitialProps (loads data).
May be we need to delay a bit as well. (Allow the browser to render data).

You can also listen to router events like this: https://github.com/zeit/next.js/blob/canary/client/on-demand-entries-client.js#L8

That's a best suited method for a package like this.

@fmaylinch
Copy link

fmaylinch commented Nov 1, 2018

Hi @sojungko , can you please tell me where I should execute that function?

EDIT: I think I found out, that the place to run that code is the _app.js. I didn't know about this because I just discovered this framework.

Also, did you find a better solution? Thanks!

@fmaylinch
Copy link

fmaylinch commented Nov 1, 2018

@arunoda About your last comment, how do we know in onRouteChangeComplete that the back button was clicked and we're not just going to another page?

Or, is there a better way to handle this issue now?

@karanmartian
Copy link

@fmaylinch did you find the right way to solve this? i m struggling with this problem too since several days. Any help will be greatly appreciated.

@fmaylinch
Copy link

fmaylinch commented Sep 11, 2019

Sorry @karanmartian, I saw your message and forgot to reply. Today I found your message again.

I am not working on the project now, but I did this hack to solve it. This code is in my _app.js file, but outside of my App class.

export default class MyApp extends App {
    ....
}

initRouterListeners();

function initRouterListeners() {

    console.log("Init router listeners");

    const routes = [];

    Router.events.on('routeChangeStart', (url) => {
        pushCurrentRouteInfo();
    });

    Router.events.on('routeChangeComplete', (url) => {
        fixScrollPosition();
    });


    // Hack to set scrollTop because of this issue:
    // - https://github.com/zeit/next.js/issues/1309
    // - https://github.com/zeit/next.js/issues/3303

    function pushCurrentRouteInfo() {
        routes.push({pathname: Router.pathname, scrollY: window.scrollY});
    }

    // TODO: We guess we're going back, but there must be a better way
    // https://github.com/zeit/next.js/issues/1309#issuecomment-435057091
    function isBack() {
        return routes.length >= 2 && Router.pathname === routes[routes.length - 2].pathname;
    }

    function fixScrollPosition () {

        let scrollY = 0;

        if (isBack()) {
            routes.pop(); // route where we come from
            const targetRoute = routes.pop(); // route where we return
            scrollY = targetRoute.scrollY; // scrollY we had before
        }

        console.log("Scrolling to", scrollY);
        window.requestAnimationFrame(() => window.scrollTo(0, scrollY));
        console.log("routes now:", routes);
    }
}

@tam315
Copy link

tam315 commented Sep 11, 2020

Hi, I hope this helps someone.

My requirements:

  • Restore scroll position when pop (when back or forward button clicked)
  • Scroll to top otherwise (For example, on the first load or when navigating with the Link component)

My solution:

// inside _app.tsx

scrollPositionRestorer()

function scrollPositionRestorer() {
  const scrollMemories: { [asPath: string]: number } = {};
  let isPop = false;

  if (process.browser) {
    window.history.scrollRestoration = 'manual';
    window.onpopstate = () => {
      isPop = true;
    };
  }

  Router.events.on('routeChangeStart', () => {
    saveScroll();
  });

  Router.events.on('routeChangeComplete', () => {
    if (isPop) {
      restoreScroll();
      isPop = false;
    } else {
      scrollToTop();
    }
  });

  function saveScroll() {
    scrollMemories[Router.asPath] = window.scrollY;
  }

  function restoreScroll() {
    const prevScrollY = scrollMemories[Router.asPath];
    if (prevScrollY !== undefined) {
      window.requestAnimationFrame(() => window.scrollTo(0, prevScrollY));
    }
  }

  function scrollToTop() {
    window.requestAnimationFrame(() => window.scrollTo(0, 0));
  }
}

@karanmartian

This comment has been minimized.

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Feb 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests