onClick fired long after endDrag() if still holding mouse button and releasing it over the previously dragged object

[Beholder4096]

Hello guys, it's Beholder4096, again with another bug to figure out. This time I have made a workaround for it, but it looks nasty in code so I'd rather wish this to work normally, as expected. Also this time, it's not really an obscure android issue but a real tangible testable bug so I am sure it will take you no time to figure out or correct me if I am thinking wrong.

 

So it looks like another issue with endDrag(). It seems like it doesn't really end the dragging at all in one special circumstance, when user HOLDS the mouse button, the endDrag() "ends the dragging" and then the user RELEASES the button with cursor over the thing that was supposed to "end being dragged". Please run at the attached CP and try this yourself:

 

When the square turns green grab it by the pu**y gently with mouse cursor and hold the mouse button. Move your mouse to the right until endDrag() "ends the dragging", still holding the mouse button. Then move the cursor over the green square again while still holding the mouse button down and see what happens when you release the mouse button over that square.

 

What should happen: nothing. The cursor should even not be the Trump-hand just before releasing the mouse button, it should turn to normal cursor on endDrag() and keep it as such as we clearly want to end the drag.

 

What really happens: onClick() of the draggable object is fired. Which is wrong, I think. And it clearly shows as very wrong in my webapp, the additional onClick() firing is completely unintended yet sometimes unavoidable there if this bug stays in. Therefore I had to make a small workaround. T U for reading this and checking it out.

 

See the Pen yLKVBjX by Beholder4096 (@Beholder4096) on CodePen

[GreenSock]

Well it's debatable if that'd be considered a "bug" strictly-speaking because in the case you're talking about you did press down on the element, then end the drag, and then a while later release the mouse while over the same element (and according to Draggable, you're not dragging at that point anymore), thus the browser will interpret that as a "click" and dispatch an event accordingly. Try for yourself - totally outside of Draggable, add a typical "click" listener and you'll see what I mean. 

 

That being said, I can see why it'd be helpful in this extremely uncommon edge case to prevent that onClick from firing if there was a drag at any time between mouse down and mouse up, so I added some code to do that in the beta version: 

https://assets.codepen.io/16327/Draggable3.min.js

 

Does that work the way you hoped? 

[Beholder4096]

Hmm, actually no. Seems to work exactly as before: the onClick fired when "cancelled" drag was released over the previously dragged element. When we cancel the drag, we want to help the user by lifting the virtual finger from screen/mouse.

 

As for the "extremely" uncommon case, imagine a stack of cards  and you want to use your mouse to frantically "toss" them one by one from the stack.  Similar to tossing dollar bills from hand onto strippers by placing the bills on one hand and tossing the individual bills one by one with another hand in a quick succession. I also preemptively cancel the dragging of the object mid-way and help the user by finishing the drag by animation: it is very responsive and quite natural. But INEVITABLY it will happen, that you will release your click on a card that is currently either animating or is being tossed already after animation. And then the onclick will happen which runs an unwanted action. I could do workarounds but why would an onclick happen after a release, not after press+release only? I guess this is more of a philosophical than programming issue though.

[GreenSock]

8 minutes ago, Beholder4096 said:

Hmm, actually no. Seems to work exactly as before

I bet you forgot to clear your cache and your browser was using the old file.

 

Here's a fork that just updated the Draggable file itself (with cache-busting link): 

See the Pen MWVbwWj?editors=0010 by GreenSock (@GreenSock) on CodePen

[Beholder4096]

nope, I think I have the correct file loaded; I have modified it by a small mark on top of the file and see that same mark in the dev tools, so it's not working as intended here.. I have rechecked my implementation and it seems fine, should work like the code pen but it does not. The onclick fires on the release consistently. I can give you the site and you can immediately see for yourself if you want.

 

[Beholder4096]

basically you can just toss the cards by doing upward drag, they will release from mouse by themselves. Releasing the mouse on a tossed card still shows the problematic behavior (with the correct file I think, forced those links as well).

[GreenSock]

I'm leaving my desk so I can't dig into this right now, but let me explain what I was testing (perhaps I misunderstood): 

• Press/Drag to the right until the box stops, triggering the .endDrag(). Keep pressing...
• While continuing to press, move off the element and come back again
• Release while over the element

