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

minDistance option can't be initialized to 0 #299

Closed
remomueller opened this issue Sep 13, 2017 · 7 comments
Closed

minDistance option can't be initialized to 0 #299

remomueller opened this issue Sep 13, 2017 · 7 comments

Comments

@remomueller
Copy link
Contributor

remomueller commented Sep 13, 2017

Do you want to request a feature or report a bug?
Reporting a bug
The new option to set minDistance: 0 during the signature pad initialization does not work, since 0 is skipped in the 0 || 5 evaluation. JavaScript OR Evaluation

What is the current behavior?

In the following example the option is set to 0, and the resulting minDistance is displayed below the signature pad. I expected to see it assign 0, but instead it assigned 5.
https://jsfiddle.net/bvf29kdn/1/

Which versions of SignaturePad, and which browser / device are affected by this issue? Did this work in previous versions of SignaturePad?

Using v2.3.0. The assignment of 0 bug is on the following line:
https://github.com/szimek/signature_pad/blob/v2.3.0/src/signature_pad.js#L13

The following could potentially solve this issue: (Similar to how "throttle" is assigned).

this.minDistance = 'minDistance' in opts ? opts.minDistance : 5;

Thanks! (I can submit a pull request to fix as indicated above if that sounds good, just let me know!)

@szimek
Copy link
Owner

szimek commented Sep 13, 2017

Thanks for reporting this! Yes, please create a PR for this issue ;) I'll release 2.3.1 then with this fix.

@remomueller
Copy link
Contributor Author

All set, let me know if you need any changes to the PR, thanks!

@szimek
Copy link
Owner

szimek commented Sep 13, 2017

@remomueller Just one question - did you intentionally want to set minDistance to 0, because you had some issue with the default value (if so, what was the issue?) or did you just notice this issue in some other way?

@remomueller
Copy link
Contributor Author

remomueller commented Sep 13, 2017

Yep, I was using 2.2.0 of the signature pad before this, and when I tried 2.3.0, I saw that there was a note about the slight lag introduced by the feature change (and the ability to get the 2.2.0 functionality back). I tried it out, setting the distance to 0 to emulate the 2.2.0 code, but the lag appeared to still be there. I thought I wasn't using the constructor correctly, so dove into the code and manually set it to zero there.

In the end, I decided I preferred the "snappier" response time to signing as it feels better, than trying to make slower signatures look nicer. Also, signing slowly, I didn't see that great of an improvement in the look between the two.

@szimek
Copy link
Owner

szimek commented Sep 13, 2017

Thanks! Yeah, it was just an attempt to make it less pixelated when someone draws slowly. I guess it wasn't very successful after all ;) I'd need to find more time to implement a more fancy algorithm to make it smoother...

@remomueller
Copy link
Contributor Author

In any case, thanks for creating such a cool signature library! It's far ahead of the JS libraries I tried a couple of years ago. Also, I'm sorry can't help with developing JS test suite (I'm more of a Ruby developer), maybe someone can make pull requests for those!

@remomueller
Copy link
Contributor Author

Good luck! I'll be keeping an eye on the releases and changelog as you push those out!

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

2 participants