-
-
Notifications
You must be signed in to change notification settings - Fork 101
Refactor Locale class and add LocaleProvider test #1283
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
base: welcome-screen
Are you sure you want to change the base?
Conversation
Changed the setLocale parameter in the Locale class to be nullable and updated its usage to safely invoke it. This allows for more flexible instantiation when a setLocale function is not required.
Replaces the call to locale.setLocale with locale.set in LocaleKtTest to match the updated API for changing the locale.
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.
looks good! i left a couple questions for my own learning and curiosity
val locale = java.util.Locale.getDefault() | ||
load(ClassLoader.getSystemResourceAsStream("PDE.properties")) | ||
load(ClassLoader.getSystemResourceAsStream("PDE_${locale.language}.properties") ?: InputStream.nullInputStream()) | ||
load(ClassLoader.getSystemResourceAsStream("PDE_${locale.toLanguageTag()}.properties") ?: InputStream.nullInputStream()) | ||
load(ClassLoader.getSystemResourceAsStream("PDE_${language}.properties") ?: InputStream.nullInputStream()) | ||
loadResourceUTF8("PDE.properties") | ||
loadResourceUTF8("PDE_${locale.language}.properties") | ||
loadResourceUTF8("PDE_${locale.toLanguageTag()}.properties") | ||
loadResourceUTF8("PDE_${language}.properties") |
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 so nice
CompositionLocalProvider(LocalLayoutDirection provides dir) { | ||
CompositionLocalProvider(LocalLocale provides locale) { | ||
content() | ||
} |
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.
Ah cool! So this is like how things are composable? People can pass in a block that will be wrapped by this Locale work
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.
Exactly, if you want access to localised strings you do
LocaleProvider {
val locale = LocalLocale.current
val localizedString = locale["someKey"]
}
And the beatiful things about these contexts is that if you want to display say something in Spanish within the English interface, you can override the LocalLocale
with a CompositionLocalProvider
. In that case its not super helpful but there's a lot of other cases where its very much is
As part of the Jetpack Compose UI, we would like to have a reactive component that we can use to read and set Processing's Locale live. This PR will improve on the existing functionality and adds testing.
Part of #1280