Skip to content

Commit 612e027

Browse files
authored
[ScrollLock] Add check for document body scrolls before applying scroll-lock (#7492)
### WHY are these changes introduced? Fixes Shopify/core-workflows/issues/529 When `ScrollLock` is applied it will always add a scroll bar regardless if the page is scrollable. In some cases this can have a jarring effect where the content gets shifted. ### WHAT is this pull request doing? In this fix we check if `body` is scrollable and keep the scroll bar hidden if it does not. We still want to apply `overflow: hidden` in case the page becomes scrollable after the lock is enabled. In this case the scrollbar stays hidden but reappears when ScrollLock is removed. <details> <summary>Before - ScrollLock on page without scrollbar</summary> <img src="https://user-images.githubusercontent.com/1307190/197619618-7466cd39-6837-4a0d-ab43-f0b405be5e09.gif" alt="Before - ScrollLock on page without scrollbar"> </details> <details> <summary>After - ScrollLock on page without scrollbar</summary> <img src="https://user-images.githubusercontent.com/1307190/197619622-75a99bf8-072f-46c2-87af-453779d96c87.gif" alt="After - ScrollLock on page without scrollbar"> </details> <details> <summary>After - ScrollLock on page with scrollbar</summary> <img src="https://user-images.githubusercontent.com/1307190/197619624-06a40265-a663-42c9-ab8b-3449db44baec.gif" alt="Before - ScrollLock on page without scrollbar"> </details> <!-- Summary of the changes committed. Before / after screenshots are appreciated for UI changes. Make sure to include alt text that describes the screenshot. If you include an animated gif showing your change, wrapping it in a details tag is recommended. Gifs usually autoplay, which can cause accessibility issues for people reviewing your PR: <details> <summary>Summary of your gif(s)</summary> <img src="..." alt="Description of what the gif shows"> </details> --> <!-- ℹ️ Delete the following for small / trivial changes --> ### How to 🎩 🖥 [Local development instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development) 🗒 [General tophatting guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md) 📄 [Changelog guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog) [Spin instance](https://admin.web.at.jita-yi.us.spin.dev/store/shop1/) 1. Ensure "scrollbars" are always visible on the OS <img width="433" alt="alwaysscroll" src="https://user-images.githubusercontent.com/1307190/197621019-d4d15101-bb36-4c9c-a920-db057900c549.png"> 2. Open spin instance 3. Open a page with scrollbars (Home) 4. Open the search box from the top bar 5. Ensure page is not scrollable while search box is opened and scrollbar "gutter" remains visible 6. Open a age that does not scroll (Orders) 7. Open the search box from the top bar 9. Ensure page is not scrollable while search box is opened and scrollbar "gutter" is not visible ### 🎩 checklist - [x] Tested on [mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing) - [x] Tested on [multiple browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers) - [ ] Tested for [accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md) - [ ] Updated the component's `README.md` with documentation changes - [ ] [Tophatted documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md) changes in the style guide
1 parent a8d80cf commit 612e027

File tree

3 files changed

+21
-0
lines changed

3 files changed

+21
-0
lines changed

.changeset/quiet-coats-dance.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/polaris': patch
3+
---
4+
5+
Only apply scroll-lock with scrollbar when body is scrollable

polaris-react/src/components/ScrollLock/ScrollLock.scss

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
margin: 0;
44

55
// stylelint-disable selector-max-attribute
6+
&[data-lock-scrolling-hidden] {
7+
overflow-y: hidden;
8+
}
9+
610
[data-lock-scrolling-wrapper] {
711
overflow: hidden;
812
height: 100%;

polaris-react/src/utilities/scroll-lock-manager/scroll-lock-manager.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,17 @@ import {isServer} from '../target';
22

33
export const SCROLL_LOCKING_ATTRIBUTE = 'data-lock-scrolling';
44

5+
const SCROLL_LOCKING_HIDDEN_ATTRIBUTE = 'data-lock-scrolling-hidden';
6+
57
const SCROLL_LOCKING_WRAPPER_ATTRIBUTE = 'data-lock-scrolling-wrapper';
68

79
let scrollPosition = 0;
810

11+
function isScrollBarVisible() {
12+
const {body} = document;
13+
return body.scrollHeight > body.clientHeight;
14+
}
15+
916
export class ScrollLockManager {
1017
private scrollLocks = 0;
1118
private locked = false;
@@ -29,6 +36,7 @@ export class ScrollLockManager {
2936

3037
if (scrollLocks === 0) {
3138
body.removeAttribute(SCROLL_LOCKING_ATTRIBUTE);
39+
body.removeAttribute(SCROLL_LOCKING_HIDDEN_ATTRIBUTE);
3240
if (wrapper) {
3341
wrapper.removeAttribute(SCROLL_LOCKING_WRAPPER_ATTRIBUTE);
3442
}
@@ -38,6 +46,10 @@ export class ScrollLockManager {
3846
scrollPosition = window.pageYOffset;
3947
body.setAttribute(SCROLL_LOCKING_ATTRIBUTE, '');
4048

49+
if (!isScrollBarVisible()) {
50+
body.setAttribute(SCROLL_LOCKING_HIDDEN_ATTRIBUTE, '');
51+
}
52+
4153
if (wrapper) {
4254
wrapper.setAttribute(SCROLL_LOCKING_WRAPPER_ATTRIBUTE, '');
4355
wrapper.scrollTop = scrollPosition;

0 commit comments

Comments
 (0)