-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Localize numbers server-side (maintainers' opinion requested) #21770
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
Conversation
Signed-off-by: Yarden Shoham <[email protected]>
Signed-off-by: Yarden Shoham <[email protected]>
Signed-off-by: Yarden Shoham <[email protected]>
I don't think we can drop Hmm, I don't think that is a strong enough argument not to push the work to the frontend. |
Thanks @delvh, looks like a vote for option 3 |
The frontend language should not always follow web browser, it's a default option. And users could set their prefer language which maybe different from web browser. And for a more complicated example, users' prefer location and language should become two options, but I think we currently can hold only one. |
I think that's a frontend concern and should be in JS code ... |
Option 3 decided then, thank you all |
Continued in #21786 |
still thanks ❤️ |
This one started when I encountered #12637. I wrote
RenderNumber
in b9319f9 to format the number of commits, then I thought "Why not format all other numbers in a nice way?". So I did it in c929ad2.While running a search for numbers in templates I encountered
JsPrettyNumber
andCountFmt
.JsPrettyNumber
seems to localize the numbers client-side (therefore with a "flash of unstyled content") andCountFmt
does it server-side disregarding the user's locale. So this PR can go a few ways:RenderNumber
+ use it in the places of c929ad2 (current PR state)RenderNumber
and dropJsPrettyNumber
andCountFmt
JsPrettyNumber
for number of commitsWhat do you prefer?