-
-
Notifications
You must be signed in to change notification settings - Fork 100
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
Improve localization logic #36
Conversation
@@ -15,13 +15,9 @@ struct Localization { | |||
"de": "Einstellungen", | |||
"el": "Προτιμήσεις", | |||
"en": "Preferences", | |||
"en-AU": "Preferences", | |||
"en-GB": "Preferences", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if these are the same, I still think we should keep them since they're coming from the system and for consistency between localization strings, as some string might differ.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I compared all these strings using a script to make sure they're 100% the same. In case the translation for various variants of English changes, they can be put back in just by adding them to the dictionary, but for now, there's no use in keeping them.
if locale.languageCode != nil { | ||
preferredLocale = locale | ||
break | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be simplified to:
let preferredLocale = Locale.preferredLanguages
.lazy
.map { Locale(identifier: $0) }
.first { $0.languageCode != nil }
?? Locale.current
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term "simplify" is very subjective in this context. 😄
I didn't do any performance comparisons, I just personally think my version is more readable.
Also, IIRC, locale.languageCode
was never nil in my tests, so in the overwhelming majority of cases, the loop should stop after just one iteration anyway.
} | ||
|
||
if let localizedString = localizedDict[languageCode] { | ||
return localizedString | ||
// Chinese is the only language where different region codes result in different translations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure about that? What are you basing it on? It feels weird hard-coding this. From experience, assumptions about date, time, and localizations are usually wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is based on comparing the dictionary's entries and checking which region code variants are actually different.
I admit it does feel weird to special case this, and I'd be happy to change it, but as long as Chinese is the only language where different region codes produce different translations for "Preferences", I'd prefer keeping it this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There must be some better system APIs we can use that handles the resolution here so we don't have to hard-code it like this. Good enough for now I guess, but I would want to fix this properly in the future.
Thanks :) 🙌 |
This should fix the issue where some language code/region code combinations would result in English text instead of the appropriate localization.
I noticed that most of the different region codes used the same localizations, so I removed the redundant copies. The only exceptions are the localizations for
zh-CN
,zh-HK
, andzh-TW
, because those are in fact different. To account for that,zh-*
locales are now special cased in the code.For all other language code/region code combinations, these changes should result in less error-prone localization, as the region code isn't considered at all anymore.
Additionally, the user's preferred languages are now checked for valid language codes before falling back to the system language.