-
Notifications
You must be signed in to change notification settings - Fork 0
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
Updated input validity check #3
Conversation
@sebastianseilund Need your thoughts on an alternative to this solution. We can't use unformat() to check validity because it's a bit of a doofus about the way that it handles non-numeric values. I actually raised a stink about this at https://github.com/adamwdraper/Numeral-js/ because there's no way to actually check a number's validity. If you do I think the best solution would be to do a removal of the current locale's thousands delimiter, then |
For reference, here was my issue: adamwdraper/Numeral-js#53 |
Ok, updated. My solution was to load numeral-languages, then pull the delimiters and use a regex to parse them out. Once the thousand delimiters are removed and the decimal delimiters are replaced with '.', the internal function should be good enough to determine whether or not it's valid. |
value = value.replace(re, ''); | ||
re = new RegExp('\\' + decimalDelimiter, 'g'); | ||
value = value.replace(re, '.'); | ||
return !isNaN(value); |
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 haven't checked out the code yet, but isn't value
a string here, and therefore !isNaN(value)
will always return true (because a string is not NaN
)?
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.
No, it will take the string and cast it as a number first. For instance, !isNaN('2.5') === true
.
Alright, let's get this one in then! |
Updated input validity check
Fixes billysbilling/billy-webapp#342
Fixes #1
Fixes #2