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

If your "locale" uses decimal comma like "123,45" instead of "123.45", NaN crashes the whole kitty #2

Closed
Wikinaut opened this issue Mar 13, 2020 · 9 comments

Comments

@Wikinaut
Copy link

No description provided.

@Wikinaut Wikinaut changed the title If your "locale" uses decimal comma like "123,45 €" instead of "123.45", NaN crashes the whole kitty If your "locale" uses decimal comma like "123,45" instead of "123.45", NaN crashes the whole kitty Mar 13, 2020
@Wikinaut
Copy link
Author

"kitty" = common pool of money (Deutsch: Gemeinschaftskasse. Onboard yachts: Bordkasse)

@ssimono
Copy link
Owner

ssimono commented Mar 14, 2020

Hi, thanks for reporting. So to understand better:

  • Your browser locale is set to German
  • You can add an expense of say 123,45 and the form lets you add it
  • Then the computation breaks because it can processed the NaN

Is that correct?

I will first try to make a better validation for the amount, right now it just relies on the <input type="number"> whose support varies between browser. But then I am trying to figure out if one can take the locale into account to parse the number correctly. I would expect it to be handled by the input, but apparently not.

@Wikinaut
Copy link
Author

Wikinaut commented Mar 14, 2020

Yes. Yesterday, we had the first real use case, and after adding an "incorrectly" formatted amount, the whole sheet crashed - unfixable, this is the most important thing. So I suggest to fix it two-fold:

  • try to detect the correct locale ;and
  • just in case, that a NaN occurs, try to fix this, to reduce the impact. Of course, NaN should be fully avoided.

ssimono added a commit that referenced this issue Mar 14, 2020
@Wikinaut
Copy link
Author

Now, when I enter "1,23", I got this
grafik

Uh, not nice. You now could tell the user: please use "." as decimal separator - or you simply substitute that character (be aware of possible problems with the "." when someone uses that "." as a digit grouping separator like in "123.456,78").

@ssimono
Copy link
Owner

ssimono commented Mar 15, 2020

Hi, thanks for your help. So I did a little fix just to avoid breaking the count all together if an invalid number is entered. I am now looking into validating at the time of the input.

But I was still expecting the form to do some kind of validation. Could you tell me what device/browser you are using?

@ssimono
Copy link
Owner

ssimono commented Mar 20, 2020

It should now be fixed as of 9720945, which adds a much better form validation. Depending on the browser / locale, using , for decimals is actually understood correctly when entering a number (tested with Safari/iOs in German and Chrome/Andoid in French), and I am planning to rely on the native way browsers do it without interfering at the code level.

Thanks for your valuable feedback!

@ssimono ssimono closed this as completed Mar 20, 2020
@Wikinaut
Copy link
Author

I only use Firefox, and unfortunately, the detection appears not to work. I mena, when I enter "1,23" the box shows a small red border and I have to retype as "1.23" to get that in.

https://freecount.s10a.dev/?box=50470e168b046fdcf6a0b5bc71ef1bf8

@ssimono
Copy link
Owner

ssimono commented Mar 20, 2020

I see, I my Desktop Firefox also considers "1,23" as invalid. But the mobile versions seems to handle it, which is the main target.

I could do something fancy about it in the future (pointing that it is a decimal symbol issue, or even substituting on the fly) but for now I want to just rely on what is a valid/invalid input from the browser's point of view, without making assumption about the user intent.

@Wikinaut
Copy link
Author

It should work on both Firefoxes in the same way.

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