Old behavior: onClick was fired (which resulted in an alert() window in your demo)

New behavior: onClick is NOT fired (no alert()). 

[Beholder4096]

exactly, it's the test case I made you, so I know how it works. Your testing/methodology was fine, your fix seems to work on the code pen. But when I try to load your file (or the code pen file) into my site, it loads but there is still no fix. And assuming this problem is trivial to set up and that I can check my own code for not making an obvious mistake of some kind, there must be some kind of problem still in your code that doesn't work here although it may in the pen.

 

I also tried to compare differences between old 3.11.0 and the new 3.11.0 but there are many and since the code is practically obfuscated, it would be very difficult to check for this particular difference and see what is going on in there.

 

I must go to get at least a nap myself, I will leave the site on for you so go ahead and check it out anytime you want.  I also see I must do something about calling Draggable.create() with an element that already has an instance, because an additional click listener is being piled on that element by each additional Draggable.create(). It didn't matter so far because I was working on reporting those bugs, but today I must do something about it already.

 

 

[GreenSock]

Can you please provide a simple codepen demo that illustrates the issue? That will allow us to be FAR more productive. Troubleshooting a live site is...not good. 

 

So you can verify that my fix did indeed resolve the CodePen provided earlier, right? 

[Beholder4096]

Yep, I confirm, I figured it out completely:

The same bug you already fixed happens when you .disable() and then .enable() the already created draggable instance on the object (while holding the mouse button of course and then releasing it over the object). Also, it's the same thing with .kill(). Here is the pen 

See the Pen PoRbZay by Beholder4096 (@Beholder4096) on CodePen

In my case it is even more special, I basically call Draggable.make(oObj) on the same oObj multiple times because I remove the object from the DOM very soon anyway, so it pretty much doesn't matter (although it's not nice and I have to do something about it). This also causes the same behavior, along with creation of  additional Draggable's listeners that are already present for the oObj anyway.

 

Thanks for looking into this so extensively.This bug represents a kind of anomalous behavior that makes the draggable objects a bit more unpredictable, if nothing else. So I believe it should be fixed, although it can be quite easily prevented with a workaround.

[GreenSock]

Yeah, what an odd edge case  I updated the latest beta to maintain that state variable even after disable() now which seems to work. 

[Beholder4096]

I never got the link to the new beta version, am I missing something? Do you have some kind of repo I don't know about?

[GreenSock]

I provided the link above. Same one. 

https://assets.codepen.io/16327/Draggable3.min.js

 

Here's a fork: 

See the Pen QWmGQrE by GreenSock (@GreenSock) on CodePen

[Beholder4096]

Hmm, .kill() still allows the same bug to return from the dead.

 

Other than that, works really nice, thank you! One more possible bug to solve (will post in another thread) and I can finally move to normal development.

[Beholder4096]

Actually no, it also seems to be a small bug in my own implementation, since kill() removes the object's Draggable instance from lookup table, I cannot find it and then I recreate the draggable instead of just enabling it. It is the Draggable.create() on the old object that causes the same bug, just as I reported earlier.

 

I will also fix this in my code somehow.

[GreenSock]

12 minutes ago, Beholder4096 said:

Hmm, .kill() still allows the same bug to return from the dead.

 

I cannot replicate that - what are the steps to reproduce exactly? Can you please provide a minimal demo? 

 

You aren't trying to enable() a killed Draggable, are you? You definitely shouldn't. kill() should be considered permanent.

[GreenSock]

1 minute ago, Beholder4096 said:

Actually no, it also seems to be a small bug in my own implementation, since kill() removes the object's Draggable instance from lookup table, I cannot find it and then I recreate the draggable instead of just enabling it. It is the Draggable.create() on the old object that causes the same bug, just as I reported earlier.

 

Okay, so you're all set now? No more bug reports? 

[Beholder4096]

yes, that's what I was doing, you're right. I reported earlier. Our posts crossed. Thank you.

 

1 more unrelated issue, will post in another thread.

[Beholder4096]

Actually, I have realized there are no more issues. The ones that I had after this I was able to sort out myself. Thank you again for your support. I might have an interesting suggestion or two later but I need time to think them through and prepare test cases.