search

hibiyasleep / kagerou

ACT-FF14 OverlayPlugin Skin - Modern, easily configurable, always up-to-date.
Other
336 stars 96 forks source link

feat: Add thousands separator #188

Closed Sundava closed 2 years ago

Sundava commented 2 years ago

Implementation proposal for #187

Some help would be appreciated for localization.

Option menu:

image

Examples with and without -k suffix abbreviations:

image image

hibiyasleep commented 2 years ago

Locally merging & working, please wait a few hours or days. thank you for PR always!

in locale that , separates number, decimal point becomes . and vice versa; so upper += sep lowerEndsAt breaks this, however, I'm fixing this with some refactoring works.

honestly - I don't really like thousands separators (prefer space even required, by aforementioned reason) and it was why this wasn't implemented so long... however, this opinion isn't strong enough to reject incoming PRs,

hibiyasleep commented 2 years ago

I've made a draft branch (feature/1000-sep, b35b784 as of now), since no manual tests should be happen at 3AM; please review this if you have spear time or something. I'll continue this tomorrow.

Sundava commented 2 years ago

Looks good to me.

Side note, regarding the , or . for the decimal separator: If we want to be nitpicky, the French SI format uses spaces for thousands separator and comma for decimal point, while the English one use spaces for thousands but dot for decimal (See here) The clean way to implement it would be to with locale format presets (Or a decimal separator option), but that's outside the scope of this PR and I don't think it's worth the trouble

hibiyasleep commented 2 years ago

oh my god, I just hope no one wants of that two pixels... ok, then I'll soon merge this.