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

feat(router): Add option to not reset scroll to the top on navigate/link #11380

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

guitheengineer
Copy link

@guitheengineer guitheengineer commented Aug 28, 2024

Problem

After every route navigation, when the page being rendered mounts, redwood scrolls to the top.
This is the desired behavior for most navigations, but not for a param update, where the same page is being rendered and just the param changes.

A common use case for this is using route params as state, e.g an id param that sets the current item being viewed. In this case we don’t want our browser to scroll to the top because they may lose the position in which they were previously when seeing those records
(e.g user is deep down seeing the 36th item on a table, after clicking on a row, they get scrolled to the top and lose where they were)

See reproduction of the issue below. When opening something like a sidebar that sets the id param, the scroll resets to the top.

Screen.Recording.2024-08-28.at.4.02.28.PM.mov

Note that redwood already avoids scrolling to the top if its a hash param update, but is not always that a hash is used for these cases.

Solution

Implement a scroll option on the navigate and link. Defaults to true, when set to false, won’t scroll to top.

navigate(`?id=${id}`, { scroll: false })

<Link to={`?id=${id}`} options={{ scroll: false }} />
Screen.Recording.2024-08-28.at.4.03.17.PM.mov

This would solve #9054

@guitheengineer guitheengineer changed the title feat(router): Add option to not scroll to top on navigate/link feat(router): Add option to not reset scroll to top on navigate/link Aug 28, 2024
@Josh-Walker-GM Josh-Walker-GM added the contributor-pr Use to automatically add PRs to the "Contributor PRs" Project Board label Aug 28, 2024
@Josh-Walker-GM
Copy link
Collaborator

Hey @guitheengineer 👋 Thanks for this! We'll probably need to have a discussion about this as a team just because it changes/introduces a user facing interface. Given our schedule, it could be a week before I have any good feedback on this 😬 Just know that we will get back to you within the week and we very much appreciate the contribution!

@guitheengineer guitheengineer changed the title feat(router): Add option to not reset scroll to top on navigate/link feat(router): Add option to not reset scroll to the top on navigate/link Aug 30, 2024
@guitheengineer
Copy link
Author

guitheengineer commented Sep 1, 2024

Hey @guitheengineer 👋 Thanks for this! We'll probably need to have a discussion about this as a team just because it changes/introduces a user facing interface. Given our schedule, it could be a week before I have any good feedback on this 😬 Just know that we will get back to you within the week and we very much appreciate the contribution!

Sounds good!

For context, we used redwood and had to develop our own "hack" to get over the scroll reset in several pages

  useEffect(() => {
    const original = window.scrollTo
    window.scrollTo = () => {}

    return () => {
      window.scrollTo = original
    }
  })

An option to have a scroll: false exists in popular alternatives like react-router, next router, etc and would solve our problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor-pr Use to automatically add PRs to the "Contributor PRs" Project Board
Projects
Status: To Process
Development

Successfully merging this pull request may close these issues.

2 participants