feat: Add the ability to override scroll restoration without conditionally rendering the `ScrollRestoration` component
Closes: #2810
This PR adds a mechanism for preventing scroll restoration at the level of an individual link or usage of the location object. Here's how the current solution would be used in a link:
<Link to="#" state={{scroll: false}}>click me</Link>
And via the location object (using search params as an example):
const [searchParams, setSearchParams] = useSearchParams();
searchParams.set("a", "b");
setSearchParams(searchParams, { state: { scroll: false } });
This is still a WIP. I'm unsure how to test the change (as far as I can tell this component is still untested) and whether using the location state is the right way to override/stop the scroll restoration. As such, I haven't yet added tests or documentation. Nonetheless, this does fix a frustrating user experience for us (reproduced here) so I'd welcome any feedback or suggestions.
Also, all credit for this should go to @wladiston. I'm simply opening a PR to implement the solution he suggested because I'd like for our app not to rely on a local copy of the ScrollRestoration component's source code
- [ ] Docs
- [ ] Tests
Hi @filipemir,
Welcome, and thank you for contributing to Remix!
Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.
You may review the CLA and sign it by adding your name to contributors.yml.
Once the CLA is signed, the CLA Signed label will be added to the pull request.
If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].
Thanks!
- The Remix team
Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳
@chaance or @ryanflorence is there anything I can do to help get this or an alternative solution to this problem merged? Happy to receive feedback and attempt to implement it but I'd love to not have to maintain my own copy of ScrollRestoration fork indefinitely.
You don't have to maintain a fork. Just copy+paste the file and add it to your own app/components folder.
We're so used to just npm installing code, that we don't realize we can simply inline the code we want and customize it as needed.
I have my own <LiveReload/> component that fetches /build/__purge__ before refreshing the page. Then in my Express adapter I use that route to purge my cache instead of on every request.
@kiliman that's what I meant by maintaining a fork. I wasn't speaking about literally forking the remix repo, just that I have to maintain a copy of the LiveReload component in our own repo. The issue it creates is the same whether it's a repo fork or not: any substantive changes to LiveReload in Remix will have to be ported over to our code base as well. It makes every Remix update an opportunity for a regression here. And if the internals of ScrollRestoration ever change substantially it could become a real problem. I will keep doing this indefinitely if there's no progress on this ticket, but I think it would benefit the project to have an official solution to this problem.
Thanks @filipemir, we have actually been working on some changes to LiveReload internally which is why we haven't merged this just yet. It's on our radar and we'll let you know when anything changes.
Looks like I may have confused the issue. This was about <ScrollRestoration/> and I believe @filipemir is also referring to that, but typed <LiveReload/> as I used that as an example.
Anyway, yes I understand that code may change. But we have to be responsible for our own code either way, whether we write it or import it. I don't see the component changing all that much.
I'm not saying that the PR shouldn't be merged. I'm just saying that our priorities don't always match the team's despite what we may desire.
Thanks, @chaance. It's not a rush. I just wanted to make sure it wasn't lost in noise. I'll check back in some time if I haven't heard back. Thank for all the hard work :)
And yes, this is for <ScrollRestoration />. No issues with <LiveReload /> on my end
And yes, this is for <ScrollRestoration />. No issues with <LiveReload /> on my end
Please release this. I need this because it is so frustrating right now.
@DHFW my understanding is that the Remix team has a different fix in the works for this, but it requires an update on React router which is still pending. @chaance correct me if I'm wrong.
@filipemir Thank you so much for the effort 🙏🏼
This have been implemented as of Remix 1.10.0.
You can use the new preventScrollReset on <Link /> https://reactrouter.com/en/main/components/link#preventscrollreset