search

radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
15.6k stars 800 forks source link

[DropdownMenu] Unexpected blur behavior when clicking the menu trigger #1903

Open filiptammergard opened 1 year ago

filiptammergard commented 1 year ago

Bug report

Current Behavior

When using Mantine's DatePicker and Radix UI's DropdownMenu in the same page, the date picker doesn't close when clicking the menu trigger, but the menu is still opening. However, when the menu trigger is clicked again to close the menu, the date picker is also closed.

https://user-images.githubusercontent.com/44197016/214293988-a0693a31-a9b0-4d60-a9f3-a9ae84cc8b28.mov

Expected behavior

I'd expect the date picker to close when clicking on the menu trigger. The menu should probably not open on menu trigger click, but rather only close the date picker. I.e. it should work as when I click a regular button (see video), or when I click anywhere else to blue the date picker.

Reproducible example

CodeSandbox Template

Suggested solution

I think there's something related to preventing defaults in the DropdownMenu.Trigger component that causes this, since it doesn't happen with any other component I have tried. When clicking any other interactive or non-interactive component (or anywhere else), the date picker closes.

Additional context

Since the DatePicker works as expected with all other components I have tried, I'm pretty confident the problem comes from @radix-ui/react-dropdown-menu and not from Mantine.

Your environment

Software Name(s) Version
Radix Package(s) react-dropdown-menu 2.0.2
React n/a 18.2.0
Browser Chrome 109.0.5414.87 (Official Build) (x86_64)
Operating System MacOS Ventura 13.1
joaom00 commented 1 year ago

Hey @filiptammergard, it seems that the link of the codesandbox isn't right

filiptammergard commented 1 year ago

Hey @filiptammergard, it seems that the link of the codesandbox isn't right

Better now?

joaom00 commented 1 year ago

The DropdownMenu.Content is opened when onPointerDown on DropdownMenu.Trigger and Mantine listen for mousedown and touchstart events to close the datepicker when clicked outside. If you override the clickOutsideEvents prop in DatePicker to listen to pointerdown event, then it will close when you click on DropdownMenu.Trigger.

CodeSandbox

filiptammergard commented 1 year ago

Nice find @joaom00! But clicking with a mouse is both a mouse event and a pointer event. How come the date picker doesn't close when I click the dropdown menu trigger, while it does click when I click anywhere else (including other interactive elements)?

joaom00 commented 1 year ago

it's because of that https://github.com/radix-ui/primitives/blob/21a7c97dc8efa79fecca36428eec49f187294085/packages/react/dropdown-menu/src/DropdownMenu.tsx#L124

when event.preventDefault in onPointerDown the mousedown doesn't catch it, but pointerdown does.

You can see here

filiptammergard commented 1 year ago

Nice! Took some research to understand the why behind this, so I wrote a blog post on it for future lurkers: https://tammergard.se/blog/mouse-touch-and-pointer-events/

Thanks for helping out @joaom00!

If there some other way to avoid focus competition? Since calling preventDefault on pointerdown has the side-effect of not firing compatibility mouse events, and therefore might cause weird bugs when listening to mousedown, I think it would be preferable to avoid doing that if possible.

joaom00 commented 1 year ago

Nice post @filiptammergard! and nice blog too 😄. I had no idea about this side-effect either until this issue.

filiptammergard commented 1 year ago

Thanks! What do you think about using onMouseDown instead of onPointerDown in this case, to avoid the side-effect? 🙂

joaom00 commented 1 year ago

Since there is "compatibility mouse events", it seems to be a good option, but I think they would have to do it in other components too

joaom00 commented 1 year ago

Coming here again to say that using onMouseDown instead of onPointerDown seems more interesting since it would solve #1912 and #1641.

The mousedown event doesn't fire when scrolling the page while "touchdown" trigger. @filiptammergard wrote a great post about this and you can test this behavior there.

TmLev commented 1 year ago

Is there any workaround we can use until it's resolved by Radix team?

joaom00 commented 1 year ago

@TmLev Given that onMouseDown might be a good candidate to fix the issue, in the meantime, you can rewrite the logic in the onMouseDown event and use event.preventDefault in onPointerDown. I don't know if this will break some others behaviors 🤔

Actually, it doesn't work.

https://github.com/radix-ui/primitives/blob/21a7c97dc8efa79fecca36428eec49f187294085/packages/react/dropdown-menu/src/DropdownMenu.tsx#L117-L126

TmLev commented 1 year ago

onMouseDown didn't work for me – had to use onClick, but it helped with the scrolling issue. Thanks!

joaom00 commented 1 year ago

@TmLev Ohh, I made a mistake. Sorry for that! 😅

When you use event.preventDefault in onPointerDown, the onMouseDown is not fired.

Just a note, using onClick could work but you lose the ability to select a value in one click lifecycle (down, move, up).

TmLev commented 1 year ago

Just a note, using onClick could work but you lose the ability to select a value in one click lifecycle (down, move, up).

So what solution would be better in that case?

joaom00 commented 1 year ago

So what solution would be better in that case?

I think this could work