Skip to content

Fix React scroll restoration on popState #2357

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sebastiandedeyne
Copy link
Contributor

Using @inertiajs/react, the scroll position isn't being restored because Scroll.restore is called before the new page is rendered. Wrapping it in requestAnimationFrame solves the issue, but I'm not sure if this is okay to have for the other adapters.

@pascalbaljet
Copy link
Contributor

Thanks for this! Can confirm that this fixes it. I'll add a test and check if we need this on the other packages as well.

@pascalbaljet
Copy link
Contributor

Hmm, I can't seem to break it in the test app, just in the playground, so I have to dig a little bit deeper 🤔

@sebastiandedeyne
Copy link
Contributor Author

Not sure of this helps but it’s broken when the page you’re navigating back from a shorter page to a longer page.

Suppose page A is 1000px high and page B is 500px:

This is what should happen:

  • User lands on A, body is 1000px high
  • User scrolls to 750px
  • User navigates to B, body is 500px high
  • User clicks back
  • React renders A, body is back
  • Inertia scrolls to 750px ✅

This is what now incorrectly happens due to an issue in Inertia:

  • User lands on A, body is 1000px high
  • User scrolls to 750px
  • User navigates to B, body is 500px high
  • User clicks back
  • Inertia scrolls to 750px ❌ This doesn’t work because body is only 500px high, so it doesn’t scroll far enough
  • React renders A

In my PR I wrap the scroll in requestAnimationFrame. In practice this delays the scroll until the next React render, when page A is back on screen.​​​​​​​​​​​​​​​​

@pascalbaljet
Copy link
Contributor

I think the test app is just too simple for this problem to appear 😅 But I can reproduce it if I deliberately make the resolveComponent method slow, then you can see the timing issue where the scroll restoring has already happened before the page was loaded, so that should be fixed as well.

I'll add a fix for that and also check SSR compatibility, probably calling requestAnimationFrame directly will break SSR.

@pascalbaljet
Copy link
Contributor

pascalbaljet commented Jun 13, 2025

I fiddled around some more and it could be possible that the delay of requestAnimationFrame is not enough. Unfortunately, Vue would make this much simpler with nextTick(). I've tested with observing the body height, but that feels a bit weird in scroll.ts:

public static observeWindowResize(callback: () => void): VoidFunction {
  const observer = new ResizeObserver(callback)
  observer.observe(document.documentElement)

  return () => observer.disconnect()
}

public static restoreDocument(): void {
  if (typeof window === 'undefined') {
    return
  }

  const scrollPosition = history.getDocumentScrollPosition()
  const scrollToPosition = () => window.scrollTo(scrollPosition.left, scrollPosition.top)

  // The newest page may not have been rendered yet, so we need to wait for the document to resize
  // before scrolling to the saved position.
  if (scrollPosition.top > document.documentElement.scrollHeight - window.innerHeight) {
    const stopObserving = this.observeWindowResize(() => {
      // todo: check height again...
      scrollToPosition()
      stopObserving()
    })

    // Observe for 100ms to allow the document to resize
    setTimeout(stopObserving, 100)
    return
  }

  scrollToPosition()
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants