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

[Bug?]: Add option to disable scrolling to the top for a Link component #9054

Open
1 task done
jakubfiglak opened this issue Aug 22, 2023 · 5 comments
Open
1 task done
Labels
bug/needs-info More information is needed for reproduction

Comments

@jakubfiglak
Copy link

jakubfiglak commented Aug 22, 2023

What's not working?

Navigating through the app with the Link component always scrolls the page to the top even if the only thing that's changed is the query parameters. This is not always desired, for example when you use query parameters to store the filters state on the page. It would be great if there was an option to pass scroll: false or something similar to the Link component and navigate calls (this is how it's done in Next.js - https://nextjs.org/docs/pages/api-reference/components/link#scroll-1)

This is related to #9004 where there's a comment that a decision has been made to reset a scroll position on query string params change but there should be an option to opt out from this behavior in my opinion.

How do we reproduce the bug?

Repo: https://github.com/jakubfiglak/redwood-scroll-repro
Live: https://redwood-scroll-repro.vercel.app/
Screen recording:

Screen.Recording.2023-08-22.at.12.10.40.mov

What's your environment? (If it applies)

No response

Are you interested in working on this?

  • I'm interested in working on this
@jakubfiglak jakubfiglak added the bug/needs-info More information is needed for reproduction label Aug 22, 2023
@xmonkee
Copy link

xmonkee commented Aug 24, 2023

I would also like this option. I have some tabs on some pages, and they use URL hashes for navigating between them, and this causes a very jarring effect.

@jakubfiglak I am currently doing the following to "fix" this: Remember the scroll position and scroll back to it on navigation. It's causes a barely visible flash sometimes.

On the parent component:

const scrollY = useRef(0)

useEffect(() => {
    setTimeout(() => {
      window.scrollTo(0, scrollY.current)
    }, 0)
}, [queryParams])

And replace Link with button with the following

onClick={() => {
    props.scrollY.current = window.scrollY
    navigate(href)
 }}

@jakubfiglak
Copy link
Author

I would also like this option. I have some tabs on some pages, and they use URL hashes for navigating between them, and this causes a very jarring effect.

@jakubfiglak I am currently doing the following to "fix" this: Remember the scroll position and scroll back to it on navigation. It's causes a barely visible flash sometimes.

On the parent component:

const scrollY = useRef(0)

useEffect(() => {
    setTimeout(() => {
      window.scrollTo(0, scrollY.current)
    }, 0)
}, [queryParams])

And replace Link with button with the following

onClick={() => {
    props.scrollY.current = window.scrollY
    navigate(href)
 }}

Hey @xmonkee if you're having this issue with hash parameters as well, probably the only thing you have to do is to update Redwood to the latest version. This behavior's been changed for hashes with #9004. I've updated my reproduction project with the example using hash parameters so you can see it's working properly now.

Thanks for the suggested workaround but it feels very hacky to me. I'm gonna continue to look for something else, no offence! I even already have something that restores the scroll position to the desired value but then the scroll bounces back and forth and it's really annoyinh.

@jtoar
Copy link
Contributor

jtoar commented Sep 11, 2023

Hey @jakubfiglak, sorry for the late response here. What you're proposing sounds sensible—are you still interested in opening a PR? I'd be happy to review.

@jakubfiglak
Copy link
Author

Hey @jakubfiglak, sorry for the late response here. What you're proposing sounds sensible—are you still interested in opening a PR? I'd be happy to review.

Hey @jtoar no worries! Yup, I'm still interested, will try and push something up this week.

@jtoar
Copy link
Contributor

jtoar commented Sep 13, 2023

@jakubfiglak awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/needs-info More information is needed for reproduction
Projects
None yet
Development

No branches or pull requests

3 participants