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

Provide consistent locale-specific number formatting #59

Open
julen opened this issue Oct 25, 2016 · 4 comments
Open

Provide consistent locale-specific number formatting #59

julen opened this issue Oct 25, 2016 · 4 comments
Labels

Comments

@julen
Copy link
Contributor

julen commented Oct 25, 2016

In some places .toLocaleString() has been introduced.

This method accepts a locale code as its first parameter (even an array of locales for matching). We must set this parameter to the UI locale in effect and probably provide a thin wrapper formatting function so that we get the same experience everywhere. Otherwise, the OS locale will always be used, and has high chances to mismatch Zing's UI locale.

@julen julen added the bug label Oct 25, 2016
julen referenced this issue Oct 25, 2016
This covers browser table only; as we migrate to React components,
let's make sure we consistently display numbers the same way.
@iafan
Copy link
Contributor

iafan commented Oct 25, 2016

Note that we don't want a server-side-based consistency. Numbers are formatted according to user current locale settings, and that's the desired behavior. We only need to display numbers consistently for a given user throughout the site (to make sure that all numbers we display are passed through .toLocaleString(). Same applies to dates and currencies.

@iafan
Copy link
Contributor

iafan commented Oct 25, 2016

Otherwise, the OS locale will always be used

While this is exactly the behavior I'd ideally like to get, it turns out using .toLocaleString() without parameters might default to en-US in some browsers. We could use navigator.language to determine the preferred language, but this is ultimately not the same thing as OS-wide preferred locale settings. Having said that, I'd prefer to keep things simple and use .toLocaleString() unless locale formatting preferences are implemented for the user (but I believe this is not worth the trouble).

One can play around with this jsfiddle to compare results across browsers.

@julen
Copy link
Contributor Author

julen commented Oct 31, 2016

For me a simple .toLocaleString() is what offers the worst results. It uses en-US both in Chrome and Firefox; Safari says it's using my own system locale, but the formatting doesn't match.

Chrome on the other hand doesn't seem to detect my preferred language in navigator.language. Firefox at least does, and displays the formatting as I would expect it.

I wouldn't consider a trouble implementing this as a user preference, but rather a need, as it's the most reliable way to get an experience that matches the user's expectations (same as with timezones). Zero configuration would be ideal for sure, but it doesn't provide acceptable results IMHO.

@iafan
Copy link
Contributor

iafan commented Nov 1, 2016

I don't think people switch browsers often, so the end result is not that bad — it is still consistent within a browser and more readable than with no formatting at all. I'm not saying that we shouldn't implement this preference (and this indeed seems like the only option can ultimately offer the best results), but I also have a feeling that even if we spend time implementing this setting, little to no people will bother to change it. So I'd make the zero configuration to work reasonably. If you want to make a wrapper function and use .toLocaleString(navigator.language) and if that provides slightly better results at least in some browsers, let's do this first.

julen added a commit that referenced this issue Nov 9, 2016
While this still doesn't offer a consistent and predictable experience due to
browser inconsistencies when it comes to `toLocaleString()` implementation, it
is the least bad option we can easily offer for the time being.

Refs. #59.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants