thepassle / app-tools

129 stars 8 forks source link

[router] Recommended way of handling `#` hash changes navigation #26

Closed JulianCataldo closed 10 months ago

JulianCataldo commented 11 months ago

Hello! I really like how this router behave, having tried some other options before (lit router, nano store router…).

I tried to handle hash navigation, typically for a "table of contents with scroll to heading" feature.

Ideally, I'd like to prevent the router for handling this altogether, and preserve the native behaviour.

So after some fiddling :

None of this paths will allow me to have a "pass-through" for hash changes with scrolling, but maybe I've overlooked something 🤔

I can think of some ways to handle this, one of them is to ignore some links, with a local marker (data attributes) or in a centralised fashion; sometimes you don't want to cater for this in the DOM itself, especially if it's pre-rendered from an external source. IDK what the best, maybe both, or none? Other way: have a dedicated handling for hash changes in the library. But that adds more API surface, so that might be undesirable.

Thank you, and Happy New Year 2024!

thepassle commented 11 months ago

Would you mind posting an expected/actual behavior that youre trying to achieve to illustrate it a bit better for my understanding? Then I’ll try to see what we can do :)

JulianCataldo commented 11 months ago

When I click this:

<a href="#my-heading">My Heading</a>

I need it to scroll to:

<h2 id="my-heading">My Heading</h2>

Which is the default behaviour, instead of being picked by the SPA router (hence, doing nothing).

Maybe something a bit like:

<a href="#my-heading" data-router-ignore>My Heading</a>

Can do the trick.

thepassle commented 10 months ago

Hmm, right. So we could add a check for something like:

    if (url.hash !== this.url.hash) return;

so If you navigate from / to /#foo, or from /#bar to /#foo that will work. However, if you navigate from /foo to /bar#baz, we do need to handle the route change, so the hash likely wont work then. Im not really sure how other routers handle this 🤔 Do you happen to know?

JulianCataldo commented 10 months ago

I'm trying to find some missing pieces here:

https://github.com/nanostores/router/blob/c35b28f8b86f9a92e46f2dbf2070647358dfd077/index.js#L70C36-L70C51

window.dispatchEvent(new HashChangeEvent('hashchange'))

and window.history.pushState

@lit-labs/router isn't taking hash into account whatsoever, anywhere.

wouter has some extensive "additional parameters" support https://github.com/molefrog/wouter?tab=readme-ov-file#customizing-the-location-hook

I'll keep you updated on my findings.

JulianCataldo commented 10 months ago

Ok, it looks like we can't just scroll to an element when it's not part of the root document tree. https://stackoverflow.com/questions/43425398/anchor-tag-a-id-jump-with-hash-inside-shadow-dom

So I'll have to add a programmatic way of doing this anyway ;) Which was a thing I thought could be avoidable at first.

We can close issue this I think. "additional parameters" (query, hash…) can be an interested feature to look for, but I think one can manage with already available primitives (w. the lib. hooks and the browser native APIs).

thepassle commented 10 months ago

Would maybe be a good usecase for a plugin, let me know if you cook up something cool, maybe we can ship it from the core lib then :)

If its okay with you I'll close this issue then, feel free to reopen if you want to discuss more

JulianCataldo commented 10 months ago

There is a culprit I've found yesterday while using element.scrollIntoView + scrollToTop plugin. They clash together, as expected.

My quick solution is to add my own programmatic scroll to top in my app logic, and disable the plugin. The dirty solution is to add a delay, but that causes jumps, as expected.