Skip to content
This repository has been archived by the owner on Feb 23, 2023. It is now read-only.

using reach router's navigate for hash links #169

Closed
wants to merge 2 commits into from

Conversation

jeesunikim
Copy link
Contributor

Problem:
hash-links-bug-480

On /docs' Run Core Node - configuring, for example, if you click links that are targeting hash links. They won't work most of the time (some of the hash links I clicked for the first time worked; I've tried multiple times on incognito, but happens randomly).

Gatsby's Recommendations for programmatic, in-app navigation recommends to use an anchor tag (which what we were already using) or reach/router's navigate.

I implemented navigate for hash links and this fixes the problem. But it does feel hacky since anchor tag should work so I'd like you guys' opinions.

It looks like there are scroll handling issues within Gatsby
Several Fixes for Scroll Handling and Restoration
Fix gatsby-react-router-scroll

@jeesunikim jeesunikim requested a review from vcarl June 22, 2020 21:21
@stellar-jenkins
Copy link

content/docs/run-core-node/configuring.mdx Outdated Show resolved Hide resolved
@@ -155,7 +155,7 @@ export const Link = ({ href, newTab, ...props }) => {
return <BasicLink href={url} {...props} />;
case LINK_DESTINATIONS.docs:
case LINK_DESTINATIONS.hash:
return <BasicLink href={url} {...props} />;
return <BasicLink onClick={() => navigate(url)} href={url} {...props} />;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm I'm not sure we should wholesale override the click behavior for BasicLink. I see there's a special case for hash links, maybe something needs to change there?

Copy link
Contributor Author

@jeesunikim jeesunikim Jun 23, 2020

Choose a reason for hiding this comment

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

Gotcha! I thought LINK_DESTINATIONS.hash would be for hash links. I like adding it to makeLinkedHeader! I tried with plain H2, H3, H4 by removing makeLinkedHeader but same thing still occurs. I can dig into it for another hour tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to update Gatsby and see if it fixes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some updates - hash anchor tags work fine in react components. Just the ones that are addressed in .mdx files seem to be broken.

src/constants/docsComponentMapping.js Show resolved Hide resolved
@stellar-jenkins
Copy link

@jeesunikim jeesunikim closed this Jun 29, 2020
@stellar-jenkins
Copy link

Something went wrong with PR preview build please check

@jeesunikim
Copy link
Contributor Author

PR 175

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants