next.js icon indicating copy to clipboard operation
next.js copied to clipboard

<Link> not scrolling top sticky header

Open LouisGS44 opened this issue 1 year ago β€’ 7 comments

Link to the code that reproduces this issue

https://codesandbox.io/p/devbox/header-no-scroll-nextjs-x84kd3?embed=1&file=%2Fapp%2Flayout.tsx

To Reproduce

  1. Start the application in development (next dev)

Current vs. Expected behavior

A sticky header is present at the top of the layout (it has an opacity of 0.8 for viewing the page below). If the scroll height is lower than the header height and I click on the link to another page, the scroll doesn't return to the top of the page.

On the other hand, if the scroll height is greater than the header height, then the scroll does return to the top of the page as expected.

https://github.com/vercel/next.js/assets/25907732/5b86dd8d-12e3-4d3c-9065-91b037b7dd5d

Provide environment information

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 23.1.0: Mon Oct  9 21:27:27 PDT 2023; root:xnu-10002.41.9~6/RELEASE_X86_64
  Available memory (MB): 16384
  Available CPU cores: 12
Binaries:
  Node: 20.9.0
  npm: 10.2.5
  Yarn: N/A
  pnpm: N/A
Relevant Packages:
  next: 14.2.0 // There is a newer version (14.2.1) available, upgrade recommended! 
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.4.5
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

App Router

Which stage(s) are affected? (Select all that apply)

next dev (local), next build (local), next start (local)

Additional context

No response

NEXT-3099

LouisGS44 avatar Apr 13 '24 09:04 LouisGS44

Some scuffed workaround for me is to use {position: "fixed", width: "100%"} in layout.tsx instead of {position: "sticky"} and manually add a paddingTop to each individual page component. In your case:

export default function Home() {
  return (
    <div style={{ paddingTop: 150 }}>
      <div style={{ height: 1200, backgroundColor: "brown" }}>Page 1</div>
      <div style={{ height: 700, backgroundColor: "purple" }}></div>
    </div>
  );
}

This way the top of each {children} is actually aligned with the top of DOM and router will scroll to top on navigation.

Javad9s avatar Apr 13 '24 11:04 Javad9s

Yes thanks I'm using this solution in my production application but I have nested layouts that I can't use. Some of my pages have the same heading so I have to create a component and import it at the top of every pages. It works but it doesn't use the power of nested layouts

LouisGS44 avatar Apr 13 '24 14:04 LouisGS44

This behavior occurs because the browser doesn't automatically reset the scroll position when you navigate to a new page within the same application. To resolve this issue, you can reset the scroll position whenever a new page is loaded. Instead of using position-fixed, use a higher-order component to reset the browser scroll position.

"use client";
import { usePathname } from "next/navigation";
import { useEffect } from "react";

const ResetLayout = ({ children }) => {
  const pathname = usePathname();

  useEffect(() => {
    const handleRouteChange = () => {
      window.scrollTo(0, 0);
    };
    handleRouteChange();

    return () => {};
  }, [pathname]);

  return <>{children}</>;
};

export default ResetLayout;

AzizTareq295 avatar Apr 13 '24 17:04 AzizTareq295

