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

Decimal symbol detection is broken for decimal and percent styles #1

Closed
ragulka opened this issue Oct 1, 2021 · 2 comments
Closed

Comments

@ragulka
Copy link
Contributor

ragulka commented Oct 1, 2021

I understand this is a very-very early release of this package, but I wanted to try it out nonetheless, as it would be immensely helpful for a project I'm working on.

I noticed that it's currently impossible to use the decimal style with... decimals! 😅 As I ventured into the code, I also noticed that there is no type declaration for the percent style, which is supported by Intl.NumberFormat. Regardless of the missing type, I tried setting the type to percent and noticed that it does not work as expected, either.

Issue description and reproduction

Given the following, it's impossible to enter any decimals in the input:

new NumberInput({
    el: inputElementFromSomeWhere,
    options: {
        formatStyle: NumberFormatStyle.Decimal,
        precision: 4,
    },
})

Here's a bare-bones reproduction of the issue: https://codepen.io/ragulka/pen/NWgmqaE

  1. Notice that the input value is 123456.789 initially
  2. Click on the input - the value is formatted, but the decimal point is lost (value is converted to an integer)
  3. Try entering an amount with a decimal place - when hitting comma or period on the keyboard, nothing happens

I would expect to be able to enter at least 4 decimal places, since I've set precision: 4.

Tracking down the cause

As I tried to debug this, I noticed that the NumberFormat class tries to detect the decimal symbol and when it cannot find one, it'll set minimumFractionDigits and maximumFractionDigits to 0, regardless of the precision option.

The culprit seem to be these lines. In order to detect the decimal symbol (as well as other symbols, like the negative, etc), a number is formatted with the given style. Then, it tries to see if there are any decimal places by checking if there are any zeroes in the formatted number. Since there are no zeroes in the input number (123456), having zeroes would mean there's a decimal point right where the first zero is inserted.

The logic above assumes that there are decimal places (minimumFractionDigits > 0) in the default format for the given style. This is only true for currency, though. While decimal places are perfectly valid for decimals and percentages, decimals and percentages do not have a minimum amount of decimals by default. Formatting the number 123456 as decimal with will result in 123,456 - not 123,456.00. Also formatting 1 as percent will yield 100% which will throw off the decimal detection completely.

In conclusion, it seems the decimal symbol detection is flawed - while it works for currencies (by lucky chance), it's not really suitable for other numbers.

Proposed change

There are more reliable ways to get the decimal and thousand separators.

If IE11 is not a concern (Vue 3 does not support it), I'd suggest simply using NumberFormat#formatToParts.

var formatter = new Intl.NumberFormat('de-DE', { style: 'decimal' });

formatter.formatToParts(123456.789);

// return value:
[
  { type: "integer",  value: "123"   },
  { type: "group",    value: "."   },
  { type: "integer",  value: "456" },
  { type: "decimal",  value: ","   },
  { type: "fraction", value: "789"  }
]

However, if IE11 is a thing, it's possible to use the other solution in the linked SO answer.

In any case, it's imperative to use a number with existing decimal places as input for the formatter, regardless of the chosen solution.

EDIT: I have a proof-of-concept implementation in my fork here - it's raw, but it shows the concept and it works as expected as far as I tried.

@dm4t2
Copy link
Owner

dm4t2 commented Oct 6, 2021

Hi, thanks for reporting and your suggestions 👍
As you probably already noticed the current source is an adaption of Vue Currency Input, which targets the currency format style.
The decimal format style issues will be fixed shortly with the upcoming official release. As well the percent format style will be fully supported.

dm4t2 added a commit that referenced this issue Oct 16, 2021
@dm4t2
Copy link
Owner

dm4t2 commented Oct 16, 2021

Fixed in v1.0.0 🎉

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