-
Notifications
You must be signed in to change notification settings - Fork 6k
Provide default locales when on platforms without locale passing support. #6673
Conversation
| // support passing platform locales to Flutter. This will allow tests on | ||
| // these platforms to have similar expected behavior as Android and iOS where | ||
| // Locale passing is supported. | ||
| if ((_locales == null || _locales.isEmpty) && (Platform.isLinux || Platform.isMacOS || Platform.isFuchsia)) { |
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.
Instead of a hard-coded list of platforms, I recommend using the FLUTTER_TEST environment variable (and only doing it in debug builds). See foundation/platform.dart in the framework repo.
| if ((_locales == null || _locales.isEmpty) && (Platform.isLinux || Platform.isMacOS || Platform.isFuchsia)) { | ||
| // TODO(garyq): As we add support for passing locales on additional platforms, | ||
| // return null for newly supported platforms. | ||
| _locales = <Locale>[Locale("en", "US"), Locale("zh", "CN")]; |
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 Chinese?
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.
it's probably better to just not have any special case logic here and just rely on the framework to handle the defaulting.
| if ((_locales == null || _locales.isEmpty) && (Platform.isLinux || Platform.isMacOS || Platform.isFuchsia)) { | ||
| // TODO(garyq): As we add support for passing locales on additional platforms, | ||
| // return null for newly supported platforms. | ||
| _locales = <Locale>[Locale("en", "US"), Locale("zh", "CN")]; |
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.
Can these default locales be set during startup of flutter_tester?
This would ensure that the default locales aren't used on any platform that isn't a test
|
As Jason suggested, I think I'll inject the locales in during flutter tester startup. This PR can be closed. Stay tuned for a follow up PR with that. That way this test-only code can stay separate from the actual code. |
|
New version: #6689 |
As encountered by @jason-simmons, platforms that do not support locale passing currently do not initialize the locale variables in dart:ui Window. This leads to inconsistent null expectation behavior on production environments vs test environments.
This provides a list of minimal default preferred locales to window when the platform in question does not support passing the device locale up to flutter. Test environments can now expect to not obtain null locales after the app is running just as production apps can.
Tests are on the framework-side PR: flutter/flutter#23596