-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Fix for touchEnded triggers twice among other issues with touchEnded/mouseReleased on mobile #6738
Conversation
@@ -731,6 +731,11 @@ p5.prototype._onmouseup = function(e) { | |||
const context = this._isGlobal ? window : this; | |||
let executeDefault; | |||
this._setProperty('mouseIsPressed', false); | |||
|
|||
if (this.touchend) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this get called implicitly because the browser triggers a mouse event after the touch event? It'd be helpful adding a comment here explaining if so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add a comment for this and the other PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the comment
I think this looks good to me, but could an events steward with more familiarity with the control flow double check? @limzykenneth, @richardegil, @angelabelle, @littlejacinthe, @TanviKumar, @tuminzee |
Could you guys also check if this sorta thing happens to any of the other events for touchscreens? The fact that this happened twice (#6740) might mean that this sorta issue's in other event handlers that deal with touch as well. |
Hello everyone, I just want to add that I did the tests for this on Safari, and it all looks good. I also tried it out on Chrome on my Android phone, and Safari on my iPhone behaved the same way. So, it seems like things are working fine on both browsers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for testing this out! I'm going to merge this in now that we've confirmed it also works as expected in different platforms.
Resolves #6737
Changes:
Adds an instance variable called touchend to p5 and changed p5.prototype._ontouchend & p5.prototype._onmouseup to function as documented.
On touchscreen
PR Checklist
npm run lint
passes