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

[using-page-transitions] Scrolls to top before animating #7921

Closed
bogdancss opened this issue Sep 6, 2018 · 11 comments · Fixed by #8061
Closed

[using-page-transitions] Scrolls to top before animating #7921

bogdancss opened this issue Sep 6, 2018 · 11 comments · Fixed by #8061
Labels
status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby

Comments

@bogdancss
Copy link
Contributor

On the using-page-transitions example, the page scrolls to top just before fade-out animations starts.
If you have a long page, and click a link to trigger a route change, it immediately scrolls to top of the current page, then the fade-out animation begins, then the new page loads in.
I see the gatsby-react-router-scroll is a gatsby dependency - is there a way to pass a timeout so you don't get that page flash?

This behaviour can be checked here https://www.kaizen.co.uk/

@kakadiadarpan
Copy link
Contributor

Hi @b0gd4n, can you also provide the information with regards to package versions by running gatsby info --clipboard?

@kakadiadarpan kakadiadarpan added the status: needs more info Needs triaging and reproducible examples or more information to be resolved label Sep 7, 2018
@bogdancss
Copy link
Contributor Author

@kakadiadarpan here's the clip:


  System:
    OS: macOS 10.14
    CPU: x64 Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 8.10.0 - /usr/local/bin/node
    Yarn: 1.9.2 - /usr/local/bin/yarn
    npm: 6.4.1 - /usr/local/bin/npm
  Browsers:
    Chrome: 69.0.3497.81
    Firefox: 61.0.1
    Safari: 12.0
  npmPackages:
    gatsby: next => 2.0.0-rc.6 
    gatsby-image: next => 2.0.0-rc.1 
    gatsby-plugin-canonical-urls: next => 2.0.0-rc.1 
    gatsby-plugin-catch-links: next => 2.0.2-rc.1 
    gatsby-plugin-favicon: ^3.1.2 => 3.1.2 
    gatsby-plugin-google-tagmanager: ^1.0.19 => 1.0.19 
    gatsby-plugin-layout: ^1.0.0-rc.3 => 1.0.0-rc.3 
    gatsby-plugin-mailchimp: ^2.2.3 => 2.2.3 
    gatsby-plugin-manifest: next => 2.0.2-rc.1 
    gatsby-plugin-netlify-cache: next => 1.0.0-beta0 
    gatsby-plugin-nprogress: next => 2.0.0-rc.1 
    gatsby-plugin-offline: ^2.0.0-rc.1 => 2.0.0-rc.1 
    gatsby-plugin-polyfill-io: ^1.0.4 => 1.0.4 
    gatsby-plugin-react-helmet: next => 3.0.0-rc.1 
    gatsby-plugin-sharp: next => 2.0.0-rc.3 
    gatsby-plugin-sitemap: next => 2.0.0-rc.1 
    gatsby-plugin-styled-components: next => 3.0.0-rc.1 
    gatsby-source-filesystem: ^2.0.1-rc.1 => 2.0.1-rc.1 
    gatsby-source-wordpress: next => 3.0.0-rc.1 
    gatsby-transformer-sharp: next => 2.1.1-rc.2 
  npmGlobalPackages:
    gatsby-cli: 1.1.58
    gatsby: 1.9.277

@bogdancss
Copy link
Contributor Author

This behaviour can be seen on the using-page-transitions example https://streamable.com/mzg5s
The only modifications I've made to the example was to add a few p with lorem just to make the pages longer.

@kakadiadarpan
Copy link
Contributor

Thanks for sharing the details and the video. We'll look into this.

@kakadiadarpan kakadiadarpan added type: bug An issue or pull request relating to a bug in Gatsby status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. status: inkteam to review and removed status: needs more info Needs triaging and reproducible examples or more information to be resolved labels Sep 7, 2018
@stefanprobst
Copy link
Contributor

I think this has to do with something gatsby-react-router-scroll does. You can try setting in gatsby-browser.js:

exports.shouldUpdateScroll = () => false

@bogdancss
Copy link
Contributor Author

@stefanprobst it def has something to do with that.
I added that line, but this disables scroll to top altogether. I just want to delay the scrolling just after the fade out happens.

@bogdancss
Copy link
Contributor Author

bogdancss commented Sep 9, 2018

I think might work

exports.shouldUpdateScroll = () => {
  setTimeout(() => { return true }, 300)
}

EDIT: scratch that - this does not work - still scrolls to top before fading out. I even tried 3000ms delay

@stefanprobst
Copy link
Contributor

As a workaround try this:

const transitionDelay = 250

exports.shouldUpdateScroll = () => false

exports.onRouteUpdate = () =>
  window.setTimeout(() => window.scrollTo(0, 0), transitionDelay)

@bogdancss
Copy link
Contributor Author

@stefanprobst this kind of works, but on page nav it always goes to top (even when navigating back). The point of the gatsby-react-router-scroll plugin is whenever you go back to a page, it will scroll to wherever you were in the previous page. I would still want this, but just after a bit of a delay.

@stefanprobst
Copy link
Contributor

@b0gd4n AFAIU what happens in using-page-transitions is that RTG can animate the page transition only when the location has already changed and thus the scroll position already updated. So I can think of two possible solutions. Either
(1) make ScrollContext accept a delay. Not sure what the best way to do this would be, but wrapping this line in a setTimeout (and leaving everything else default) seems to do exactly what you want.
Or (2) update the scroll position manually in shouldUpdateScroll. To get access to the saved scroll positions we would need to pass them on here I guess:

const results = apiRunner(`shouldUpdateScroll`, {
  prevRouterProps,
  pathname,
  savedScrollPositions: this._stateStorage
})

And then access them in shouldUpdateScroll like so:

exports.shouldUpdateScroll = ({ pathname, savedScrollPositions }) => {
  const pos = savedScrollPositions.read({ pathname })
  // Update position
  return false
}

(3) Without any of that , for now the best I could think of is this:

const transitionDelay = 250

exports.shouldUpdateScroll = ({ pathname }) => {
  // // We use a temporary hack here, see #7758
  if (window.__navigatingToLink) {
    window.setTimeout(() => window.scrollTo(0, 0), transitionDelay)
  } else {
    const savedPosition = JSON.parse(
      window.sessionStorage.getItem(`@@scroll|${pathname}`)
    )
    window.setTimeout(() => window.scrollTo(...savedPosition), transitionDelay)
  }
  return false
}

Would be good to also check for initial render with location.key === 'initial' but atm we only get the pathname in shouldUpdateScroll, and onRouteUpdate is not called when navigating with brower buttons (which should be fixed soon)

@ryanwiemer
Copy link
Contributor

@stefanprobst since #8061 got merged in what is the recommended way now to set a delay on the scroll position during the page transition? Is there example code anywhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants