feat: Opt out scrolling upon a hash change
Discussion as a feature in https://github.com/TanStack/router/discussions/1155
Adds ability to opt out of behavior where the router (specifically, the DOM/UI library implementation) handles scrolling to hash element matching the current hash history.
I am driven to add this new router option because I am currently working with an existing code-base which controls the current router hash location based on the browser's current scroll position.
A minimum reproduction case where the scrollIntoView behavior conflicts with the desired behavior to control the router location externally is located at:
https://stackblitz.com/edit/tanstack-router-cyyfgk?file=src%2Fmain.tsx
Hi! Can we also add a similar configuration flag to your PR that allows you to scroll to the hash smoothly? And can you tell me why your PR hasn't been reviewed yet?
Been going through our open PRs, and regarding this one, I've got a question about why this implementation is required.
This functionality can be achieved using the getKey callback function that you can pass into the <ScrollRestoration> component.
It has the added benefit of being customizable by the end user to suit their exact scroll restoration requirements.
In this case, you could use to opt-out of a hash change triggering the scroll restoration.
<ScrollRestoration
getKey={location => {
return [location.pathname, location.search].join(",")
}}
/>
Perhaps, we can include a section in the Scroll Restoration documentation on some popular recipes for using the getKey callback to get this behaviour and others.
Thanks for the thoughtful consideration!
Been going through our open PRs, and regarding this one, I've got a question about why this implementation is required.
It looks like in your comment you are proposing a way to use the ScrollRestoration API in order to mimic the behavior of scrolling on hash router change?
Thanks for pointing me towards the Scroll Restoration capabilities documentation, as this revealed a new thought to perhaps approach this issue as an 'opt in' capability.
I.e., using the useElementScrollRestoration hook and while removing the existing scrollIntoView behavior:
useElementScrollRestoration(
getElement() { return document.getElementById(router.location.hash) }
getKey(location) { return [location.pathname, location.search, location.hash].join(",") }
)
However, this would present the following difficulties vs. adding a new router configuration option:
- This would opt users out of a likely 'sensible default' behavior to scroll users to the hash on route changes, especially after transitions complete.
- This would probably be considered a breaking change, whereas adding the proposed
scrollOnHashChangeoption would be considered a minor, additive change to theRouterOptionsAPI. - I agree that requiring opting in would require improved patterns documentation for situations like this; if this part of documentation continues to become enhanced, perhaps removing the hash-based
scrollIntoViewfunctionality from theRouterProviderwould indeed be more consistent with the TanStack's broader behavior for leaving the complexities of scroll state management to the user. I am certainly willing to assist in this as part of the feature development. - It is perhaps worth looking into the broader resliency of the
useElementScrollRestorationAPI in general before encouraging user's to further leverage it for advanced use-cases; I wonder if there are any potential gotchas with exposing this functionality to router users, such as whether it's possible to easily create conflicting scroll handlers. The spirit of the hook makes a lot of sense, but it's potential to fragment rather than co-locate side-effects on an important piece global state like scroll position perhaps lends itself to being located in a more centralized location (e.g. as a more customizable, albeit more complexRouterOptionsproperty).
I believe I'm understanding what you are going for.
But I'm a bit hesitant about widening our API surface unless we absolutely need to and as such I want to make sure we've covered the alternates before moving to adding a new piece of configuration to the Router as well as how it'd play along with the existing implementation and ScrollRestoration component.
Especially since something like scroll restoration behaviour could/would eventually need to become a per-route based configuration option as people start exploring this avenue.
With that in mind:
- Is any of this possible using the
ScrollRestorationcomponent? or does this have to be a Router configuration option? - Are you still basing your need for this configuration based on the original example? Or is it tied to something broader? (if so what is it)
- What are the impacts of using it with existing
navigateandScrollRestoration?
I've tweaked the main.tsx file from your original example, to use the navigate calls we'd see in a normal TSR app, and it does seem to cause some wonky behaviour scroll behaviours.
I hope the tweaked example helps you understand my hesitancy here.
But I'm a bit hesitant about widening our API surface unless we absolutely need to and as such I want to make sure we've covered the alternates before moving to adding a new piece of configuration to the Router as well as how it'd play along with the existing implementation and ScrollRestoration component.
AH, now I understand your motivations. I'm certainly motivated by an edge case where the code performing external hash changes is completely outside my own codebase, so I definitely own that that's an edge case that TS router shouldn't design around.
Others are liking and thumbs upping this idea still, so I'm wondering the other use-cases that are motivating folks.
I'll play around with the Scroll restoration API as is if there's any way for it to override the "hard coded" hash scrolling behavior. What I suspect is that the way the opinion is so hard coded into the Provider, that there will be scroll conflicts.
Perhaps then, an alternative is to have the Provider defer to the location key as a source of truth vs the hash 🤔. Will play around, and work on top of assumption that the user desires the "update history" behavior and has it in their control, vs my current assumption of it being externally controlled and rigid.
I have followed up in the related issue with a call for more concrete use cases where the current default scroll behavior is problematic
https://github.com/TanStack/router/discussions/1155#discussioncomment-8966186
@elliottgrieco-toast pinging for a checkup.
Checking the original discussion thread, discussions, and Discord questions, I haven't seen any movement on the need for this.
For navigation controlled outside of React, the people seem to be fine with consuming the router using its created instance or by using the useRouter hook.
Agreed -- an attempt was made; I'm ready to close this issue at your discretion.
Closing this for now because of the discussion above.
Thanks for the contribution, @elliottgrieco-toast!
Appreciate your understanding on this.