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

[docs] Fix scroll shift #24418

Merged

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Jan 14, 2021

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work docs Improvements or additions to the documentation labels Jan 14, 2021
@oliviertassinari oliviertassinari changed the title [docs] Fix content-layout-shift [docs] Fix scroll shift Jan 14, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Jan 14, 2021

No bundle size changes

Generated by 🚫 dangerJS against 5a67098

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That scroll flipping is so annoying.

Though it seems like there is still some unstable scrolling:

  1. Visit page
  2. Scroll a bit
  3. reload
    scroll-unstable

Profiler filmstrip:

scroll-unstable-filmstrip

Profile: https://drive.google.com/file/d/1AMxC9PBf0may4TtrRHkU_uBSnA_ru8og/view?usp=sharing

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jan 14, 2021

Though it seems like there is still some unstable scrolling:

@eps1lon This is a different issue and only reproducible when there is a hash in the URL. I can observe the following: Next.js's reset the scroll with the scrollToHash() method, then, on the onload event, the browser restore the previous scroll position => double jump.

I have reported the bug in vercel/next.js#13653 (comment).

Thanks @dmtrKovalenko for reporting it in https://twitter.com/dmtrKovalenko/status/1348936828271910914. if we destructure your video there are 4 different issues bundled at once:

  1. dark/light mode switch: Is using with Material UI or similar possible? pacocoursey/next-themes#2
  2. the scrollIntoView issue of this PR that resets the scroll to the top position. Fixed ✅
  3. The Next.js hash restore issue Scrolling happens when user returns to a page with hash using browser back button vercel/next.js#13653 (comment)
  4. The date picker demo unstable rendering (the demo server-side is smaller than the demo client-side) https://next.material-ui.com/components/date-picker/#responsiveness

server

client

@eps1lon
Copy link
Member

eps1lon commented Jan 15, 2021

This is a different issue and only reproducible when there is a hash in the URL.

It's this exact issue that is still reproducible after scroll. So we did improve it, but not fully fix it.

@oliviertassinari
Copy link
Member Author

I had a look at why the scroll into view was added. It's explained in #15409 (comment). I can't reproduce anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants