Fix scrollbar-induced layout shift #11363
Conversation
|
@versile2 Do you think this will break your recent changes? |
|
if it had the latest changes no. |
|
Your branch is not up to date. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #11363 +/- ##
=======================================
Coverage 91.12% 91.12%
=======================================
Files 465 465
Lines 14409 14409
Branches 2788 2788
=======================================
Hits 13130 13130
Misses 641 641
Partials 638 638 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR removes manual scrollbar padding logic in JS/SCSS and leverages the native scrollbar-gutter CSS property to fix page “dancing” when scrollbars appear.
- JS: Always apply the lock class, removing custom padding/no-padding logic
- SCSS: Simplify
.scroll-lockedto only hide overflow - ThemeProvider: Inject
html { scrollbar-gutter: stable; }into generated styles
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/MudBlazor/TScripts/mudScrollManager.js | Remove JS-based scrollbar padding logic; always add/remove lock class |
| src/MudBlazor/Styles/layout/_scroll.scss | Simplify .scroll-locked to only overflow: hidden; |
| src/MudBlazor/Components/ThemeProvider/MudThemeProvider.razor.cs | Add native scrollbar-gutter: stable; CSS rule |
Comments suppressed due to low confidence (2)
src/MudBlazor/TScripts/mudScrollManager.js:60
- The updated locking logic isn’t covered by existing tests; consider adding unit tests to verify behavior when CSS 'scrollbar-gutter' is unsupported and ensure the lock count logic works as expected.
lockScroll(selector, lockclass) {
src/MudBlazor/TScripts/mudScrollManager.js:63
- Consider retaining the fallback logic or adding a feature-detection check for CSS 'scrollbar-gutter' support to prevent layout shifts in browsers that don't support the property.
element.classList.add(lockclass);
src/MudBlazor/Components/ThemeProvider/MudThemeProvider.razor.cs
Outdated
Show resolved
Hide resolved
|
It has no visible effect on Firefox, and Firefox doesn't dance, but fixes both Chrome and Edge from dancing. It does have a display on Dialogs and does not interfere with my PR just aims to ensure the class isn't added/removed unnecessarily. Old way removes the scrollbar With Stable Gutters, keeps but unable to use the scrollbar |
|
Description
Resolves #9056 and fixes the page "dancing" by using native CSS properties to handle the scrollbar gutters which are now widely supported.
How Has This Been Tested?
visually
Tested on Edge and FireFox.
Type of Changes
Video3.mp4
Checklist
dev).