Remove event listener `touchstart` after adding it.

thread-pond / signature-pad

A jQuery plugin for assisting in the creation of an HTML5 canvas based signature pad. Records the drawn signature in JSON for later regeneration.

BSD 3-Clause "New" or "Revised" License

727 stars 292 forks source link

Remove event listener `touchstart` after adding it. #205

Closed hassaanalansary closed 4 years ago

hassaanalansary commented 5 years ago

Issue: when you start drawing by mouse and change to drawing by touch , the mouse button stay triggered. causing the undesired drawing on the canvas.
Fix: Remove event listener touchstart after adding it.
Explanation: After changing from this.ontouchstart to this.addEventListener('touchstart'). we should replace this.ontouchstart = null; by this.removeEventListener('touchstart', touchStartProcedure)

hassaanalansary commented 5 years ago

@mockdeep I am very bad at naming functions, please review function touchStartProcedure name. and change it to what you see fit. Thanks

mockdeep commented 5 years ago

@hassaanalansary thanks for following up on this!

mockdeep commented 5 years ago

@hassaanalansary you resolved the conversation above, but it doesn't look like you pushed any code changes. Is there more to be done on this?

hassaanalansary commented 5 years ago

No, i believe not. Sorry for the bad follow up. It has been crazy with me. You can accept the pull request and close it if you please. Thanks

hassaanalansary commented 5 years ago

No, i believe not. Sorry for the bad follow up. It has been crazy with me. You can accept the pull request and close it if you please. Thanks

hassaanalansary commented 4 years ago

@mockdeep can you review the change so it can be merged. I know it has been very long since I start the PR. Better late than never. :)

mockdeep commented 4 years ago

@hassaanalansary I unresolved one comment. It would be good to still rename the function. @olivertitze are you able to have another look as well?

hassaanalansary commented 4 years ago

@olivertitze fixed that

ehsanmohebbi commented 4 years ago

I reviewed and tested this request. This disables the "Draw it" option by adding the line 366: this.removeEventListener('touchstart', startTouchDrawing), and the issue is real. the switching between mouse and touch is not working. when i use touch pad to draw and then want to use the touch screen, it is not working the same at reverse.

hassaanalansary commented 4 years ago

@ehsanmohebbi Did you test this on a Desktop or mobile device? and which browser? Are you sure the issue is because of this PR or this one https://github.com/thread-pond/signature-pad/pull/203

ehsanmohebbi commented 4 years ago

@hassaanalansary I tested it on Windows 10 touch screen device using google chrome browser.

mockdeep commented 4 years ago

@ehsanmohebbi so this fixes the issue for you? When you say "This disables the "Draw it" option" do you mean it is introducing a new bug?

mockdeep commented 4 years ago

Okay, I'm going to go ahead and merge this. Feel free to open a new issue or PR if things are still broken.