Skip to content
This repository has been archived by the owner on Sep 7, 2020. It is now read-only.

fix scroll behaviour when clicking on anchors #1195

Closed
wants to merge 6 commits into from
Closed

fix scroll behaviour when clicking on anchors #1195

wants to merge 6 commits into from

Conversation

fazouane-marouane
Copy link

Hi,

We're having issues with internal links generated from markdown documents. Links to the same documents (normal html anchors) did not work as expected: when clicking on the links, no scrolling was done.

This was mainly due to a "event.preventDefault()". That's what this PR is all about.

@MoOx
Copy link
Owner

MoOx commented Nov 10, 2017

Sorry for the delay!

Shouldn't we check that it's the same page? What happen if a click on a link with an anchor to another page?

@MoOx
Copy link
Owner

MoOx commented Nov 14, 2017

@fazouane-marouane up :)

@fazouane-marouane
Copy link
Author

Hi

I can try checking against the current location first. But the scroll probably won’t happen in the target page.
Sorry I’ve got nothing better to propose.
If linking to outside pages with anchors is something that’s done usually, then maybe we need a more advanced Link component.
What are your thoughts on this?

@fazouane-marouane
Copy link
Author

@MoOx The proposed fix should be Okay now. I changed the Link component and I based it on react-router-hash-link.
Please, tell me what changes should I make to the PR mergeable.

Copy link
Owner

@MoOx MoOx left a comment

Choose a reason for hiding this comment

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

I think we should try to keep previous component and adjust like you started to. RR4 (via rr4dom) is not an option imo.
What was the problem if your initial change except that it didn't support hash on other urls? Seems not that hard to adjust, but I may miss something

@@ -31,6 +31,9 @@
"isomorphic-fetch": "^2.2.1",
"prop-types": "^15.5.8",
"react-hot-loader": "^3.0.0-beta.7",
"react-router-dom": "^4.2.2",
Copy link
Owner

Choose a reason for hiding this comment

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

This is not an option for us. We can't use RR v4, see #1130

Copy link
Author

Choose a reason for hiding this comment

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

Well, finally someone came up with a solution not on phenomic side. you can find it here parisjs/parisjs-website@a044ffb

It consists of having a hashLinkScroll function:

function hashLinkScroll() {
   const { hash } = window.location
 
   if (hash !== '') {
     setTimeout(() => {
       const id = hash.replace('#', '')
       const element = document.getElementById(id)
       if (element) element.scrollIntoView()
     }, 0)
   }
 }

Then, you'd use it like so <Router ... onUpdate={ hashLinkScroll } > ...</Router>.
So maybe, we just need to document this or find a way to automatically inject the hashLinkScroll in phenomic once and for all. What are your thoughts on this?

@MoOx
Copy link
Owner

MoOx commented Mar 15, 2018

I guess documentation about his could be great. I will start to document the project in the coming weeks and will include this when I will add support for this. Thanks for digging into this problem!

@MoOx MoOx closed this Mar 15, 2018
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.

None yet

2 participants