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

Reconcile handling of mouse and touch drag start events #545

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

szakharchenko
Copy link

Attach the mousedown event handler to the node, likewise to the touchstart, so that it may be cancelled if needed and otherwise handled more uniformly

…events

Attach the mousedown event handler to the node, likewise to the
touchstart, so that it may be cancelled if needed, avoiding a click
after drag.
@STRML
Copy link
Collaborator

STRML commented Mar 5, 2021

Hmm, this is interesting. Can you point me to a use case / example code where this change makes a difference for you?

@szakharchenko
Copy link
Author

I'm not sure I can. Having different code paths for mouse and touch events just looks inelegant (e.g. one receives a browser event and another a synthetic event). I made this change as part of handling click-vs-drag issues like #531, #419 and #409 . It seems like I've made things to work even without this change though, so I can't say it's absolutely necessary, it just looks like it would make things a bit cleaner. This is a merge offer, not a pull request;)

@szakharchenko
Copy link
Author

The comment would need to be reworded though; this doesn't by itself make it possible to cancel the click event after drag.

@STRML
Copy link
Collaborator

STRML commented Mar 5, 2021

That makes sense. I'll test it and see if this changes anything upstream.

Pkmmte added a commit to Pkmmte/react-draggable that referenced this pull request Apr 28, 2021
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

Successfully merging this pull request may close these issues.

2 participants