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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

useDrag on a parent element triggering onClick for child elements #173

Closed
lourd opened this issue May 17, 2020 · 8 comments
Closed

useDrag on a parent element triggering onClick for child elements #173

lourd opened this issue May 17, 2020 · 8 comments

Comments

@lourd
Copy link

lourd commented May 17, 2020

Hey y'all,

Long time listener, first time caller 馃樃First off -- this library, and the other collection of react-spring libraries, are top-notch. It's so evident from using them that the authors care deeply about the excellence of their products and the tools they use to build them. The abstractions are good, the docs are thorough and clear (and beautiful!), and the examples are plentiful and up-to-date. Thank you all for the thought and effort that's gone into these projects 馃挋

I have a question about event capturing with useDrag. I'm making a prototype that needs to emulate touch scrolling on desktop. I've figured out how to do that, but have the issue where items within the scrolling have their onClick handlers triggered when scrolling around. This makes sense, but I was wondering if y'all have pointers or ideas for workarounds. Hacky suggestions welcome, this is just a prototype.

I made a demo of the challenge. The ideal behavior is that of typical mobile scrolling -- once a user starts scrolling the view by clicking and dragging (inadvertently on an element), releasing their finger/the mouse button while still over the element should not trigger onClick. As you can see from the demo, it is triggered.

I was hoping using onClickCapture for the child element in conjunction with the eventOptions.capture option to useDrag might work, but alas, it does not.

Thanks in advance for any help you can offer 馃枛

@dbismut
Copy link
Collaborator

dbismut commented May 17, 2020

Hey @lourd thanks for the praise! Your challenge is not trivial, but I think there's a few ways we could address it, more or less elegantly.

The main problem actually comes from the chronology between mouseup and click events: ideally, there would be a way to signify to the children鈥檚 click handler that the scroller has been dragged, and therefore to cancel the click logic if this was the case. Something like:

/* this doesn't work !! */
// useDrag logic
useDrag(
  ({ active }) => {
    hasDragged.current = active
    /* rest of the logic */
  },
  { filterTaps: true }
)

// children click logic
<div onClick={ () => if(!hasDragged.current) alert('clicked', index) } />

// 1. user starts dragging -> hasDragged is true
// 2. user stops dragging
// 3. the children click handler fires but hasDragged is true so the logic doesn't run
// 4. useDrag sets hasDragged back to false 

Unfortunately, useDrag relies on mouseup which triggers before onClick (even when playing with capture I believe), so hasDragged.current is always false when the children click handler fires 馃槩

Notice the filterTaps options which you weren't using and that makes the drag handler not respond to gestures that have a displacement smaller than 2 pixels. In all solutions, this is a useful option.

Possible Solutions

These possible solutions are not hard tested, you can explore each of them further!

1. Reset the hasDragged flag in the scroll container onClick handler.

Not perfect implementation but might be sufficient in your case: since the onClick handler on parents triggers after the children handler, use the parent to reset the hasDragged handler. Not very pretty IMO.
https://codesandbox.io/s/t62ch

2. Delay resetting the hasDragged flag with setTimeout

We artificially set the hasDragged flag after the children click has triggered with setTimeout and a 0ms delay.

https://codesandbox.io/s/i3enu

3. filterTaps everywhere

Here we wrap children inside a component that also has a useDrag handler with the filterTaps set. Not capture necessary. Note that this compiles to mousedown and mouseup handlers exclusively, no click handler is effectively used.

https://codesandbox.io/s/z0gru

4. use onMouseUp in children instead of onClick

The logic I was describing in my pseudo code earlier works with mouseup!
https://codesandbox.io/s/x823n

5. use stopPropagation

This one is probably the most scalable, since it removes the logic from the children handler, which would be useful if you have nested components that can't know about the parent drag status. The idea is to use capture and mix it with idea number 2 and stop propagation to children from the parent click handler, therefore removing the need for the hasDragged flag in the children logic.

https://codesandbox.io/s/geux9

Here you go :) Let me know which one you prefer :)

@lourd
Copy link
Author

lourd commented May 17, 2020

Holy cow @dbismut, this is gold! Your ultimate conclusion is brilliant, solves the problem absolutely perfectly. Bravo 馃憦馃憦

You were so helpful I gave $20 to the react-spring OpenCollective. Seriously, thank you! 馃檹

@lourd lourd closed this as completed May 17, 2020
@dbismut
Copy link
Collaborator

dbismut commented May 17, 2020

That鈥檚 really cool, much appreciated :)

@dbismut
Copy link
Collaborator

dbismut commented May 18, 2020

Btw, this will be the default in next version of react-use-gesture when setting filterTaps: true. I realize this is a common use case that should be addressed in a more simple way! Thanks for reporting it @lourd :)

@puckey
Copy link

puckey commented May 21, 2020

@dbismut Wow, I lucked out with this well formulated issue and the list of possible solutions. I just matched @lourd's donation as a small token of thanks.

@dbismut
Copy link
Collaborator

dbismut commented May 21, 2020

That means a lot thank you @puckey

@Rohit544

This comment has been minimized.

@enheit
Copy link

enheit commented Sep 17, 2021

I had a similar issue with useDrag function where onClick handler was triggered on the child elements during drag gesture. The filterTraps: true prop fixed this problem.

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

5 participants