Language-specific LocalizationDelegates #12645
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hixie
reviewed
Oct 20, 2017
| @@ -188,6 +188,9 @@ class _MaterialLocalizationsDelegate extends LocalizationsDelegate<MaterialLocal | |||
| const _MaterialLocalizationsDelegate(); | |||
|
|
|||
| @override | |||
| bool isSupported(Locale locale) => locale.languageCode == 'en'; | |||
Hixie
reviewed
Oct 20, 2017
| final Set<Type> types = new Set<Type>(); | ||
| final List<LocalizationsDelegate<dynamic>> delegates = <LocalizationsDelegate<dynamic>>[]; | ||
| for (LocalizationsDelegate<dynamic> delegate in allDelegates) { | ||
| if (!types.contains(delegate.type)) { | ||
| if (delegate.isSupported(locale) && !types.contains(delegate.type)) { |
There was a problem hiding this comment.
i would flip the order in which you make these calls, so that we don't contact the delegate if we don't need to.
Hixie
reviewed
Oct 20, 2017
| @@ -97,6 +99,12 @@ abstract class LocalizationsDelegate<T> { | |||
| /// const constructors so that they can be used in const expressions. | |||
| const LocalizationsDelegate(); | |||
|
|
|||
| /// Return true if the instance of 'T' loaded by this delegate's [load] | |||
| /// method supports the given `locale`'s language. | |||
| bool isSupported(Locale locale) => true; | |||
There was a problem hiding this comment.
maybe prefix with something like:
/// Whether resources for the given locale can be loaded by this delegate.
///
Also either use backticks or no quotes at all around T. Single quotes makes it look like a string.
Hixie
reviewed
Oct 20, 2017
| /// method supports the given `locale`'s language. | ||
| bool isSupported(Locale locale) => true; | ||
| // TODO(hansmuller): remove the default implementation after existing | ||
| // apps have had a chance to update. |
There was a problem hiding this comment.
unless you mean that you plan to remove it in a couple of days, i would remove it now. the longer we leave this the more likely it is we'll have people depending on it.
Hixie
reviewed
Oct 20, 2017
| // This is convenient simplification. It would be more correct test if locale's | ||
| // text-direction is LTR. | ||
| @override | ||
| bool isSupported(Locale locale) => true; |
There was a problem hiding this comment.
this'll get resolved once we have the concrete classes, right?
Hixie
reviewed
Oct 20, 2017
| @@ -68,6 +68,9 @@ class _WidgetsLocalizationsDelegate extends LocalizationsDelegate<WidgetsLocaliz | |||
| const _WidgetsLocalizationsDelegate(); | |||
|
|
|||
| @override | |||
| bool isSupported(Locale locale) => true; | |||
|
|
HansMuller
force-pushed
the
localization_delegate_supports
branch
from
718cc44
to
0bc7771
Compare
HansMuller
force-pushed
the
localization_delegate_supports
branch
from
0bc7771
to
72151fb
Compare
This was referenced Dec 19, 2017
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make it easier to add language-specific versions of MaterialLocalizations.
Apps can define their own implementations of MaterialLocalizations that support languages which are not supported by the flutter_localizations package.
When a Flutter app starts or switches locales it will now select the first LocalizationDelegate for each LocalizationDelegate.type that supports the new locale. LocalizationDelegates must override their isSupported(Locale) method.
So to add material library support for a new language an app would define a subclass of MaterialLocalizations, say MyPolishMaterialLocalizations. The new Polish localizations would be loaded by a LocalizationsDelegate defined like this:
A Material app that included the new Polish localizations would be created like this: