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

fix(gatsby-react-router-scroll): Properly scroll to 0 for new pages #25749

Merged
merged 2 commits into from
Jul 14, 2020

Conversation

blainekasten
Copy link
Contributor

Description

This fixes #25745 and #25522.

The problem is that on page load, the location key is a constant of initial. So when you navigate between two pages that uses full page reloads - the key is going to be initial between each page which then matches in the state and reuses the value.

The solution is to add the pathname to the key. So the key that we store in localStorage for a scroll position for a page should always include pathname and key if it's provided.

Related Issues

Fixes #25745
Fixes #25522

@blainekasten blainekasten requested a review from a team as a code owner July 14, 2020 16:18
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 14, 2020
@blainekasten blainekasten removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 14, 2020
@gatsby-cloud-staging
Copy link

Your pull request can be previewed in Gatsby Cloud: https://build-8628de84-c5dc-44d5-af51-caad8747964f.staging-previews.gtsb.io

Copy link
Contributor

@sidharthachatterjee sidharthachatterjee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Blainey ❤️

@sidharthachatterjee sidharthachatterjee added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Jul 14, 2020
@gatsbybot gatsbybot merged commit 4ca8f05 into master Jul 14, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix/scrolling-on-load branch July 14, 2020 17:25
@ediblecode
Copy link
Contributor

Thanks for fixing this. I've just upgraded gatsby and run into what I think is issue #25522 so very much waiting for this fix. I can't find any information about Gatsby release cycles/how long it takes to get a new version onto npm after a merge - please can anyone advise when this will be available?

@ediblecode
Copy link
Contributor

Ignore my previous comment: just seen 2.24.3 was published a couple of minutes ago https://github.com/gatsbyjs/gatsby/releases/tag/gatsby%402.24.3. Thanks all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect scroll top position when navigating between pages Gatsby Scroll Issues on Page Change/Link Click
4 participants