Optional number elongation #18
Conversation
|
Hey! I did a quite significant refactoring of the internals so you probably have some conflicts. |
Heyla, if you agree with the concepts in the PR I don't mind fixing the conflicts sometime soon :) |
|
It's an interesting idea. I'd love to explore the implications a bit further! If some input value is being kept in sync in the DOM and JavaScript I'm not sure how this would affect that. For example in React it's common to use controlled inputs |
|
I rebased and noticed that 2 does not get superscripted The glyphs in strings of numbers are not as diverse as in text on average. 100, 100.000, 2019, etc will be more common. Doubling the even numbers is a good start but I'm worried about bias towards too much elongation (more than the ~30% on average we have for text) on average. Especially due to zero being common. I'm wondering if there are better numbers to pick based on frequency analysis of digits in a huge corpora of English text, like wikipedia for example. Anyway, I like the idea of opt-in elongation of numbers. Superscript seems fine but have you explored any other options for glyphs to use in the pseudo language for numbers? |
| @@ -126,30 +134,40 @@ const strategies = { | |||
| }; | |||
|
|
|||
| const psuedoLocalizeString = (string, options = { strategy: "accented" }) => { | |||
There was a problem hiding this comment.
There will be conflicts on this line after rebase, I think the best fix is
{ strategy = 'accented', elongateNumbers = false } = {}to keep deep defaults and avoid can not find elongateNumbers of undefined Errors in edge cases!
|
|
||
| let pseudoLocalizedText = ""; | ||
| for (let character of string) { | ||
| if (opts.map[character]) { | ||
| const convertedCharacter = strategyOptions.map[character] | ||
| const characterAsInt = parseInt(character) |
There was a problem hiding this comment.
I think we can move this inline to line 146.
!isNaN(parseInt(character))imo it's still totally legible and we avoid executing parseInt for every character if elongateNumbers number is false.
Now that I think about it do we need to parseInt?
would
options.elongateNumbers &&
EVEN_NUMBER_MAP[character]not be enough?
| @@ -126,30 +134,40 @@ const strategies = { | |||
| }; | |||
|
|
|||
| const psuedoLocalizeString = (string, options = { strategy: "accented" }) => { | |||
| let opts = strategies[options.strategy]; | |||
| let strategyOptions = strategies[options.strategy]; | |||
I agree that the current approach is rather naive, I can do some research at a later stage for the numbers - it would really depend on what kind of numbers we're dealing with to exercise the UI (I assume for Spotify, play counts will have a much wider range than a number of seconds, for instance - the same for our use case of review count vs distance from a certain place in km's). As mentioned, since our use case currencies are the most urgent, I think it's best if we approach those separately. It should be easier to get a more realistic average elongation based on USD or EUR for example. I'll look into a separate concept based on this branch for currencies and come back to a more realistic number example later based on our usage. |
Fix #15.
For now I decided to only elongate even numbers to avoid elongating the numbers too much, it might make sense to review this later.
Our main use case is currencies, but also to exercise the UI with longer numbers like the amount of reviews, for example.