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.

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

Dialog/Drawer issues with controlled state #624

Open
hartwm opened this issue Jan 9, 2024 · 11 comments · May be fixed by #1182
Open

Dialog/Drawer issues with controlled state #624

hartwm opened this issue Jan 9, 2024 · 11 comments · May be fixed by #1182
Labels
type: bug A confirmed report of unexpected behavior in the application

Comments

@hartwm
Copy link

hartwm commented Jan 9, 2024

Describe the bug

If using a store in melt to control open, then clickoutside still works. In Drawer/Dialog it doesn't.
https://shadcdn-drawer.vercel.app/

Reproduction

https://github.com/hartwm/shadcdn-drawer

Logs

No response

System Info

System:
    OS: macOS 13.5.1
    CPU: (10) x64 Apple M1 Pro
    Memory: 29.54 MB / 32.00 GB
    Shell: 5.1.4 - /usr/local/bin/bash
  Binaries:
    Node: 18.16.0 - ~/.nvm/versions/node/v18.16.0/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.4 - /usr/local/bin/npm
    pnpm: 8.12.1 - ~/Library/pnpm/pnpm
  Browsers:
    Brave Browser: 120.1.61.109
    Chrome: 120.0.6099.199
    Safari: 16.6
  npmPackages:
    @sveltejs/kit: ^2.0.0 => 2.1.2 
    bits-ui: ^0.13.6 => 0.13.6 
    svelte: ^4.2.7 => 4.2.8 
    vaul-svelte: ^0.0.6 => 0.0.6

Severity

annoyance

@hartwm hartwm added the type: bug A confirmed report of unexpected behavior in the application label Jan 9, 2024
@huntabyte
Copy link
Owner

I can reproduce the Drawer, but if I apply bind:open={$dialogOpen} to the Dialog then it works. In theory, both should work without bind, so I will investigate this further.

@shimkiv
Copy link
Contributor

shimkiv commented Jan 15, 2024

Similar issue happens when Drawer is invoked from within the Dialog. Then closing the Drawer also closes the background/parent dialog. Including weird case when clicking within the Drawer closes both Drawer and parent Dialog.


Screen.Recording.2024-01-15.at.19.15.08.mov

@pitzzahh
Copy link

There is a similar issue when creating a toast when a Drawer or Dialog is open. Closing/dragging to close the toast also closes the Drawer or Dialog.
If a Drawer has a form in the <Drawer.Content></Drawer.Content>t , it won't drag to close itself, only if you click/interact with the form first.

Dialog.mp4
drawer.mp4

@pitzzahh
Copy link

There is a similar issue when creating a toast when a Drawer or Dialog is open. Closing/dragging to close the toast also closes the Drawer or Dialog. If a Drawer has a form in the <Drawer.Content></Drawer.Content>t , it won't drag to close itself, only if you click/interact with the form first.

Dialog.mp4
drawer.mp4

I got the drawer drag work when first opened by wrapping the form in a div and giving it a height of 1/2

@huntabyte
Copy link
Owner

huntabyte commented Jan 28, 2024

There is a similar issue when creating a toast when a Drawer or Dialog is open. Closing/dragging to close the toast also closes the Drawer or Dialog.

This is because it considers the toast to be an "outside click" event, and closes the drawer on those events. You can use the onOutsideClick prop on the Dialog.Root to ignore certain conditions.

For example, if all of your toasts had a common data- attribute, you could prevent that behavior from occurring by doing something like this:

