Migrate time picker tests cases relying on localization from material_ui to flutter_localization#188753
Draft
elliette wants to merge 3 commits into
Draft
Migrate time picker tests cases relying on localization from material_ui to flutter_localization#188753elliette wants to merge 3 commits into
elliette wants to merge 3 commits into
Conversation
github-actions
Bot
added
framework
11 tasks
…_ui to flutter_localization
elliette
force-pushed
the
time-picker-localizations
branch
from
June 30, 2026 17:26
234b7ab to
7184892
Compare
elliette
commented
Jun 30, 2026
|
|
||
| // This file is run as part of a reduced test set in CI on Mac and Windows | ||
| // machines. | ||
| @Tags(<String>['reduced-test-set']) |
Member
Author
There was a problem hiding this comment.
It seems a bit weird to need to add this when the only golden test is currently skipped. I could also move that test case to a temporarily_disabled_test directory (like we had in cupertino_ui/material_ui) if that's preferred.
elliette
changed the title
elliette
marked this pull request as ready for review
June 30, 2026 18:42
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the mediaQueryBoilerplate helper in time_picker_test.dart to support custom locales and adds regression tests for Farsi 24-hour formatting, non-English separator alignment, and Korean day period spacing. The review feedback suggests extending mediaQueryBoilerplate with a textDirection parameter to properly support and test right-to-left (RTL) layouts, specifically for the new Farsi test case.
Comment on lines
659
to
+681
| WidgetTester tester, { | ||
| required bool alwaysUse24HourFormat, | ||
| required bool useMaterial3, | ||
| Locale locale = const Locale('en', 'US'), | ||
| }) async { | ||
| await tester.pumpWidget( | ||
| MaterialApp( | ||
| theme: ThemeData(useMaterial3: useMaterial3), | ||
| builder: (BuildContext context, Widget? child) { | ||
| return MediaQuery( | ||
| data: MediaQueryData(alwaysUse24HourFormat: alwaysUse24HourFormat), | ||
| child: child!, | ||
| ); | ||
| }, | ||
| home: Material( | ||
| child: Directionality( | ||
| textDirection: TextDirection.ltr, | ||
| child: Navigator( | ||
| onGenerateRoute: (RouteSettings settings) { | ||
| return MaterialPageRoute<void>( | ||
| builder: (BuildContext context) { | ||
| return TextButton( | ||
| onPressed: () { | ||
| showTimePicker( | ||
| context: context, | ||
| initialTime: const TimeOfDay(hour: 7, minute: 0), | ||
| Theme( | ||
| data: ThemeData(useMaterial3: useMaterial3), | ||
| child: Localizations( | ||
| locale: locale, | ||
| delegates: const <LocalizationsDelegate<dynamic>>[ | ||
| GlobalMaterialLocalizations.delegate, | ||
| GlobalWidgetsLocalizations.delegate, | ||
| ], | ||
| child: MediaQuery( | ||
| data: MediaQueryData( | ||
| alwaysUse24HourFormat: alwaysUse24HourFormat, | ||
| size: tester.view.physicalSize / tester.view.devicePixelRatio, | ||
| ), | ||
| child: Material( | ||
| child: Directionality( | ||
| textDirection: TextDirection.ltr, | ||
| child: Navigator( |
Contributor
There was a problem hiding this comment.
The mediaQueryBoilerplate helper currently hardcodes textDirection: TextDirection.ltr. Since this helper is now being used for localized tests (including RTL locales like Farsi fa_IR), hardcoding LTR prevents the time picker from being tested in its actual RTL layout.
To fix this, we should add an optional textDirection parameter to mediaQueryBoilerplate and pass it to the Directionality widget.
WidgetTester tester, {
required bool alwaysUse24HourFormat,
required bool useMaterial3,
Locale locale = const Locale('en', 'US'),
TextDirection textDirection = TextDirection.ltr,
}) async {
await tester.pumpWidget(
Theme(
data: ThemeData(useMaterial3: useMaterial3),
child: Localizations(
locale: locale,
delegates: const <LocalizationsDelegate<dynamic>>[
GlobalMaterialLocalizations.delegate,
GlobalWidgetsLocalizations.delegate,
],
child: MediaQuery(
data: MediaQueryData(
alwaysUse24HourFormat: alwaysUse24HourFormat,
size: tester.view.physicalSize / tester.view.devicePixelRatio,
),
child: Material(
child: Directionality(
textDirection: textDirection,
child: Navigator(
Comment on lines
+873
to
+878
| await mediaQueryBoilerplate( | ||
| tester, | ||
| locale: const Locale('fa', 'IR'), | ||
| useMaterial3: true, | ||
| alwaysUse24HourFormat: true, | ||
| ); |
Contributor
There was a problem hiding this comment.
Pass textDirection: TextDirection.rtl to mediaQueryBoilerplate so that the Farsi time picker test runs with the correct RTL layout.
await mediaQueryBoilerplate(
tester,
locale: const Locale('fa', 'IR'),
textDirection: TextDirection.rtl,
useMaterial3: true,
alwaysUse24HourFormat: true,
);
auto-submit Bot
pushed a commit
to flutter/packages
that referenced
this pull request
Jun 30, 2026
Port over `time_picker_test` from flutter/flutter#184279 The test cases that depend on `flutter_localizations` are being moved to `flutter_localizations` in PR flutter/flutter#188753, see flutter/flutter#188757 for rationale. Work towards flutter/flutter#182636 and flutter/flutter#188395 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
Piinks
marked this pull request as draft
July 15, 2026 21:15
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Work towards #188757
The added test cases in this PR were split out of flutter/packages#12061. To avoid a circular dependency in
material_ui, we are moving any test cases that depend onflutter_localizationsout of thematerial_uipackage and intoflutter_localizations.Note: One of those test cases is currently skipped, since it relies on golden testing infrastructure that is not currently enabled for
flutter_localizations. I've filed #188758 to track re-enabling it.