primitives icon indicating copy to clipboard operation
primitives copied to clipboard

[DropdownMenu] `DropdownMenu.Trigger` reacts to scrolling on mobile devices

Open TmLev opened this issue 2 years ago • 6 comments

Bug report

Current Behavior

Scrolling on mobile devices may trigger DropdownMenu.Trigger even when not intended. In my case, I have a table where rows have DropdownMenu.Trigger as a last column. Scrolling with right thumb almost always triggers some row's dropdown menu.

https://user-images.githubusercontent.com/22966523/215288503-eb96c5c3-1b14-4ad3-b877-3956c3c092c1.mov

Expected behavior

Scrolling shouldn't act as a regular tap.

Reproducible example

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

Toggle responsive mode.

Suggested solution

This may be hard, I don't know. Track movement and open menu only when touch cursor doesn't move far enough?..

Additional context

Nothing.

Your environment

Software Name(s) Version
Radix Package(s) @radix-ui/react-dropdown-menu 2.0.2 (latest)
React n/a 18.2
Browser Chrome Any recent one
Assistive tech
Node n/a
npm/yarn
Operating System Android and macOS 13 / 12

TmLev avatar Jan 28 '23 20:01 TmLev

I'm facing the same issue in my app. The behavior seems to be caused by using onPointerDown event. https://github.com/radix-ui/primitives/blob/c31c97274ff357aea99afe6c01c1c8c58b6356e0/packages/react/dropdown-menu/src/DropdownMenu.tsx#L117

This event fires always when the trigger is touched. Wouldn't it better to use onClick, which also fires on mobile devices, but wouldn't fire when the user is only scrolling?

Fensterbank avatar Nov 21 '23 15:11 Fensterbank

@TmLev In case you're still blocked by this issue, I created a local patch which solves the issue in our product.

patches/@radix-ui+react-dropdown-menu+2.0.6.patch

diff --git a/node_modules/@radix-ui/react-dropdown-menu/dist/index.js b/node_modules/@radix-ui/react-dropdown-menu/dist/index.js
index c05924d..205468b 100644
--- a/node_modules/@radix-ui/react-dropdown-menu/dist/index.js
+++ b/node_modules/@radix-ui/react-dropdown-menu/dist/index.js
@@ -118,7 +118,7 @@ const $d1bf075a6b218014$export$d2469213b3befba9 = /*#__PURE__*/ $7dQ7Q$react.for
         disabled: disabled
     }, triggerProps, {
         ref: $7dQ7Q$radixuireactcomposerefs.composeRefs(forwardedRef, context.triggerRef),
-        onPointerDown: $7dQ7Q$radixuiprimitive.composeEventHandlers(props.onPointerDown, (event)=>{
+        onClick: $7dQ7Q$radixuiprimitive.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) {
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..d7d8eb0 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,7 @@ const $d08ef79370b62062$export$d2469213b3befba9 = /*#__PURE__*/ $9kmUS$forwardRe
         disabled: disabled
     }, triggerProps, {
         ref: $9kmUS$composeRefs(forwardedRef, context.triggerRef),
-        onPointerDown: $9kmUS$composeEventHandlers(props.onPointerDown, (event)=>{
+        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) {

You can apply this patch using patch-package as your postinstall script.

Fensterbank avatar Nov 21 '23 16:11 Fensterbank

Had the same problem and now fixed it by preventing the event on the onPointerDown event (thanks to your comment @Fensterbank ), so far it seems to solve the issue.

const [open, setOpen] = useState(false);
//...    
<DropdownMenu open={open} onOpenChange={(open) => setOpen(open)}>
      <DropdownMenuTrigger
        asChild
        onPointerDown={(e) => e.preventDefault()}
        onClick={() => setOpen(!open)}
      >
          <Button>click me</Button>
      </DropdownMenuTrigger>
//...
</DropdownMenu>

Kulimantang avatar Nov 22 '23 08:11 Kulimantang

Follow up: I did notice after that change, that if I use the mouse to click on the trigger to close the open dropdown, it directly re-opens. Probably this happens because any blur effect closing the dropdown happens before the onClick event.

Therefore the solution of @Kulimantang is the better way due to more flexibility. In our case, in the mobile view, the dropdown content is full size (so you can't click the trigger to close it), so this is a good approach that works in both scenarios:

const isTouchDevice = 'ontouchstart' in window
const [open, setOpen] = useState(false);
//...    
<DropdownMenu open={open} onOpenChange={(open) => setOpen(open)}>
      <DropdownMenuTrigger
        asChild
        {...(touchDevice
          ? {
              onPointerDown: (e) => e.preventDefault(),
              onClick: () => setOpen(!open)
            }
          : undefined)}
      >
          <Button>click me</Button>
      </DropdownMenuTrigger>
//...
</DropdownMenu>

Fensterbank avatar Dec 04 '23 07:12 Fensterbank

Thanks for the solutions! This should be fixed by Radix itself! Any updates? Would a PR be welcome?

jackblackCH avatar Feb 07 '24 17:02 jackblackCH

I just want to add that another side effect of using onPointerDown is if you want the dropdown content to appear on top of the trigger, whichever dropdown button is under your cursor when the dropdown opens gets clicked immediately.

xvvvyz avatar Aug 01 '24 16:08 xvvvyz