-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
Fix layout shift caused by redundant styles #6865
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I think its a great suggestion, |
Im pretty sure we added scrollbar-gutter and then removed for some reasons... @canerakdas any insights here? 🤔 |
We removed it in this PR because it looks better and did not create a layout shift What I don't understand is that I couldn't find this fix in the Radix primitives changelog 🤔 |
Not sure I got your comment, are you saying a recent change on the Primitives created such layout shift? |
I mean I think the recent changes fixed the layout shift. Even though I reverted all changes, I couldn't reproduce the layout shift issue 🤷♂️ |
Most likely, Radix Dropdown has a solution to eliminate layout shift from the very beginning. But the styles |
This PR will need to be rebased or recreated now that #6850 merged. |
a118538
to
8c4c891
Compare
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Unit Test Coverage ReportUnit Test Report
|
Description
Initially, @shoaibkh4n noticed that when opening and closing the drop-down list on the page https://nodejs.org/en/download layout-shift occurs and suggested a solution – add
scrollbar-gutter: stable;
.This really helped, but it led to an empty space on other pages equal to the size of the scrollbar (by the way, this happens only in SPA mode). @daklay noticed this and suggested setting
scrollbar-gutter: auto
, but this is unnecessary, since it is the default value.In fact, Radix UI Dropdown Menu is already solving this problem, but it was prevented by the thoughtless assignment of such styles:
There is a difference between
width: 100%
andwidth: auto
. Therefore, you just need to delete these styles.Validation
After this changes no layout shift and empty spaces on other pages:
screen.mp4
Related Issues
Check List
npm run format
to ensure the code follows the style guide.npm run test
to check if all tests are passing.npx turbo build
to check if the website builds without errors.