emilkowalski / vaul

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

add preventClose argument to onClose callback #350

Closed cervantes-x closed 1 week ago

cervantes-x commented 4 months ago

This PR enhances the Drawer component by introducing a new feature to the onClose callback. It adds a preventClose argument that, when called, will prevent the drawer from closing and reset its position. This is particularly useful when dealing with forms or interactive content inside the drawer, ensuring that users don't accidentally close it before completing their actions.

Example:

      <Drawer
        onClose={(preventClose) => {
          if (getUnsavedChanges() ) { // custom method to check for unsaved changes
            openUnsavedChangesDialog(); // show a dialog to the user about unsaved changes
            preventClose?.(); // prevent the drawer from closing
          }
        }}
        open={drawerOpen}
        onOpenChange={(open) => {
          setDrawerOpen(open);
        }}
      >
        ...
        <DrawerContent>
          ...
        </DrawerContent>
      </Drawer>

https://github.com/emilkowalski/vaul/assets/117078849/883fd48f-8a1d-4ae8-957c-ee99ccf34a7e

vercel[bot] commented 4 months ago

@cervantes-x is attempting to deploy a commit to the emil Team on Vercel.

A member of the Team first needs to authorize it.

artemis-prime commented 4 months ago

Why not just have onClose return a boolean? Say, if true, then the drawer doesn't close. Why do it through a called function?

rortan134 commented 4 months ago

I believe this is already possible with the dismissible property, just set it to false when the drawer shouldn't close. (Also it looks like it isn't documented but there is an example listed)

cervantes-x commented 4 months ago

Why not just have onClose return a boolean? Say, if true, then the drawer doesn't close. Why do it through a called function?

To be honest, I didn't consider it. Since it made more sense to me to explicitly call a function instead of just returning a boolean in onClose. But I guess that approach would be easier. I will see if I can update it this week.

I believe this is already possible with the dismissible property, just set it to false when the drawer shouldn't close. (Also it looks like it isn't documented but there is an example listed)

This is true, however for my app it was necessary to provide our users with an explanation for why the drawer isn't dismissible. Additionally, I wanted to keep the ability to slide the drawer down. This is also the native behavior in IOS drawers.

joaom00 commented 4 months ago

actually this API already exists through Radix using onInteractOuside={event => event.preventDefault()} but this may not work very well as there are some state synchronization issues when you control the open state of the vaul. This is something I would like to fix soon

image

emilkowalski commented 1 week ago

The sync issues are now gone, let's use the suggestion that @joaom00 provided and not bloat the API. Thanks for this PR tho!