globalizejs / globalize

A JavaScript library for internationalization and localization that leverages the official Unicode CLDR JSON data
https://globalizejs.com
MIT License
4.8k stars 605 forks source link

Number: Compact formatting with significat digits fix #830

Closed shivijais closed 6 years ago

shivijais commented 6 years ago

Fixes: #821

Removed the logic of rounding the number twice - with a precision of {actual precision required + 2} and then again with the actual precision. This was causing wrong rounding off a number. For eg.

Globalize.numberFormatter({
        minimumSignificantDigits: 1,
        maximumSignificantDigits: 3,
    })(12.849872883)

Here the number 12.849872883 is first converted to 12.850 and then when finally rounded off becomes 12.9. But Ideally this should be formatted to 12.8.

shivijais commented 6 years ago

@rxaviers Can you please take a look at this PR. The travis-ci is failing on timeout issue with https://github.com/SlexAxton/messageformat which is now moved to https://github.com/messageformat/messageformat

rxaviers commented 6 years ago

Thank you so far

rxaviers commented 6 years ago

Thanks for the quick fix. Tests are passing, build is failing simply because of commit message which I can fix.

Having said that, I want to be sure the code simplification in the round function doesn't cause any side effect and therefore I plan to review this again. It's on my end, I hope to do it soon and will update this PR.

shivijais commented 6 years ago

@rxaviers any update on this?

rxaviers commented 6 years ago

Thank you! Released as 1.4.0-alpha.3