From 3bc8495874c2f48d8c348fcab0b859a2dc44f995 Mon Sep 17 00:00:00 2001 From: Konnor Rogers Date: Wed, 6 Mar 2024 09:07:14 -0500 Subject: [PATCH] Fixes scroll lock layout shift (#1895) * fix scroll lock layout shift * changelog entry * changelog entry * prettier * add notes about browser support --- docs/pages/resources/changelog.md | 1 + src/internal/scroll.ts | 26 ++++++++++++++++++++------ src/themes/_utility.css | 3 ++- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index 8ece4d0e..902e9a35 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -16,6 +16,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti - Added the Slovenian translation [#1893] - Added support for `contextElement` to `VirtualElements` in `` [#1874] +- Fixed a bug in `.sl-scroll-lock` causing layout shifts. [#1895] - Fixed a bug in `` that caused the rating to not reset in some circumstances [#1877] - Fixed a bug in `` that caused the menu to not close when rendered in a shadow root [#1878] - Fixed a bug in `` that caused a new stacking context resulting in tooltips being clipped [#1709] diff --git a/src/internal/scroll.ts b/src/internal/scroll.ts index 81460e2e..85939ed5 100644 --- a/src/internal/scroll.ts +++ b/src/internal/scroll.ts @@ -8,6 +8,19 @@ function getScrollbarWidth() { return Math.abs(window.innerWidth - documentWidth); } +/** + * Used in conjunction with `scrollbarWidth` to set proper body padding in case the user has padding already on the `` element. + */ +function getExistingBodyPadding() { + const padding = Number(getComputedStyle(document.body).paddingRight.replace(/px/, '')); + + if (isNaN(padding) || !padding) { + return 0; + } + + return padding; +} + /** * Prevents body scrolling. Keeps track of which elements requested a lock so multiple levels of locking are possible * without premature unlocking. @@ -17,10 +30,11 @@ export function lockBodyScrolling(lockingEl: HTMLElement) { // When the first lock is created, set the scroll lock size to match the scrollbar's width to prevent content from // shifting. We only do this on the first lock because the scrollbar width will measure zero after overflow is hidden. - if (!document.body.classList.contains('sl-scroll-lock')) { - const scrollbarWidth = getScrollbarWidth(); // must be measured before the `sl-scroll-lock` class is applied - document.body.classList.add('sl-scroll-lock'); - document.body.style.setProperty('--sl-scroll-lock-size', `${scrollbarWidth}px`); + if (!document.documentElement.classList.contains('sl-scroll-lock')) { + /** Scrollbar width + body padding calculation can go away once Safari has scrollbar-gutter support. */ + const scrollbarWidth = getScrollbarWidth() + getExistingBodyPadding(); // must be measured before the `sl-scroll-lock` class is applied + document.documentElement.classList.add('sl-scroll-lock'); + document.documentElement.style.setProperty('--sl-scroll-lock-size', `${scrollbarWidth}px`); } } @@ -31,8 +45,8 @@ export function unlockBodyScrolling(lockingEl: HTMLElement) { locks.delete(lockingEl); if (locks.size === 0) { - document.body.classList.remove('sl-scroll-lock'); - document.body.style.removeProperty('--sl-scroll-lock-size'); + document.documentElement.classList.remove('sl-scroll-lock'); + document.documentElement.style.removeProperty('--sl-scroll-lock-size'); } } diff --git a/src/themes/_utility.css b/src/themes/_utility.css index 4d383d3d..f0e2eefc 100644 --- a/src/themes/_utility.css +++ b/src/themes/_utility.css @@ -11,8 +11,9 @@ } } +/** This can go away once Safari has scrollbar-gutter support. */ @supports not (scrollbar-gutter: stable) { - .sl-scroll-lock { + .sl-scroll-lock body { padding-right: var(--sl-scroll-lock-size) !important; overflow: hidden !important; }