Document and/or fix/prevent edge cases.

AndrolGenhald commented 3 years ago

I was thinking about this a bit today when looking through the code, there are a lot of edge cases but I'm not sure there's a good way to deal with them. Several could be solved by ensuring the value passed to format is either a number or a formatted string, which would allow removing the prefix and suffix correctly, but it's possible for the user to add input in the middle of the prefix or suffix. Adding a selection event handler to prevent the cursor from going into the prefix or suffix could solve that problem well enough at the cost of potentially strange interactions with other JavaScript code modifying input values.

[x] A prefix or suffix containing '-' causes the value to always be negative, format checks for the existence of a - in the string to decide if it should be negative, and this happens before removing the prefix and suffix. Even if it's moved after the prefix and suffix removal it would be an issue if the user entered it into the middle of the prefix or suffix.
[x] Numbers in the prefix or suffix cause the value to change unpredictably. I tried a suffix of '.00' while keeping precision 2, and it removes the '.00' from the precision formatting, effectively dividing by 100 if you ever enter two 0s in a row. It seems like format can be called recursively in some cases? This specific problem won't occur with reasonable usage, a suffix of '.00' could be useful but precision would likely be set to 0 in that case, but I could also see having a suffix of '000' for a field whose value is stored as thousands or something, which would have the same problem.
[x] Setting the decimal to include a digit causes the input to be maxed instantly. Should digits be disallowed from the option, with an error thrown?
[ ] The thousands option has the same issue, but it only appears once the user enters a large enough number.
[ ] The onkeydown handler for the directive checking 'Backspace' and 'Delete' seems to cause some weird interactions, but I haven't reproduced it consistently.

jonathanpmartins commented 3 years ago

I was thinking about this a bit today when looking through the code, there are a lot of edge cases but I'm not sure there's a good way to deal with them. Several could be solved by ensuring the value passed to format is either a number or a formatted string, which would allow removing the prefix and suffix correctly, but it's possible for the user to add input in the middle of the prefix or suffix. Adding a selection event handler to prevent the cursor from going into the prefix or suffix could solve that problem well enough at the cost of potentially strange interactions with other JavaScript code modifying input values.

Wow... you are brainstorming much more than I do! kkkkkk Good to see that!

Numbers in the prefix or suffix cause the value to change unpredictably.

I did catch that issue first time today taking a look at the example page. It is a edge case, but would be nice if the lib could tread it accordingly.

Setting the decimal to include a digit causes the input to be maxed instantly. Should digits be disallowed from the option, with an error thrown?

The thousands option has the same issue, but it only appears once the user enters a large enough number.

I think this is the way! Lock it and throw and error!

The onkeydown handler for the directive checking 'Backspace' and 'Delete' seems to cause some weird interactions, but I haven't reproduced it consistently.

This of course can be improved. It is there to make the "allow-blank" feature to work. https://github.com/jonathanpmartins/v-money3/issues/13

You are welcome to become a full time collaborator if you will. My time is short at the moment and I will be glad to have partners to speed up features and improve misbehavior in this project!

jonathanpmartins commented 3 years ago

What issues would you like to Document as "non resolvable problems", and what are the issues that would be nice to implement?

AndrolGenhald commented 3 years ago

I might put in a pull request or two over the next couple weeks, but I don't think any of this is really urgent besides maybe the allow-blank handling misbehaving, which may have been due to interaction with another bug. Just wanted to get some thoughts written down on what I thought could be improved when I was looking through things.

jonathanpmartins commented 3 years ago

Nice... Pull requests will be welcomed! In the mean time I will focus on better testing to get all this edge cases well documented!

AndrolGenhald commented 3 years ago

Well it looks like you've already handled all of the easy ones! I also appreciate #29, that's a good UX improvement I briefly thought about but didn't write down.

jonathanpmartins commented 3 years ago

The easy and fast ones! hehehe Should number be restricted from prefix and suffix too? What do you think about it?

AndrolGenhald commented 3 years ago

Should number be restricted from prefix and suffix too? What do you think about it?

Honestly that's probably the easiest solution. It's nice to leave it as open as possible so users don't run into restrictions, but I don't think there's a good way to fix this, and it's probably better to prevent it than to have someone try it and get confusing behavior. If anyone complains it can always be revisited in the future.

jonathanpmartins commented 3 years ago

I honestly prefer to throw new Error. It create a feedback loop that show the developer the boundaries of the library.

jonathanpmartins commented 3 years ago

I honestly prefer to throw new Error. It create a feedback loop that show the developer the boundaries of the library.

I change my mind. Throwing error can cause unexpected problems with reactivity.

Number are now filter on decimal and thousands too.

jonathanpmartins / v-money3

Document and/or fix/prevent edge cases. #26