huntabyte / vaul-svelte

An unstyled drawer component for Svelte.
https://vaul-svelte.com
MIT License
467 stars 19 forks source link

next: allow users to override scroll prevention #96

Closed bugproof closed 1 week ago

bugproof commented 1 month ago

Describe the bug

those styles get added to body after closing the drawer which makes it impossible to scroll the page:

right: unset; pointer-events: auto; overflow: hidden;

so when drawer opens overflow:hidden is probably added to body to prevent scrolling the background but it is never cleared when closing the drawer... - correct me if my diagnosis is wrong. This works in svelte 4 version. Vaul pre-release version is completely unusable as a result.

Reproduction

will provide later

Logs

No response

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (32) x64 AMD Ryzen 9 7950X3D 16-Core Processor
    Memory: 18.45 GB / 63.09 GB
  Binaries:
    Node: 20.11.1 - C:\Program Files\nodejs\node.EXE
    npm: 10.4.0 - C:\Program Files\nodejs\npm.CMD
    pnpm: 8.15.4 - ~\AppData\Local\pnpm\pnpm.EXE
    bun: 1.1.31 - ~\.bun\bin\bun.EXE
  Browsers:
    Edge: Chromium (127.0.2651.74)
    Internet Explorer: 11.0.22621.3527
  npmPackages:
    @sveltejs/kit: ^2.7.2 => 2.7.2
    bits-ui: ^1.0.0-next.22 => 1.0.0-next.22
    svelte: ^5.0.5 => 5.0.5
    typescript: ^5.6.3 => 5.6.3
    vaul-svelte: ^1.0.0-next.0 => 1.0.0-next.0

Severity

blocking an upgrade

bugproof commented 1 month ago

This might not be an issue with vaul itself but dialog in bits-ui next... https://github.com/huntabyte/bits-ui/commit/07bfcd44722d4b0dbcf1fa25d5932765ed993cd2

this is most likely bugged file : https://github.com/huntabyte/bits-ui/blob/next/packages/bits-ui/src/lib/internal/use-body-scroll-lock.svelte.ts

Interestingly this only happens in drawer. Dialog correctly resets to overflow: hidden auto but vaul stays at overflow:hidden...

bugproof commented 1 month ago

I fixed this in code by doing:

<Drawer.Root bind:open={isOpen} onClose={() => document.body.style.overflow = "hidden auto"} preventScrollRestoration>

There is also some bug with scroll jumping back to the top of the page sometimes when closing the drawer...

huntabyte commented 1 month ago

Would you mind providing a reproduction of this? I'm not able to reproduce it with the docs site (see video below)

https://github.com/user-attachments/assets/0f0467c4-7b37-451b-aa45-2abb5c1c1bd5

bugproof commented 1 month ago

just ran the docs site locally like you and I have this issue are you sure you've updated all packages? (bits-ui specifically) the docs site reproduces this.

https://github.com/user-attachments/assets/c4d48e0c-d594-42b8-9538-af0788ca3f4f

bugproof commented 1 month ago

EDIT: after adding:

"pnpm": {
        "overrides": {
            "svelte": "5.1.0",
            "bits-ui": "1.0.0-next.26"
        }
    }

It started working!! Couple of hours and it turned out to be a dependency issue.

vault-svelte dependencies need to be updated.

For some reason vaul-svelte was still using "bits-ui": "1.0.0-next.15", even if I had "bits-ui": "1.0.0-next.26" installed in my project.

I added this:

"pnpm": {
        "overrides": {
            "svelte": "5.1.0",
            "bits-ui": "1.0.0-next.26"
        }
    }

and everything is fine now (I hope)... so vaul-svelte package needs to update it's dependencies for now above fix works for this. It was driving me nuts.

@huntabyte it's just a matter of updating packages and pushing new version of vaul-svelte... you can close this when it's done.

drloloto commented 3 weeks ago

For me the bug somehow persists even with manual patching on the lastest 34. It only happens in nested components with overlays. It also only happens if I click the backdrop, if I use the close Button of the Drawer everything works fine. https://github.com/drloloto/shadtest.git

chanmathew commented 1 week ago

I'm also getting this issue actually on bits-ui@next57. The issue now is when you first load the page and the drawer is open, the entire body is disabled and no pointer events are registering.

I think the issue is that there's almost like the overlay blocking behavior, even if you don't have the overlay rendered. If you tap outside the drawer once, then it seems to "dismiss" the overlay and unlock the pointer events on the body.

huntabyte commented 1 week ago

Mind providing a repro @chanmathew ?

huntabyte commented 1 week ago

For me the bug somehow persists even with manual patching on the lastest 34. It only happens in nested components with overlays. It also only happens if I click the backdrop, if I use the close Button of the Drawer everything works fine.

I'm not able to reproduce this. Believe it should have been fixed in a recent version of bits.

chanmathew commented 1 week ago

@huntabyte here you go: https://stackblitz.com/edit/github-aibhpm-9bybdm?file=src%2Froutes%2F%2Bpage.svelte

Side note: I've found this is also the case for the shadcn-svelte Dialog overlay, where the body has pointer-events:none applied when the dialog is opened, and so you can't select items from an autocomplete dropdown for example. As a workaround I added a [&~*]:pointer-events-auto to the dialog-overlay class, but that doesn't address this issue with the drawer.

huntabyte commented 1 week ago

I'm not able to reproduce this behavior with your Stackblitz @chanmathew:

https://github.com/user-attachments/assets/145cd681-a659-4450-8026-5e9ef525a420

huntabyte commented 1 week ago

FYSA for anyone else curious. When you use preventScroll={true} (the default on dialogs/certain floating *.Content components), you are operating in a "modal" mode where if the body cannot be scrolled, the body should not be able to be interacted with, which is why pointer events are disabled on the body when scroll is disabled.

huntabyte commented 1 week ago

If you're still running into issues with this, clear your node modules, vite cache, etc. and ensure you're running the latest version of bits-ui@next and vaul-svelte@next.

chanmathew commented 1 week ago

@huntabyte Maybe this is more clear. When the page first loads, and the drawer is opened, even though there is no overlay present, there is a pointer-events: none on the body, and it takes me 2 clicks to focus the text area that is on the page itself behind the drawer. This is problematic for usages where if you have let's say a map or some interactive element on the page, and you have the drawer half snap, you can't interact with the map even though there is no overlay present, until you initiate that first tap to remove the lock.

https://share.cleanshot.com/D736xlBB

huntabyte commented 1 week ago

Got it! I see where the issue is now, for some reason I'm not allowing the preventScroll prop to be overwritten (which is what is responsible for adding pointer-events: none to the body. Will work on a vaul-svelte patch for this soon.

huntabyte commented 1 week ago

@chanmathew I have a pk-pr-new preview package deployed right now and added to this stackblitz. Let me know if there is anything else not working around this idea (besides the other issue with the snap points combobox input) https://stackblitz.com/edit/github-aibhpm-tufhzj?file=src%2Froutes%2F%2Bpage.svelte

Once I get your confirmation I'll merge/release it to vaul-svelte@next

chanmathew commented 1 week ago

Thanks for the quick fix @huntabyte! 🫡 Just tested it and looks good to me, lock is gone from the body and I can interact with elements behind the drawer immediately.