The element check for if it should scroll (specifically topOfElementInViewport https://github.com/vercel/next.js/blob/87fb29ee7143c1e9a4c129585f9546c3f5e0b2b8/packages/next/src/client/components/layout-router.tsx#L151) returns true even though it's not fully in the viewport because of the position: sticky element. We'll have to figure out some special handling for that case πŸ€”

timneutkens avatar Apr 13 '24 19:04 timneutkens

I've noticed some funniness with this sort of thing too, it's good to see an issue for it.

@timneutkens if you or your team look into this further then perhaps one option could be to take into account the value of the CSS property scroll-padding-top on the scroll parent (e.g. html in many cases) when working out if an element is visible or not in topOfElementInViewport.

It's a modern CSS way of preventing a sticky header from overlapping content that you scroll to via anchor links, scrollIntoView() etc, so quite a few users with a sticky header will have it set to the height of their sticky header.

Although, some people also use scroll-margin-top on ::target so it wouldn't be a universal fix.

philwolstenholme avatar Apr 20 '24 19:04 philwolstenholme

What an annoying issue. I read in the documentation that it's supposed to scroll to the top of the page (i.e. barely scroll to cover the top of whatever the page.tsx outputs), but this is clearly not what people always want. I want to put a simple padding in my layout so that the page fits neatly in the site, but when I do that it no longer scrolls to the top. I have to wrap the padding or page in a custom component that I have to import into every page, which defeats the purpose of having the layout in the first place. I also tried using a template, but since it goes off of pages, this didn't work either.

I should be able to override the default behavior and have more control over where the scroll boundaries are. I am unfortunately not experienced enough to contribute myself, but I would look into that.

haraldev01 avatar Jun 24 '24 08:06 haraldev01

hello everyone may this help you guys const pathname = usePathname(); useEffect(() => { window.scrollTo(0, 0); }, [pathname]);

aiqbalsyah avatar Jul 02 '24 12:07 aiqbalsyah

The element check for if it should scroll (specifically topOfElementInViewport

https://github.com/vercel/next.js/blob/87fb29ee7143c1e9a4c129585f9546c3f5e0b2b8/packages/next/src/client/components/layout-router.tsx#L151

) returns true even though it's not fully in the viewport because of the position: sticky element. We'll have to figure out some special handling for that case πŸ€”

It happens even on: Next.js Documentation. It is related to the common pattern of having a sticky navigation and the check done in topOfElementInViewport because it does not match the UX expectation to scroll to the top.

With such sticky navigation, it is usual to set scroll-padding-top: var(--header-height); and could be a sign that it is intended to be used in the calculation.

const scrollPaddingTop = Number(window.getComputedStyle(document.documentElement).scrollPaddingTop) || 0;

artola avatar Jul 09 '24 07:07 artola

Some sites use scroll-margin-top on ::target, so checking for scroll-padding-top on html or body wouldn't be a universal fix.

Maybe Next could check for the value of a specific CSS custom property and it could be up to the individual Next.js site to keep that custom property updated and correct?

philwolstenholme avatar Jul 09 '24 08:07 philwolstenholme

I am facing the similar issue. Any cleaner solution yet?

Edit by maintainer bot: Comment was automatically minimized because it was considered unhelpful. (If you think this was by mistake, let us know). Please only comment if it adds context to the issue. If you want to express that you have the same problem, use the upvote πŸ‘ on the issue description or subscribe to the issue for updates. Thanks!

utkarshuday avatar Sep 06 '24 16:09 utkarshuday

Same problem here - I managed to solve this issue by moving the navigation component from the layout to each single page. Not gonna lie this makes me angry, hope the next.js team will do something with this bug. πŸ˜’

The weird thing that happened in my case it that this bug was present only on mobile devices.

real-marty avatar Nov 16 '24 14:11 real-marty

Same problem here - I managed to solve this issue by moving the navigation component from the layout to each single page. Not gonna lie this makes me angry, hope the next.js team will do something with this bug. πŸ˜’

The weird thing that happened in my case it that this bug was present only on mobile devices.

You only need to move the margin/padding which make up for the sticky header to each individual page

welschjohannes avatar Dec 06 '24 16:12 welschjohannes

At least this issue is still open...

This is the only fix that worked for me after wasting the entire day testing out tons of different things.

This scrolls to top on every navigation except for back/forwards navigations, so you still restore scroll position for those.

use-scroll-fix.ts

import { usePathname } from "next/navigation"
import { useEffect, useRef } from "react"

export function useScrollFix() {
    const pathname = usePathname()
    const isStatePopped = useRef(false)

    // Handling the scroll position to ensure clicking on the links
    // scrolls the page to the top with the sticky positioned navbar.
    useEffect(() => {
        const onPopState = () => (isStatePopped.current = true)

        window.addEventListener("popstate", onPopState)
        return () => window.removeEventListener("popstate", onPopState)
    }, [])

    useEffect(() => {
        if (!isStatePopped.current) {
            // navigation occurred without pressing
            // the browser's back or forward buttons
            window.scrollTo(0, 0)
        } else {
            isStatePopped.current = false
        }
    }, [pathname])
}

daveycodez avatar Feb 04 '25 07:02 daveycodez

You should not use useEffect for layouts, just add a div with the height that is equal to the sticky/fixed navigation bar at the top. So, your typical fix for your typical server rendered page is adding a div with the height of the sticky/fixed element. You would need to add this div to all pages. h-navbar is the height of the fixed navigation bar at the top:

// app/about/page.tsx your typical route page. 

export default async function AboutPage(): Promise<JSX.Element> {
  return (
    <>
      {/* fixesNext.js not scrolling back to the top when navigating to this page: */}
      <div className="lg:h-navbar" />
      <About />
    </>
  )
}

I believe adding this div to layout.tsx does not fix the bug. You have to put it into page.tsx

welschjohannes avatar Feb 04 '25 07:02 welschjohannes

I was testing something like that and still had issues. Either way it’s too many extra lines of code and overhead for my taste. My hook is one line in my layout and it works. Hopefully they fix the bug one day.

daveycodez avatar Feb 04 '25 08:02 daveycodez

All work on PPR should be paused for this basic functionality. Plus static dynamic routes.

daveycodez avatar Mar 19 '25 18:03 daveycodez

The element check for if it should scroll (specifically topOfElementInViewport

next.js/packages/next/src/client/components/layout-router.tsx

Line 151 in 87fb29e

function topOfElementInViewport(element: HTMLElement, viewportHeight: number) { ) returns true even though it's not fully in the viewport because of the position: sticky element. We'll have to figure out some special handling for that case πŸ€”

Interesting way of scroll to the top (by finding the first element, then scrolling up till the top of this element is in the viewport). Why not reset the full page scroll to 0?

Netail avatar Mar 19 '25 19:03 Netail

Interesting way of scroll to the top (by finding the first element, then scrolling up till the top of this element is in the viewport). Why not reset the full page scroll to 0?

It's to allow for pages that have nested/multiple layouts within them, like this tabs example: https://app-router.vercel.app/layouts/electronics or https://app-router.vercel.app/parallel-routes

If you go to https://app-router.vercel.app/layouts/electronics and arrange your browser window so the page is scrollable, then scroll down and click back-and-forth between the 'Phones' and 'Tablets' links then you'll make the page change tabs and the pathname will update, but Next won't scroll to the very top of the page each time. This is something that's lost in lots of the 'make a useEffect that watches usePathname and calls scroll(0,0) on pathname change' suggestions in threads like this one.

What Next is actually doing is finding the first element within the current layout and scrolling to the top of it. That allows for long pages that have nested layouts within them for things like tabs or similar setups where we don't want to send the user to the top of the page each time they click on a link that might only change a small part of the page.

philwolstenholme avatar Mar 19 '25 21:03 philwolstenholme

Why not reset the full page scroll to 0?

It's so easy. Don't care about top element to scroll to. Scroll to the top of the page, like every normal site since the 90s

daveycodez avatar Apr 06 '25 08:04 daveycodez

I found a solution, in the root layout add data-scroll-behavior="smooth"

export default function RootLayout({
  children,
}: Readonly<{
  children: React.ReactNode;
}>) {
  return (
    <html lang="en" data-scroll-behavior="smooth">
      <body>
        {children}
      </body>
    </html>
  );
}

and in the css:


html[data-scroll-behavior="smooth"] {
  scroll-behavior: smooth;
}

webdiego avatar Nov 09 '25 13:11 webdiego

Any progress?

The solution above didnt work for me.

vitaliyhan avatar Dec 02 '25 21:12 vitaliyhan