-
Notifications
You must be signed in to change notification settings - Fork 27.5k
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
Router.push('/link') don't scroll top of the page when triggered #3249
Comments
whether it should scroll to top or not is decided by the user. I think you can do it in |
I agree there should be a |
|
Thanks @gralagland your solution seems not so ugly, but it would be nice to have such option to keep DRY principe. The only fast solution for now is to make live template to write this 😅 |
Woaw... You mean I need to review every single page to add |
@MBach you can also hook |
I just want to chime in and say I found this behavior extremely surprising. Been building production sites with Next.js for almost a year now and only just now discovered that And in terms of matching default behavior like |
@gragland it doesn't work |
To make it work globally you can use the built-in next.js router event listener: |
@timneutkens Is there a reason why the default scroll behavior for the imperative |
Router.back() doesn't return promise, but has the same issue, what to do, manually scrolling up? |
@alexsenichev Did you try @macmenak solution? |
|
Hi. Why would we have the I think this issue should be revisited. Sorry but tagging the contributors: @liweinan0423 @timneutkens @exogen @goldenshun |
It would be nice if there was an option on |
@markjackson02 I don't see how this would be a breaking change since this is the default way browser would have handle this. I do not agree with the fact that we have two different approaches for the same functionality. |
It would definitely be a breaking change. People have been building apps with the way it is now and expect it to function that way. I'm not against them both having the same default functionality but I don't think it's worth the migration effort. Anyway, this issue is closed so we should open a new issue with whatever we want to do. |
@markjackson02 OMHO, the breaking change is that it's not working that way yet! The wrong behavior's already made. So I think it would be a good idea to put it the way that it should. But I agree with you, contributors will probably not give attention here. |
@Timer @timneutkens Also, got bitten by the difference in behavior of |
You can use this to route and scroll to the top of the page: // route using nextjs router and scroll to the top of the page
const routeToTop = (router, ...args) => {
if(typeof window !== 'undefined') window.scrollTo(0, 0)
return router.push.apply(router, args)
}
// usage
const router = useRouter()
routeToTop(router, '/my/page')
routeToTop(router, '/articles/[slug]', `/articles/${article.slug}`)
routeToTop(router, {
pathname: '/search',
query: {q: searchQuery},
}) Btw, this isn't as nice of a user experience as the |
I added a related issue here for those interested: #15206 |
FWIW, here's my take on a workaround that was shared in a related issue. import { useRouter } from 'next/router';
import { useEffect } from 'react';
/**
* React hook that forces a scroll reset to a particular set of coordinates in the document
* after `next/router` finishes transitioning to a new page client side. It smoothly scrolls back to
* the top by default.
*
* @see https://github.com/vercel/next.js/issues/3249
* @see https://github.com/vercel/next.js/issues/15206
* @see https://developer.mozilla.org/en-US/docs/Web/API/ScrollToOptions
* @param {ScrollOptions} [options={}] Hook options.
* @param {ScrollBehavior} [options.behavior='smooth'] Specifies whether the scrolling should animate smoothly,
* or happen instantly in a single jump.
* @param {number} [options.left=0] Specifies the number of pixels along the X axis to scroll the window.
* @param {number} [options.top=0] Specifies the number of pixels along the Y axis to scroll the window.
*/
function useRouterScroll({ behavior = 'smooth', left = 0, top = 0 } = {}) {
const router = useRouter();
useEffect(() => {
// Scroll to given coordinates when router finishes navigating
// This fixes an inconsistent behaviour between `<Link/>` and `next/router`
// See https://github.com/vercel/next.js/issues/3249
const handleRouteChangeComplete = () => {
window.scrollTo({ top, left, behavior });
};
router.events.on('routeChangeComplete', handleRouteChangeComplete);
// If the component is unmounted, unsubscribe from the event
return () => {
router.events.off('routeChangeComplete', handleRouteChangeComplete);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [behavior, left, top]);
}
export default useRouterScroll; Use it somewhere at the top of your component tree ( // _app.js
function MyApp({ Component, pageProps }) {
// Make sure pages scroll to the top after we navigate to them using `next/router`
useRouterScroll();
/* ... */
}
export default MyApp; |
@nfantone when does these fields change? |
@rakeshshubhu This only affects navigation through |
You don't have to. I instead create one module and export it to those components which need it. So once it needs change you change that one single place. Also you notice am passing the router as param since you can only use React Hooks e.g. useRouter() inside a component
|
Here's a simplified version of @nfantone's
|
@Timer would you accept a PR for adding a flag/parameter to |
This pull request makes `Router#push` and `Router#replace` function identically to `<Link />`, i.e. reset scroll when the new render is complete. Users can opt out of this new behavior via: ```tsx const path = '/my-page' router.push(path, path, { scroll: false }) ``` --- Fixes vercel#3249
This pull request makes `Router#push` and `Router#replace` function identically to `<Link />`, i.e. reset scroll when the new render is complete. Users can opt out of this new behavior via: ```tsx const path = '/my-page' router.push(path, path, { scroll: false }) ``` --- Fixes #3249
This is fixed on |
@Timer Hi, FYI this is not documented in https://nextjs.org/docs/api-reference/next/router#routerpush - Would be good to add there. |
Is it possible to make links work with new
|
Is NOT fixed with |
if you don't mind using const router = useRouter();
// ... do stuff
// page you want to go to, data you want to query, etc.
router.push(
// query params, URL, etc.
null,
{ scroll: false }
);
|
An old-school workaround from me. Just get the particular component in the view. Here's how. I use it with the router.push("/myaccount") for my peace of mind! I'm using react device detect. For some reason, in mobile browsers, it is not going to the top of the page after clicking router.push. The desktop browser is working fine. So, I wrapped my mobile menu bar Whatever happens, my component will be smoothly scrolled to the top! 😅 import React, { useEffect, useRef } from 'react';
function MyAccount() {
const containerRef = useRef(null);
useEffect(() => {
setTimeout(() => {
containerRef.current.scrollIntoView({ behavior: 'smooth' });
}, 250);
}, []);
return (
<div ref={containerRef}>
{/* It will be your particular component that
needs to be scrolled to the top after users click to router.push
In my case, it's menubar.
*/}
<MenuBar />
</div>
);
}
export default MyAccount; |
This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you. |
Hello,
maybe I am missing something, but when I use this function and redirect to another page scroll doesn't reset to top but the page loads scrolled to middle, I know why this issue happens, but can I somehow fix it with some flag like
JAVASCRIPT Router.pusht({pathname: '/link', scrollreset: true})
or something like this, didn't find anything similar in documentationThe text was updated successfully, but these errors were encountered: