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

Popover inside element with tabIndex="0" cannot be closed by pressing its trigger on safari #2105

Closed
SomeHats opened this issue Apr 24, 2023 · 3 comments · Fixed by #2110
Closed

Comments

@SomeHats
Copy link

SomeHats commented Apr 24, 2023

Bug report

Current Behavior

Kapture 2023-04-24 at 16 25 32

In most browsers, if you create a popover inside an element with tabindex="0", pressing the trigger opens the popover & pressing the trigger again closes it.

On safari (mac & ios), pressing the trigger opens the popover as expected. When pressing the trigger for a second time, the popover closes on pointer down, but re-appears again on pointer up, making it impossible to completely close a popover by pressing its trigger on safari.

Expected behavior

Popovers in safari should close when the trigger is pressed again.

Reproducible example

https://stackblitz.com/edit/react-ts-zhrg64?file=App.tsx

Additional context

I spent a little while investigating this to see if I could fix it in user-land but couldn't figure something out. The issue here is that safari focuses the container with tabindex="0" when the button is pressed down, so radix-ui treats that as a focus-outside. Then, when the click is released, radix sees that as the end of a press, but we've already closed the popover, so it gets opened again.

Your environment

Software Name(s) Version
Radix Package(s) @radix-ui/react-popover 1.0.5
React n/a 18.2.0
Browser Safari 16.2
Assistive tech n/a n/a
Node n/a n/a
npm/yarn n/a n/a
Operating System macOS, iPadOS, iOS macOS 13, iOS + iPadOS 16
@benoitgrelard
Copy link
Collaborator

Hey Alex, nice seeing you here! Hope you're well!

Thanks for the investigation too, this made me realise this is the more general issue that was already expressed here: #1458.
I will keep yours open, close the other one as duplicate and look for a fix.

@benoitgrelard
Copy link
Collaborator

That should be fixed for you now and available in the latest rc: https://www.npmjs.com/package/@radix-ui/react-popover/v/1.0.6-rc.5

@SomeHats
Copy link
Author

oh hey!! fancy seeing you here 😊 thank you so much for the fix - it works perfectly 🤩

steveruizok pushed a commit to tldraw/tldraw that referenced this issue May 3, 2023
Temporarily moving us to a prerelease of this dep. This resolves an
issue where the mobile style menu popover couldn't be closed by tapping
the style button again. See
radix-ui/primitives#2105 for details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants