feat: basic support for anchor links #238
Conversation
There was a problem hiding this comment.
Thanks for this PR. While I agree this can be an incredibly important feature, the implementation seems to be a bit hacky and there are a few issues I can see with it.
Every time you set window.location.hash, a new entry in the history stack is added. This will likely break the back button navigation and will cause undefined behaviors.
I would recommend a solution that is based on element.scrollIntoView(). For example, given a "sub-hash", it would wait for the component to be mounted then trigger a navigation to document.getElementById(subHash).scrollIntoView().
I also would prefer a different name so it's not confused with the hash that is used to identify the router. How about anchor? (Open to other suggestions)
| /** | ||
| * Readable store that returns the current sub hash | ||
| */ | ||
| export const hash = derived( |
There was a problem hiding this comment.
Good idea to export the sub-hash, but I would name it differently, such as anchor?
| if (window.location.hash.match(/^#[^\/]\w*/)) { | ||
| if (!lastLoc) { | ||
| // Could be another behavior, but this one seems right to me | ||
| window.location.hash = '#' |
There was a problem hiding this comment.
As explained, this creates a new entry in the history stack, so it will break back navigation.
| returnToHash = returnToHash + '?' + lastLoc.querystring | ||
| } | ||
|
|
||
| window.location.hash = returnToHash |
| // we want to scroll to the id in question and then | ||
| // revert back to the intended route | ||
| if (newLoc.hash && lastLoc === null) { | ||
| window.location.hash = newLoc.hash |
|
@NoahCardoza any follow-up? May I take care of polishing the PR? |
|
@carbogninalberto Take it way! @ItalyPaleAle Sorry for going dark. Totally lost track of this PR and am too busy to work on it at the moment. I'd love to see it be fixed up though! |
Thank you, the next few days I'm going to fix it up! |
|
Any chance of an update on this? Is currently blocking my documentation navigation |
Based on the discussion in #157, and @vigenere23's example: I've added some basic support to anchor links.
Full disclosure, I haven't tested every edge case, only done enough for what my personal project needs. However, if you have any suggestions I'd be happy to incorporate them.
I also didn't bother listing since it seemed the project wasn't passing when I first cloned it and didn't want to create a mess.
If anyone else needed this functionality before it get's merged you can install it via: