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

Add scroll-related options to useRouter() #791

Open
9j opened this issue Jul 13, 2024 · 6 comments
Open

Add scroll-related options to useRouter() #791

9j opened this issue Jul 13, 2024 · 6 comments

Comments

@9j
Copy link

9j commented Jul 13, 2024

Hello,

I've noticed a bug-like behavior when handling state with query parameters. Every time the query changes, the scroll position resets to the top of the page. This can be disruptive to the user experience, especially on pages with a lot of content.
To address this, I suggest adding scroll-related options to the useRouter() hook. This would allow developers to control scroll behavior when navigating or updating query parameters.

If you're open to it, I'd be willing to work on this feature. Would you be interested in a pull request that implements this functionality?

Thank you for your consideration.

@dai-shi
Copy link
Owner

dai-shi commented Jul 14, 2024

Thanks for reporting.
We are pretty sensitive to add features with options to our API. (We generally prefer making API extensible instead.)
Can you simply fix the bug-like behavior?

waku_new_path: url.pathname !== window.location.pathname,

Ah, I see. So, the current behavior is intentional. Do you want to keep the scroll position if only query param changes? Can you investigate how other frameworks handle it?

@dai-shi
Copy link
Owner

dai-shi commented Jul 14, 2024

btw, I think waku_new_path is a hack. I wonder if there's better way of implementing it. Suggestions are welcome.

@9j
Copy link
Author

9j commented Jul 14, 2024

I've investigated this issue and found an interesting approach in Next.js. They use session storage to save scroll coordinates, as shown in this code:

https://github.com/vercel/next.js/blob/96092dd42dc8d7fe4ef0b6ea9889020b2da2af86/packages/next/src/shared/lib/router/router.ts#L948-L967

While this method might seem a bit hacky, it provides a smooth user experience. I think it would be beneficial if Waku could implement a similar feature, allowing developers to optionally decide whether to preserve scroll position when pushing new routes or updating query parameters.

I'd like to propose a new approach for Waku.

The proposed method is router.push(url, { preserveScroll: true }). This approach can effectively maintain scroll position without using session storage like Next.js does.

The advantages of this approach are:

  1. It can be implemented without significantly altering Waku's existing Router design.
  2. It provides developers with flexibility to control scroll behavior.

This method could be simpler yet effective. It also seems like a good way to improve user experience while maintaining Waku's philosophy of API extensibility.

I'd be happy to work on a pull request to implement this feature if you're interested. What do you think about this approach? Please let me know if there are any additional considerations or modifications needed.

@dai-shi
Copy link
Owner

dai-shi commented Jul 14, 2024

I agree that we shouldn't use sessionStorage.
What's the difference with our approach.
Can we do the same hack with waku_new_path without sessionStorage?

I agree adding an option unstable_preserveScroll (with unstable prefix) is another good option which allows a user more control, but I would like to know what is impossible without such an option.

@rmarscher
Copy link
Contributor

Yes, it would be nice for router.back() to have a similar option - or default to this behavior so it is similar to hitting the back button in the browser.

@dai-shi
Copy link
Owner

dai-shi commented Sep 15, 2024

Yes, we would like to make it the default behavior (prefer less options), using waku_new_path hack.

Would you like to work on it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants