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): Respect hash as source of truth for scroll position #28555

Merged
merged 2 commits into from
Dec 9, 2020

Conversation

sidharthachatterjee
Copy link
Contributor

Currently we do not navigate correctly to hashes unless the scroll position is zero. If you're scrolled to any position other than the top, you will need to click twice to scroll to the correct position. This behaviour is completely broken at the moment.

This PR fixes this (albeit not completely) by always respecting the hash and scrolling to it irrespective of if there is browser scrolling state or not. This is iteratively better than our current behaviour. However, the ideal behaviour (which we ought to handle in a separate PR) is to respect hash except when navigating back in the browser (in which case, we ought to restore scroll position). Reach currently has this bug which we can track at reach/router#228

Closes #25778

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 9, 2020
mxstbr
mxstbr previously approved these changes Dec 9, 2020
Copy link
Contributor

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

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

This is so much better than before! 💯 Let's see if we can fix the reach bug in the future, but this is a massive improvement over users having to double click on links to navigate to anchors.

@LekoArts LekoArts added topic: reach/router and Links and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 9, 2020
@KyleAMathews
Copy link
Contributor

Ooo nice! I'd noticed this too.

@sidharthachatterjee
Copy link
Contributor Author

@KyleAMathews Yeah, it was annoying one. @mxstbr and I paired on this earlier today.

@sophstad
Copy link

sophstad commented Jan 4, 2021

@sidharthachatterjee Thanks for this fix, very much appreciated! Was wondering about this:

the ideal behaviour (which we ought to handle in a separate PR) is to respect hash except when navigating back in the browser (in which case, we ought to restore scroll position)

Is there an issue/PR where we can track this work? Thanks!

@blueboxes
Copy link

Has a fix been merged in? I am using gatsby-react-router-scroll 3.7 and still seeing the issue

@TheDarkStrix
Copy link

TheDarkStrix commented Feb 20, 2021

Using gatsby-react-router-scroll gatsby-react-router-scroll": "^3.7.0" still doesn't fix the issue, is there anything that must be done specific to get this working ?

the current implementation :

<Link to={"#contact"} > Contact </Link>

in the other div :
<div id="#contact" >.....</div>

@1-800-jono
Copy link
Contributor

I can also confirm that this is not working using gatsby-react-router-scroll": "^3.7.0"

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.

Internal anchors with hashes not working
8 participants