Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Dialog scroll lock pushes body content when opened #1132

Closed
p-hennessy opened this issue Jan 15, 2023 · 1 comment
Closed

Dialog scroll lock pushes body content when opened #1132

p-hennessy opened this issue Jan 15, 2023 · 1 comment
Assignees
@claviska
Labels
bug Things that aren't working right in the library.

Comments

@p-hennessy
Copy link

p-hennessy commented Jan 15, 2023 •
edited
Loading

Describe the bug

When a Dialog is opened, it applies a .sl-scroll-lock class to the body element which sets overflow: hidden.
This causes the scrollbar to disappear which causes the body content to slide to the right by 16px. Users have told me this a bit jarring.

Browser / OS

  • OS: Windows 10
  • Browser: Chrome
  • Browser version: Version 108.0.5359.125 (Official Build) (64-bit)

(I've also tested this on Firefox and Edge, and the same issue appears)

Additional information

I've found a possible workaround but this has a side effect of snapping the viewport back to the top of the body content which is less than ideal.

.sl-scroll-lock {
    position: fixed;
    inline-size: 100%;
    overflow-y: scroll !important; /* Overrides the sl stylesheet */
}

This is another workaround that sort of works but would not work depending on scrollbar width between browsers / devices:

.sl-scroll-lock {
    padding-right:16px;
}

I'm wondering if there's a better solution for locking the scrolling that doesn't have this side effect, maybe through JavaScript blocking scrolling events? I don't do much front-end stuff so forgive me if that is a dumb question haha.

Screenshots

Before:
Screenshot_192

After:
Screenshot_193
@p-hennessy p-hennessy added the bug Things that aren't working right in the library. label Jan 15, 2023
@claviska
Copy link
Member

After doing some research, I decided to use the same approach as Bootstrap which is to measure the scrollbar width and apply that value padding on the body. However, I'm using a CSS custom property called --sl-scroll-lock-size and the sl-scroll-lock class to avoid collisions and prevent breaking style properties that might change while scrolling is locked.

Since Bootstrap is more of a framework, they have two helpers for fixed/sticky elements. Shoelace doesn't have those opinions [yet], but you can adjust for fixed/sticky elements yourself by tapping into the --sl-scroll-lock-size custom property to apply a negative margin. (That custom property will only be set on the <body> when scrolling is locked.)

Fixed in 511182b. This will be available in the next release.

@mparpaillon mparpaillon mentioned this issue Feb 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@claviska claviska

Labels
bug Things that aren't working right in the library.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@claviska @p-hennessy