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

(Safari) inconsistent close behavior on escape keydown #32

Open
mateuszlewko opened this issue Jan 20, 2024 · 9 comments
Open

(Safari) inconsistent close behavior on escape keydown #32

mateuszlewko opened this issue Jan 20, 2024 · 9 comments

Comments

@mateuszlewko
Copy link

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.

Screen.Recording.2024-01-20.at.15.32.31.mov

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

mateuszlewko commented Jan 21, 2024

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
Copy link
Owner

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.

@mateuszlewko mateuszlewko changed the title Changing focus in scrollable drawer cases no exit animation on escape click Changing focus in scrollable drawer causes no exit animation on escape click Jan 26, 2024
@huntabyte
Copy link
Owner

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.

@huntabyte huntabyte changed the title Changing focus in scrollable drawer causes no exit animation on escape click (Safari) inconsistent close behavior on escape keydown Jan 28, 2024
@gdude2002
Copy link

gdude2002 commented Jan 30, 2024

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
Copy link
Owner

@gdude2002 likely more closely related to an issue #33 will resolve.

@gdude2002
Copy link

Ah, awesome, thanks for linking that!

@mateuszlewko
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants