Scroll position behaviour not as expected with gatsby navigate

[t2ca]

I have a page with a list of links to filter data and the following useEffect hook is causing me problems:

  useEffect(() => {
    const {
      location: { pathname, search },
    } = props

    const queryString = qs.stringify({ filters: filters })

    if (search !== `${queryString}`) {
      navigate(`${pathname}?${queryString}`, { replace: true })
    }
  }, [filters])

The issue is that every time i click on a link, the scroll position jumps to the top. Im expecting the scroll position to remain the same after clicking the link, which i believe should be the normal behaviour.

I was able to solve this issue by adding the following code to gatsby-browser.js

exports.shouldUpdateScroll = () => {
  if (window.__navigatingToLink === true) {
    return [0, 0]
  }
  return true
}

exports.onRouteUpdate = () => {
  window.__navigatingToLink = false
}

However, since gatsby-react-router-scroll was updated to version 2.0.7, the above code is no longer working for some reason. Im able to downgrade gatsby-react-router-scroll to version 2.0.6 to fix this problem but i'm looking for a real solution. Thank you

[pieh]

Can you provide more complete reproduction? Ideally one that can be cloned and run to see the issue?

[KyleAMathews]

http://gatsby.dev/reproduction for instructions

[t2ca]

The issue is that when clicking the button the scroll position jumps to the top.

index.js and gatsby-browser.js are the only files modified.

Once you downgrade gatsby-react-router-scroll to version 2.0.6

The scroll behaviour is as expected.

Thank you!

[narration-sd]

And, I may see the reason for what's happening here. NOT a good thing, if so..

• the change from gatsby-react-router-scroll 2.0.6 to 2.0.7 is indeed where the regression happened, at least as visible here. Lock 2.0.6, and works again.
• just from the commit note on 2.0.7, apparently the idea is to restrict scroll memory to history back-button only (??)
• this, apparently also, has something to do with a more obscure issue set, Improve location key for scroll-behaviors so it's not purely derived from the path #12390 - Different wrappers for different content #135 , though I'm not seeing this yet, if not digging in
• but if you need this 2.0.7 functionality, please make it or reversion a settable option!
• that's the way, right?
• don't-regress suggests the default should be original behavior, but you can sort that out by your own view, as far a needs here are concerned, thanks - as long as the option is provided so memory always happens, exactly in the previous 2.06 way.

Why is this important? Well, it's a bit too much to fully explain, but I'm providing live preview editing for Gatsby pages before they're (re-)built, when driven by a remote page data service, and this requires reloading the current page at the current url. So, no back button or route change occurs.

It's critical that an editor not see their page jump around. That's what I need to preserve, and with thanks, and please.

[narration-sd]

For some reason, the note I placed on the 2.0.7 mod isn't getting referenced here, maybe because that's not an issue post, so you can connect to those messages here: #12403 (comment)

[gatsbot]

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.

If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

Thanks for being a part of the Gatsby community! 💪💜

[narration-sd]

Right on time, as I'm about to release a complicated product, and it must not have its screen jumping around.

I've been leaving it pinned to the version before this bug showed up, and will check if there's been any silent repair.

Meanwhile, kindly do not close this issue, thanks.

[narration-sd]

(@KyleAMathews, an intermediate message deleted, as a cache apparently fooled tests, and this mod still causes the same trouble. I'll be back...)

[narration-sd]

@KyleAMathews Ok, I've fixed this for our purposes, so shipping can happen. First the fix for others interested, and then a comment or two may not go amiss.

• If you are trying to get your page to use the current scroll position it, by looking at the list Gatsby is using, this will work now, after gatsby-react-router-scroll changes at version 2.0.7. Something like:

import ScrollStorage from 'gatsby-react-router-scroll/StateStorage.js'

class yourReactComponent {

  scrollStore = new ScrollStorage()

  componentWillUpdate = () => {  //  put in also any of its args you will use
      this.location = window.location // window is safe here
      this.location.key = 'initial' // this is what targets the grrs 2.07 change
      try {
        const currentPosition = this.scrollStore.read(this.location, null)
        if (currentPosition) {
          yourDevLog ('scrolling to: ' + JSON.stringify(currentPosition))
          let [x, y] = currentPosition
          window.scrollTo(x, y)
        }
      } catch (e) {
        yourDevLog ('no currentPosition yet: ' + e)
      }
  }
}

That's I think pretty bullet-proof, so here's comment:

• honestly, the routine modified for the 2.07 changes is a very obscure piece of code, even as improved by someone of good heart. What it's hiding is apparently a clip job to marry up other systems that never bothered to be compatible. One would think you could at least unroll it a bit, and comment what it's doing and why.

Because of these difficulties, it's very clear why @pieh created a passel of tests to accompany the change. That was well done, respectful in the circumstance.

• I found I couldn't just PR the problem routine, so it would option return to expected behavior, because this would unravel why the 2.07 change was made. I'm actually going to be doing more than shown here, to become as independent as possible of any other Gatsby interiorities that might suddenly change, while this gets us out the door.

• Looking a bit further, this whole 'must return to top' thing has clearly been a problem for reach-router and Gatsby. People think they want this. They obviously always don't -- it's just one possible state, and in those cases a convenience. Reach-router actually provides code which can make it work either way.

• I submit that it would be very good then for Gatsby to implement this reach-router feature, and so that individual pages or groups of them can have either always-to-top or what-it-was-before scroll behavior. Even blog-makers will thank you, according to case, and for those doing more, it's often going to be essential. Nobody should have to hack in on your interior components as I have done.

• finishing, I realize it's common for you to expect a) a squashed test case b) an idea what the real usage is. I haven't provided, because I'm doing something unusual, making a and b both difficult, and not a help. It's pretty useful, though, what this 'unusual' accomplishes, if it takes a great deal else to get it going. I think not a detriment to your cause, at the least.

Regards,
Clive

[gatsbot]

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.

Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.

As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks again for being part of the Gatsby community!

[narration-sd]

@KyleAMathews

Just to close this off, the solution for a need I posted just above doesn't work for a home page. Thus I simply stopped using Gatsby's mechanism at all; simple to provide one's own.

Why this failed appears to show up interesting if not improper scroll memory behavior in the Gatsby system now, after the 'use location.key' modification.

I've detailed that in another posting, where there actually was some attention paid, and won't comment further here. See for the info: #12560 (comment)