-
Notifications
You must be signed in to change notification settings - Fork 776
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
implement locale fallback for all locales #5718
base: master
Are you sure you want to change the base?
Conversation
Please review & provide me some feedback. The failing CI jobs are brittle and not related to any of the code changes! |
@andrewtelnov apologies for the brutal ping. Could you check if this is something you would like in the core? |
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.
Why do you remove "getCurrentStrings"?
It can be used by somebody to get strings object by locale.
Hello Sam, Thank you, |
In this implementation a string is picked based on the best matching language, so it is no longer the case that a single object can be found for a language. For example, the result of What is missing in the library is some documentation on what is considered part of the public API. I assumed this was not part of the public API and as it was no longer used internally I chose to remove it. Note that since |
@SamMousa I got an idea.
I will need to write a unit test on this functionality. Thank you, |
@SamMousa I am sorry, I have created my own PR. Here is the related issue. Thank you, |
Will do later today |
This implements language fallbacks in a more generic approach.
Instead of picking the locale string collection at the start, we try the locales and their fallbacks in order searching for a specific string.
The goal is to allow supporting custom locales more easily:
nl-something
, which is unknown would result in strings from thenl
translations.The idea is that instead of picking a whole locale dictionary at the start, we just define all possible locales.
Imagine this situation:
In the current version, requesting an unknown language will just use the
currentLocale
, and if that doesn't exist use thedefaultLocale
.This PR changes this behavior to be more granular. It will check each dictionary:
this.locales["nl-test"]
if found returnthis.locales["nl"]
if found returnthis.locales["de-DE"]
if found returnthis.locales["de"]
if found returnthis.locales["en-US"]
if found returnthis.locales["en"]
if found returnthis.onGetExternalString(...)
Marking as a draft so we can discuss the implementation. Tests still pass even though the implementation and behavior changed, this means that the tests were not thorough enough to detect this change in behavior.