vercel / next.js

The React Framework
https://nextjs.org
MIT License
125.02k stars 26.7k forks source link

Route change when pages have smooth scroll enabled causes disorienting motion. #20125

Closed itaditya closed 3 years ago

itaditya commented 3 years ago

Bug report

Route change when pages have smooth scroll enabled causes disorienting motion.

Describe the bug

Setting scroll-behavior: smooth on html is great for scroll to pinned headings but the side-effect is that it interferes with scroll restoration that happens during route change. It's a jarring effect to see page scroll up as the content changes.

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  1. Go to https://devadi.netlify.app/blog
  2. Scroll down to the end of page.
  3. Click the last article link.
  4. It will smoothly scroll up but the content would be of the article page as url has changed. The effect is very unusual and jarring.

To reproduce it on your Next.js site, add this code to all global CSS file.

html {
    scroll-behavior: smooth;
}

Expected behavior

Screenshots

Screen Recording 2020-12-12 at 3 26 07 PM

System information

Additional context

A bigger video is present here https://twitter.com/dev__adi/status/1336583526565605376

DaniGuardiola commented 3 years ago

I second this. Let me know if you want another demo, I could share a video of my website as well, but I think the one by @itaditya is good enough.

I came up with this hacky but effective fix, just as an idea:

function disableSmoothScroll() {
  const el = document.createElement('style')
  el.textContent = '* { scroll-behavior: initial !important; }'
  document.head.appendChild(el)
  return () => el.remove()
}

function jumpToTop() {
  const undoScrollHack = disableSmoothScroll()
  window.scroll({top: 0})
  undoScrollHack()
}

I could create a PR for next/link to add a sort of disableSmoothScroll option that uses this hack when scrolling to the top (if the maintainers consider it a good option).

etienne-martin commented 3 years ago

I ended up creating a component that temporarily enables smooth scrolling on the html document during hash transitions:

import React, { FC, useEffect } from "react";
import { useRouter } from "next/router";

export const SmoothScroll: FC = ({ children }) => {
  const router = useRouter();

  useEffect(() => {
    const html = document.documentElement;
    let scrollTimeout: number;

    const debouncedRemoveSmoothScroll = () => {
      clearTimeout(scrollTimeout);
      scrollTimeout = setTimeout(() => {
        html.style.removeProperty("scroll-behavior");
      }, 100);
    };

    const handleHashChangeStart = () => {
      html.style.setProperty("scroll-behavior", "smooth", "important");
      debouncedRemoveSmoothScroll();
    };

    const handleRouteChangeStart = () => {
      html.style.removeProperty("scroll-behavior");
    };

    const handleScroll = () => {
      debouncedRemoveSmoothScroll();
    };

    router.events.on("hashChangeStart", handleHashChangeStart);
    router.events.on("routeChangeStart", handleRouteChangeStart);

    window.addEventListener("scroll", handleScroll, {
      passive: true
    });

    return () => {
      clearTimeout(scrollTimeout);
      router.events.off("hashChangeStart", handleHashChangeStart);
      router.events.off("routeChangeStart", handleRouteChangeStart);
      window.removeEventListener("scroll", handleScroll);
      html.style.removeProperty("scroll-behavior");
    };
  }, []);

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

You wrap your root component with it in _app.tsx and forget about it.

DaniGuardiola commented 3 years ago

@etienne-martin wow, nice! I didn't know those events existed. That's a nice hack. Still wish there was native Next.js support for this.

itaditya commented 3 years ago

Hey @etienne-martin after I read your solution I became curious about why you added scroll listener. I experimented and this code seems to work also

// app.js

function useNormalScrollRoutes() {
  const router = useRouter();

  useEffect(() => {
    router.events.on('routeChangeStart', () => {
      document.documentElement.classList.add('normal-scroll');
    });
    router.events.on('routeChangeComplete', () => {
      document.documentElement.classList.remove('normal-scroll');
    });
  }, []);
}

export default function MyApp({ Component, pageProps }) {
  useNormalScrollRoutes();
  return <Component {...pageProps} />;
}
// global.css

html {
  scroll-behavior: smooth;
}

html.normal-scroll {
  scroll-behavior: auto;
}

Am I missing some edge-case? Btw. thanks for giving us a user-land solution. That smooth scroll was becoming really annoying.

etienne-martin commented 3 years ago

@itaditya my first implementation was similar to yours and worked fine in dev mode but I had issues once my app was compiled statically. I had somewhat of a race condition because routing is way faster with the production build and the routeChangeStart event would fire too late (because it's async).

I'd prefer not to have a scroll listener in there but at least it's passive.

Timer commented 3 years ago

Per the CSSOM spec, JavaScript cannot override this behavior when Next.js calls window.scrollTo as the only valid two options are auto or smooth.

If you implement smooth scrolling in your app, you'll have to handle disabling it yourself as well on route changes using the above snippet(s).

Someone should probably package this up into a hook and share it on npm!

itaditya commented 3 years ago

@etienne-martin I did try it on a preview url once and even now I am not able to reproduce that edge case.

https://deploy-preview-50--devadi.netlify.app/blog/optimize-your-react-app-with-react-memo

rscotten commented 3 years ago

@itaditya has a nice app-wide solution. For those who need a more surgical approach, here's how I solved it:

The CSS scroll-behavior: smooth on html has unwanted side effects. In my case, I'm using the react-scroll-up button component to smooth scroll to the top of page when clicked, and html { scroll-behavior: smooth } disables its scroll function.

I solved this problem by disabling scroll on select NextJS <Link href="..." scroll={false} >...</Link> components and Router.push('...', '...', { scroll: false}) calls. Then I manually call

window.scrollTo({
  top: 0,
  left: 0,
  behavior: 'smooth',
});

on the specific route changes I wanted smooth scrolling on.

lucasrothman commented 2 years ago

I had a similar issue and found that it was because of bootstrap. In _reboot.scss exists:

@media (prefers-reduced-motion: no-preference)
:root {
    scroll-behavior: smooth;
}

To overwrite this I simply added

:root {
  scroll-behavior: auto !important;
}

to my global.css file

balazsorban44 commented 2 years ago

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.