<script lang="ts">
	// let's say all of our toasts have a `data-toast` attribute
	
	function onOutsideClick(event: PointerEvent) {
		if (event.target && event.target.hasAttribute("data-toast") {
			event.preventDefault()
		}
	};
</script>

<Drawer.Root {onOutsideClick}>
	<!-- ... -->
</Drawer.Root>

By calling preventDefault() on the event, it tells bits-ui to ignore that event altogether and not do what it normally would, which in this situation is closing the drawer.

Will make a note to document this a bit better!

@shimkiv
Copy link
Contributor

shimkiv commented Jan 29, 2024

My case was fixed with:

 bits-ui                            ^0.15.1  →    ^0.16.0
 vaul-svelte                         ^0.1.0  →     ^0.2.1

Thank you.

@shimkiv
Copy link
Contributor

shimkiv commented Feb 5, 2024

My case was fixed with:

 bits-ui                            ^0.15.1  →    ^0.16.0
 vaul-svelte                         ^0.1.0  →     ^0.2.1

Thank you.

Issue is back again with:

bits-ui  ^0.16.0  →  ^0.17.0

:)

@0x2E
Copy link

0x2E commented Feb 28, 2024

Similar issue here. The selection of a Select in Drawer makes the drawer close.

Update: I use this to fix it:

	onOutsideClick={(e) => {
		if (e.target && e.target instanceof HTMLElement && e.target.hasAttribute('data-select-item')) {
			e.preventDefault();
		}
	}}

Reproduction: https://stackblitz.com/edit/github-6lpr3i?file=src%2Froutes%2F%2Bpage.svelte or

<script lang="ts">
  import * as Drawer from "$lib/components/ui/drawer";
  import * as Select from "$lib/components/ui/select";

  let fix = false // true to fix
</script>
 
<Drawer.Root onOutsideClick={(e) => {
    if (!fix) return
    if (e.target && e.target instanceof HTMLElement && e.target.hasAttribute('data-select-item')) {
        e.preventDefault();
    }
  }}>
  <Drawer.Trigger>Open</Drawer.Trigger>
  <Drawer.Content class='h-[500px]'>
    <Select.Root>
    <Select.Trigger class="w-[180px]">
        <Select.Value placeholder="Theme" />
    </Select.Trigger>
    <Select.Content>
        <Select.Item value="light">Light</Select.Item>
        <Select.Item value="dark">Dark</Select.Item>
        <Select.Item value="system">System</Select.Item>
    </Select.Content>
    </Select.Root>
  </Drawer.Content>
</Drawer.Root>

@shimkiv
Copy link
Contributor

shimkiv commented Feb 28, 2024

In my case <Drawer.Root closeOnOutsideClick={false}> does not help. I unfortunately had no time to dig into this issue.

@fromaline
Copy link

Dialog doesn't work in a controlled way at all. When I close it, the open variable is still in a true state. It's a deal-breaker because I need to open dialogs from a different place. Is there a workaround?

@huntabyte huntabyte linked a pull request Sep 28, 2024 that will close this issue
3 tasks
@vcheeze
Copy link

vcheeze commented Oct 10, 2024

There is a similar issue when creating a toast when a Drawer or Dialog is open. Closing/dragging to close the toast also closes the Drawer or Dialog.

This is because it considers the toast to be an "outside click" event, and closes the drawer on those events. You can use the onOutsideClick prop on the Dialog.Root to ignore certain conditions.

For example, if all of your toasts had a common data- attribute, you could prevent that behavior from occurring by doing something like this:

<script lang="ts">
	// let's say all of our toasts have a `data-toast` attribute
	
	function onOutsideClick(event: PointerEvent) {
		if (event.target && event.target.hasAttribute("data-toast") {
			event.preventDefault()
		}
	};
</script>

<Drawer.Root {onOutsideClick}>
	<!-- ... -->
</Drawer.Root>

By calling preventDefault() on the event, it tells bits-ui to ignore that event altogether and not do what it normally would, which in this situation is closing the drawer.

Will make a note to document this a bit better!

@huntabyte how would you suggest handling the case when we have a Drawer which then opens up a Dialog? In this case, I'd want to close them in FIFO order, meaning clicking outside should close the Dialog but not the Drawer, then clicking a second time will close the Drawer.

Currently, if I apply event.preventDefault() on the Dialog overlay, neither the Dialog nor the Drawer will be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants