allow removal of thousands separator #144
Conversation
Set default prefix to "US$ " and default affixStay to "false". Added thousandsStay setting with default value of "false".
|
I just forked this repository to make this same change for my own purposes. Definitely seems like a useful feature so that decimal values can be parsed from masked fields. |
|
I'm not sure why it's saying that the change event isn't firing, btw. I reviewed the code and it looks fine. The logic for the input.change() is preserved.... |
Changed prefix, affixesStay, and thousandsStay default values from personalized to more general purpose settings.
|
I'm not sure either, I've created a pull request for my similar solution to see if the tests pass. |
|
That did make it pass, but I'm not all that happy about setting and reading from the input twice. This solution does it a maximum of twice, I believe yours does it 2-3 times (1 is extraneous I think), but really it should only need to be done once. I'm still not sure what was causing the single access method to fail though. |
|
My solution will only read and set from the input once, even if both affixStay and thousandsStay are set to false. |
|
excellent, i may have misread then! I was pretty sure that I could make it work with one, but I haven't figured out how to send something to CI without committing, which is really annoying. Isn't there a "run all tests" feature? |
|
I didn't try running the tests locally, I just committed and the tests were run. Open the /test/index.html file in a browser though, and it should run your tests and display the results. Neatly done! |
|
Nice, good to know. I was trying to use github as editor and VCS, instead of using git as version control, and I think that was a source of many problems. I'm actually setting up git in my IDE so I can just import projects and work on them as intended. I went back and looked at your fix again and I think I was reading the diffs wrong, the colors came out better on the second computer I used and made it clear what I was reading wrong. This pull request can be closed, please merge #146 |
fixes problem where only first thousands separator is removed
|
Ok, this it. Works as intended, one read and one write, now works to remove multiple separators for numbers over 999,999.99 |
|
Good catch! |
Replaced ALL occurrences of the thousands symbol using a global regexp.
|
@gbass84 , did you see my pull request to your repo? Was hoping we could merge our efforts. |
|
I didn't until now, sorry. I'll get it merged tonight. |
Merge thousandsStay setting pull requests
|
btw, when I reproduce the tests in alive environment on my machine, the test scenarios past. I'm not on my normal dev machine now to check the tests in the browser, but it might be that the test need to be fixed/changed, because I can definitely successfully unmask a negative 0-precision value with a prefix and suffix |
|
Yea the automatic testing server is inconsistent and sometimes fails when the build will pass locally. Makes it difficult to see if there are real problems. |
|
@Nannoises @gbass84 sorry for taking so long to reply guys. we need to fix the tests before this get merged. I will try to take a look at this tomorrow :) |
|
Another thing that would be super awesome is to squash your commits(making the 21 commits into one). It's better for the repo history :) |
|
don't forget me 😿 |
|
Since we pooled our efforts into this pull request from @gbass84 , I believe he will need to do the squashing. Let me know if I can help. |
|
Out of curiosity I went looking for how to do this. @gbass84 Will need to follow these instructions to update his pull request. http://eli.thegreenplace.net/2014/02/19/squashing-github-pull-requests-into-a-single-commit |
|
Yeah I'll get it done in the next day or two tops! —Reply to this email directly or view it on GitHub. |
|
This brings it back to last the last good build related to this branch |
I thought it would be nice to allow a setting to remove the thousands separator on blur. I added it to the settings with default to "thousandsStay: true" and to the blurEvent function to string replacement.
I'm using intelliJ Idea, and it reformatted some indentation, so I apologize for the diff showing more changes than there really are.