search

emilkowalski / vaul

An unstyled drawer component for React.
https://vaul.emilkowal.ski
MIT License
5.29k stars 170 forks source link

Drawer close on drag when direction is set to right and scrollable content #243

Closed imopbuilder closed 5 months ago

imopbuilder commented 5 months ago

When the Drawer direction is set to right and having a scrollable content, the drawer is only closing when the scrollbar is on top but not closing on drag when there is a scroll

Sandbox: https://codesandbox.io/p/devbox/drawer-direction-right-forked-qvsts5?workspaceId=021ca430-6b5a-4f63-b53b-b37cac3c53d8

https://github.com/emilkowalski/vaul/assets/150527559/9bf2595c-8e61-4a50-b0d8-1ed51ced632a

MilanObrenovic commented 5 months ago

I found this bug, thanks @imopbuilder for reporting it

Additional notes to leave to maintainer to get better ideas for solution:

More than likely the scroll area overrides the swipe functionality, so instead of being able to swipe, you can only scroll. this problem has been solved by MUI library, more specifically this https://mui.com/material-ui/react-drawer/#swipeable component, the SwipeableDrawer. Try it on mobile and see how flawlessly you can scroll AND swipe to close the sidebar

MUI has proven it's possible to do it, so it's up to vaul to fix this issue as well

Edit: @imopbuilder mentioned that this bug appears when direction is set to right, this is false. Direction doesn't matter. if the content is scrollable, it doesn't work in any direction

emilkowalski commented 5 months ago

@MilanObrenovic This is unrelated.

The issue here is that initially, when the only position was bottom, you were only able to drag once you scrolled all the way to the top, like an iOS Sheet. That being said now that we can use it on the sides I'll disable that functionality for right and left positioned drawers.

MilanObrenovic commented 5 months ago

@MilanObrenovic This is unrelated.

The issue here is that initially, when the only position was bottom, you were only able to drag once you scrolled all the way to the top, like an iOS Sheet. That being said now that we can use it on the sides I'll disable that functionality for right and left positioned drawers.

what do you mean with disable? is this gonna be fixed or not? I just want to use the drawer as a side nav on mobile and swipe to close it even after scrolling. Thats it

emilkowalski commented 5 months ago

This has been fixed 👍🏻

MilanObrenovic commented 5 months ago

This has been fixed 👍🏻

Perfect. I just tested and can confirm that it works. Great job, thank you

MilanObrenovic commented 5 months ago

This has been fixed 👍🏻

While further testing, i think this created a side effect. Once the drawer from left is open, if you onClickDown on the background then the sidebar closes immediately, not allowing me to click and drag my mouse. The sidebar should close when you onClickDown and then onClickUp (let go of the left mouse click). Can you test and confirm this?

emilkowalski commented 5 months ago

This could have not been caused by that PR, as this doesn’t touch the code related to that logic. Can you provide a codesandbox with reproduction?

MilanObrenovic commented 5 months ago

This could have not been caused by that PR, as this doesn’t touch the code related to that logic. Can you provide a codesandbox with reproduction?

nevermind, this has existed before. i just tested on https://ui.shadcn.com/docs/components/drawer official docs. you can try it yourself there. is this behavior intentional to close the drawer onClickDown, instead of onClickUp?

emilkowalski commented 5 months ago

Yes, Radix does that as well.