[docs] Fix navigation layout shift #35679
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The scroll position moves by 6 pixels each time you change page:
Screen.Recording.2022-12-31.at.12.52.39.mov
This regression was introduced between v4.12.4 and v5.0.0, e.g. https://v5-0-6.mui.com/components/typography/ KO, v4.12.4: OK https://v4.mui.com/components/radio-buttons/. It has been driving me crazy for over a year, it won't make it to 2023 (end of 2021: #29994 (comment)).
Why does this happen? Because Next.js REMOUNTs each page. This means that the CssBaseline that is inside the page gets unmounted, removes the
boxSizing: 'border-box',style, which then means that the 3 diamond sponsors' 1px border is no longer taken into account in the height set, which means +6 pixels when this logic runs:material-ui/docs/src/modules/components/AppNavDrawer.js
Line 145 in 6205492
On a related note, in my video, it's fireworks when changing pages 🎆, the search bar blinks, the notification badge blinks, the sponsors blink. It also feels laggy. We could solve this by moving the main layout to _app.js so it rerenders, not remounts. However, this might lead to bundle size bloat to be able to support many different layouts (marketing, docs, template, blog, etc.). Maybe it's better to wait for: https://beta.nextjs.org/docs/routing/pages-and-layouts#layouts "A layout is UI that is shared between multiple pages".