huntabyte / vaul-svelte

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

(Safari) inconsistent close behavior on escape keydown #32

Open mateuszlewko opened 7 months ago

mateuszlewko commented 7 months ago

Describe the bug

There is no exit animation on the escape button click when we change initial focus. In the attached video, I'm pressing "esc" to close the panel. The closing animation is smooth when I don't click away from the initial input box. However, when I click on something else, e.g. text below, press esc, then there is no closing animation. Tested on macOS Safari.

https://github.com/huntabyte/vaul-svelte/assets/6179597/f9cec26b-81e3-4656-8385-a49f79ddd7d3

Reproduction

  1. Open scrollable drawer: https://www.vaul-svelte.com/examples on desktop.
  2. Click away from the initial input box.
  3. Close with esc.

Logs

No response

System Info

macOS Safari.

Note that it works on Chrome on macOS.

Severity

annoyance

mateuszlewko commented 7 months ago

I also checked that closeOnEscape={false} does not seem to work, nor does closeFocus or openFocus.

I looked into closeFocus and openFocus in bits ui (https://github.com/huntabyte/bits-ui), but they don't seem to be used anywhere. They're defined in a few places, but I don't see any calls to a focus method with those props. Could you point me to where are those used?

Thanks!

huntabyte commented 7 months ago

Oh, one of those lovely "safari-isms". The openFocus / closeFocus props are handled by https://github.com/melt-ui/melt-ui.

The likely culprit of this is the fact that Safari doesn't fully respect programmatic focus traps. I'll look into this. Can you confirm if this behavior also exists for the Dialog in bits-ui? https://bits-ui.com/docs/components/dialog

mateuszlewko commented 7 months ago

The openFocus / closeFocus props are handled by https://github.com/melt-ui/melt-ui.

Oh I see, thanks for the response!

Dialog works perfectly.

huntabyte commented 7 months ago

Okay, that's interesting! Seems like I'm missing something in my dialog-wrapping logic in this project then! I will investigate as soon as I have some time! If you're keen to take a look, that's where I'd start is the closeOnEscape logic, perhaps we need to custom handle it if we aren't already.

huntabyte commented 7 months ago

I just spent some time looking into this, it appears the original suffers from the same issue regarding the scrollable content. It does seem to animate, just very rapidly. I'm unsure what is causing this difference at the moment.

gdude2002 commented 7 months ago

In Chrome, I'm noticing that binding the open property on the Root element works as expected, but the drawer closing/opening doesn't animate correctly there either when the Boolean is toggled by a non-drawer element (or the drawer Close element) - indeed, looking as if the close transition is far quicker than it should be, as you described.

Do you think this is related to the same issue?

Video: https://github.com/huntabyte/vaul-svelte/assets/204153/dbf82d34-e5c1-456a-aca0-a97196410e2a


Example, using shadcn-svelte (but svelte-vaul directly, their drawer isn't flexible enough) - imagine there's a 4rem navbar with a 1px border at the top of the page, haha

<script lang="ts">
    import { Button } from "$lib/components/ui/button";

    import { Menu } from "lucide-svelte";
    import { Drawer } from "vaul-svelte"

    let drawer0pen = false;
</script>

<Drawer.Root direction="left" bind:open={drawer0pen}>
    <Drawer.Portal class="fixed left-0 z-20" style="top: calc(4rem + 1px)">
        <Drawer.Content>
            <div role="presentation"
                 class="bg-background"
                 style="width: 75vw; height: calc(100vh - (4rem + 1px));"
                 on:click|stopPropagation
            >
                <!-- Some content -->
            </div>
        </Drawer.Content>
    </Drawer.Portal>
</Drawer.Root>

<div role="presentation"
     class="fixed left-0 bg-black/40 dark:bg-white/20 z-10 transition-opacity {drawer0pen ? 'opacity-100' : 'opacity-0'}"
     style="width: 100vw; height: calc(100vh - (4rem + 1px)) top: calc(4rem + 1px)"
     on:click={() => (drawer0pen = false)}
></div>

<Button variant="ghost" class="md:hidden" on:click={() => (drawer0pen = !drawer0pen)}>
    <Menu />
</Button>
huntabyte commented 7 months ago

@gdude2002 likely more closely related to an issue https://github.com/huntabyte/vaul-svelte/pull/33 will resolve.

gdude2002 commented 7 months ago

Ah, awesome, thanks for linking that!

mateuszlewko commented 7 months ago

It seems to work well now on the latest release, thanks!