primitives icon indicating copy to clipboard operation
primitives copied to clipboard

Dropdown Menu Button is onPointerDown as opposed to OnClick on a touch screen

Open gary-lo opened this issue 2 years ago • 6 comments

Bug report

Current Behavior

As soon as a touch is detected on the dropdown menu trigger, the menu appears as opposed to only when it's clicked.

This is most noticeable if this is on a scrollable area. Instead of being able to scroll, the menu pops up.

Expected behavior

Only show the menu when it's a click (onTouchUp)

As per the description in the documentation. "Displays menu...triggered by a button". Expected behaviour of a button is a click not onPointerDown

Reproducible example

Load the official documentation on a smartphone.

https://www.radix-ui.com/primitives/docs/components/dropdown-menu

Press down on the button (don't lift up) Scroll up with your finger.

Instead of being able to scroll the menu will pop up.

Suggested solution

Replace the trigger with onClick instead of onPointerDown?

Additional context

Your environment

Software Name(s) Version
Radix Package(s)
React n/a
Browser
Assistive tech
Node n/a
npm/yarn
Operating System

gary-lo avatar Sep 28 '23 09:09 gary-lo

Ok I looked at the source and as suspected the popover trigger is: https://github.com/radix-ui/primitives/blob/c31c97274ff357aea99afe6c01c1c8c58b6356e0/packages/react/popover/src/Popover.tsx#L151

Whereas DropDownMenu is https://github.com/radix-ui/primitives/blob/c31c97274ff357aea99afe6c01c1c8c58b6356e0/packages/react/dropdown-menu/src/DropdownMenu.tsx#L117C1-L126C14

          onPointerDown={composeEventHandlers(props.onPointerDown, (event) => {
            // only call handler if it's the left button (mousedown gets triggered by all mouse buttons)
            // but not when the control key is pressed (avoiding MacOS right click)
            if (!disabled && event.button === 0 && event.ctrlKey === false) {
              context.onOpenToggle();
              // prevent trigger focusing when opening
              // this allows the content to be given focus without competition
              if (!context.open) event.preventDefault();
            }
          })}

Not quite sure what the reasoning behind using onPointerDown is - but it would cause this side-effect particularly noticeable on touch screens.

gary-lo avatar Sep 28 '23 09:09 gary-lo

patch-package to fix it in the mean time:

diff --git a/node_modules/@radix-ui/react-dropdown-menu/dist/index.mjs b/node_modules/@radix-ui/react-dropdown-menu/dist/index.mjs
index aa181ce..a796748 100644
--- a/node_modules/@radix-ui/react-dropdown-menu/dist/index.mjs
+++ b/node_modules/@radix-ui/react-dropdown-menu/dist/index.mjs
@@ -78,7 +78,16 @@ const $d08ef79370b62062$export$d2469213b3befba9 = /*#__PURE__*/ $9kmUS$forwardRe
         disabled: disabled
     }, triggerProps, {
         ref: $9kmUS$composeRefs(forwardedRef, context.triggerRef),
-        onPointerDown: $9kmUS$composeEventHandlers(props.onPointerDown, (event)=>{
+        // onPointerDown: $9kmUS$composeEventHandlers(props.onPointerDown, (event)=>{
+        //     // only call handler if it's the left button (mousedown gets triggered by all mouse buttons)
+        //     // but not when the control key is pressed (avoiding MacOS right click)
+        //     if (!disabled && event.button === 0 && event.ctrlKey === false) {
+        //         context.onOpenToggle(); // prevent trigger focusing when opening
+        //         // this allows the content to be given focus without competition
+        //         if (!context.open) event.preventDefault();
+        //     }
+        // }),
+        onClick: $9kmUS$composeEventHandlers(props.onClick, (event)=>{
             // only call handler if it's the left button (mousedown gets triggered by all mouse buttons)
             // but not when the control key is pressed (avoiding MacOS right click)
             if (!disabled && event.button === 0 && event.ctrlKey === false) {

Also, related issue: https://github.com/radix-ui/primitives/issues/1641

arpitdalal avatar Oct 09 '23 20:10 arpitdalal

Thanks @arpitdalal

gary-lo avatar Oct 16 '23 03:10 gary-lo

You can also solve this problem without patching. Prevent the menu from opening on the pointerdown event and instead open it on the click event

const [open, setOpen] = useState(false);

<DropdownMenu.Root open={open}>
  <DropdownMenu.Trigger 
    onPointerDown={(e) => e.preventDefault()}
    onClick={() => setOpen((prev) => !prev)}
  >
    click me
  </DropdownMenu.Trigger>
   {/* other dropdown elements... */}
</DropdownMenu.Root>

layerok avatar Dec 13 '23 15:12 layerok

Great solution, @vovarudomanenko! I found handling onOpenChange beneficial as well, to achieve the same behavior as before changing the onPointerDown event. This way, the blur event will work as expected, so you can click outside the dropdown menu to close it. You can set onOpenChange={setOpen} on the DropdownMenu.Root component.

const [open, setOpen] = useState(false);

<DropdownMenu.Root open={open} onOpenChange={setOpen}>
  <DropdownMenu.Trigger 
    onPointerDown={(e) => e.preventDefault()}
    onClick={() => setOpen((prev) => !prev)}
  >
    click me
  </DropdownMenu.Trigger>
   {/* other dropdown elements... */}
</DropdownMenu.Root>

paaskus avatar Jan 21 '24 23:01 paaskus

In my case I ran into another problem. Somehow the solutions above disabled the capability to close the modal by clicking outside of it.

This is what worked for me:

const [open, setOpen] = useState(false);
const isTouchDevice = typeof window !== 'undefined' && 'ontouchstart' in window;

<DropdownMenu.Root open={open} modal={false} onOpenChange={setOpen}>
<DropdownMenu.Trigger 
  {...(isTouchDevice
        ? {
            onPointerDown: (e) => e.preventDefault(),
            onClick: () => setOpen(!menuIsOpen),
          }
        : undefined)}
>
  click me
</DropdownMenu.Trigger>
</DropdownMenu.Root>

gmhislop avatar Feb 05 '24 09:02 gmhislop

Duplicate of #1912

benoitgrelard avatar Mar 06 '24 14:03 benoitgrelard