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

Treatments and Services routing #44

Merged
merged 6 commits into from
Dec 15, 2023
Merged

Treatments and Services routing #44

merged 6 commits into from
Dec 15, 2023

Conversation

vpsx
Copy link
Collaborator

@vpsx vpsx commented Dec 11, 2023

Some notes on the hash link stuff

(Before this PR: )
Hash links do not work well with React bc elements are often not yet available in DOM on load.
The behavior was slightly better in Firefox than in Chrome, but still suboptimal. (In Firefox it would only jump to the element whose id is in the hash if the page was already loaded and you hit enter on the browser url bar a second time; in Chrome, doing that would just refresh the page, and it never jumped to the relevant element. Plus other small subtle differences.)
Historically people used rafgraph/react-router-hash-link to solve this problem, but that is incompatible with React Router v6.

(This PR: )
Adds & uses React Hash Link https://www.npmjs.com/package/react-hash-link

(Notes: )

This didn't work: https://dev.to/mindactuate/scroll-to-anchor-element-with-react-router-v6-38op
This didn't work: https://ncoughlin.com/posts/react-router-v6-hash-links/

React Router Hash Link is the historical solution but doesn't work with React Router v6: https://www.npmjs.com/package/react-router-hash-link
...but one day it might?: rafgraph/react-router-hash-link#92 (comment)

React Router considers this functionality outside its domain: remix-run/react-router#7780 (comment)

@vpsx vpsx requested a review from gregfrasco December 12, 2023 16:12
@vpsx vpsx merged commit 8f44fec into main Dec 15, 2023
2 checks passed
@vpsx vpsx deleted the ts-routing branch December 15, 2023 17:37
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.

2 participants