Closed AndrolGenhald closed 3 years ago
Great job! Cleaner is always better. Make the house look pretty! heheh
I need a little more time to take a deep look and think more about it. One thing for sure is that we should support integers or string representation of integers coming in from the database (prop modelValue). I did took a brief look and I believe that integers are correctly covered in your code, but string representations of integers not.
Fixed a few things, but #7 behavior is still a little weird. I see a few options:
"1234"
results in "1234.00"
)"1234"
becomes "12.34"
)Number
it will be formatted, but all other cases are formatted (starting with 12.3
results in "12.30"
, pasting "12.3"
results in "1.23"
).If we want pasting integers to stay as integers, allowBlank = true
will be a little weird, and I have no idea how to fix it. If the input is blank and the user types "12"
, it will result in "10.02"
, because it'll detect the "1"
as an integer, and the 2 will be appended to the "1.00"
.
I also added special behavior for precision === 0
, if the input receives (through starting value or user input) a decimal, it will round it instead of formatting it (eg "12345.6789"
becomes "12,346"
instead of "123,456,789"
).
If we want pasting integers to stay as integers,
allowBlank = true
will be a little weird, and I have no idea how to fix it. If the input is blank and the user types"12"
, it will result in"10.02"
, because it'll detect the"1"
as an integer, and the 2 will be appended to the"1.00"
.
Yes, you caught it.... That weird first type problem. I'm messing with it since the beginning. Tried to fix this before it arrived on the format function.
I don't have time at the moment to look into why those puppeteer tests aren't working as expected, I'll just comment them out and let you merge it as is if you think everything else looks good. Hopefully next week I'll have some more time to investigate.
Cool! No probs... I'm out of time for now too. I will probably pull it in a analyze it next week.
This pull request will probably not be merged. I was working on BigInt integration (branch bigint) and after some refactoring part of this code wasn't needed anymore. On the other hand part of this code was super helpful and was included by comparing branches (copy/paste
). Credits to @AndrolGenhald making a really good job analyzing the code and bringing in various edge cases.
Some features and tests not currently included will be included in the final release of this refactoring.
I was actually thinking about trying to use BigInt
instead of string
, but I was already more than half done getting arbitrary precision working with string
, so I figured I'd save it for later. If you've got it working with BigInt
that's even better!
Using BigInt
will decrease compatibility with old browsers. The problem is that I can only compile the project using es2020
.
At this point is not just about BigInt
but everything else that es2020
support. A lot of new features will not be transpiled down.
Support arbitrary precision values, rather than losing precision by parsing into a 64-bit float.
Work in progress, I broke #7 while simplifying
format
andunformat
, but the way it works right now the fix for #7 also seems to cause unexpected behavior, so I think the actual desired behavior for that needs to be thought out a bit more.