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

React: ScrollReset is slightly out of sync with Page changes, causing a flash before navigating #1802

Open
oscarnewman opened this issue Feb 20, 2024 · 0 comments
Labels
react Related to the react adapter

Comments

@oscarnewman
Copy link

Version:

  • @inertiajs/react version: 1.0.14

Describe the problem:

I noticed an issue working on a new Inertia React/Laravel app where page navigations via Link would suffer what looked like a flash of content before moving to the next page.

(this is my fist time using Inertia and it's been an awesome experience, btw!)

Looking more closely, I found that the "scroll reset" step was occurring an instant before the page itself changed to the new content. This means that if a user is scrolled down at all on a page and they navigate to another page via Inertia, they'll see an instant where their current page scrolls all the way to the top before they see the next page.

On smaller/lighter weight pages, this is less noticeable. On heavier pages or on mobile, this was causing decent Jank.

I've attached a video of a simple reproduction in the React playground, and one from my own app.

It's less consistent/harder to notice in the React playground. I think this might be because the render pass is just faster with less content and is less likely to get out of sync. I recommend scrubbing through the video frame by frame slowly to see what's happening.

In the playground:

CleanShot.2024-02-19.at.20.54.19.mp4

In my own app:

trimmed.mov

Steps to reproduce:

  1. Create a page that causes the browser viewport to scroll
  2. Include a link to another page
  3. Click that link while scrolled down, and observe a flash of scroll-to-top before the transition

Reproduction Repo:

I've updated the "React Playground" in the repo to use a Sticky header -- this makes it possible to click a navigation link while scrolled down on a page.

That change is visible here: master...oscarnewman:inertia:reproduction

I'm able to reproduce the issue in about 1/4 navigations from the bottom of the "Article" page to any other page.

Root cause / Solution

I dug into the Inertia code a bit to see if I could figure out what's happening. I think it primarily occurs in this router.setPage function (comments are my annotations):

protected setPage(
    page: Page,
    {
      visitId = this.createVisitId(),
      replace = false,
      preserveScroll = false,
      preserveState = false,
    }: {
      visitId?: VisitId
      replace?: boolean
      preserveScroll?: PreserveStateOption
      preserveState?: PreserveStateOption
    } = {},
  ): Promise<void> {
    return Promise.resolve(this.resolveComponent(page.component)).then((component) => {
      if (visitId === this.visitId) {
        page.scrollRegions = page.scrollRegions || []
        page.rememberedState = page.rememberedState || {}
        replace = replace || hrefToUrl(page.url).href === window.location.href
        replace ? this.replaceState(page) : this.pushState(page)

        // `swapCompnent` is set by the consuming package (react, vue, etc). It is responsible for actually rendering a new page
        this.swapComponent({ component, page, preserveState }).then(() => {
          if (!preserveScroll) {
            this.resetScrollPositions() // We seem to have some sort of race condition with this resetScrollPositions function
          }
          if (!replace) {
            fireNavigateEvent(page)
          }
        })
      }
    })
  }

I was curious how other frameworks solved this, and look into React React Router's Implementation.

The key seems to be that they use useLayoutEffect to ensure that the scroll position is reset before the browser repaints the new dom/vdom.

The fundamental difference then seems to be:

In Inertia:

In React RouteR:

  • Change the react component/location (somehow)
  • Listen to a change on the location object and trigger a scroll reset in useLayoutEffect

This is a little awkward with Inertia's current model of what it delegates to the framework-specific packages and what it keeps in the core, as scroll management is entirely in the core. I'm not sure if this also happens with other frameworks yet, but at the very least I imagine the react package at least will need to become aware of when a scroll reset is necessary and handle calling reseScrollPositions() itself inside the react lifecycle.

I'm gonna try to work on a PR for this as well, but wanted to put the issue up for context

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

No branches or pull requests

1 participant