Skip to content

[material_ui] Enable localized time_picker_test cases - #12391

Merged
auto-submit[bot] merged 1 commit into
flutter:mainfrom
elliette:reenable-localization-tests
Aug 7, 2026
Merged

[material_ui] Enable localized time_picker_test cases#12391
auto-submit[bot] merged 1 commit into
flutter:mainfrom
elliette:reenable-localization-tests

Conversation

@elliette

Copy link
Copy Markdown
Member

Follow up to #12061, #12119

Port over the remaining time_picker_test cases from flutter/flutter#184279 that were initially excluded in #12061 due to lack of localization support in material_ui.

Work towards flutter/flutter#182636 and flutter/flutter#188395

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. 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.

Footnotes

  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. 2

@flutter-dashboardflutter-dashboardBot added the CICD Run CI/CD label Aug 6, 2026
@elliette
elliette requested a review from justinmcAugust 6, 2026 23:44

@gemini-code-assistgemini-code-assistBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds regression and localization tests for the TimePicker widget, covering 24-hour Farsi formatting, centered separators in non-English locales, and spacing in locales using the 'a h:mm' pattern. It also updates the mediaQueryBoilerplate helper to support custom locales. Feedback recommends explicitly passing alwaysUse24HourFormat: true in the Farsi test to ensure robustness, and parameterizing the theme and golden file names in the separator alignment test to prevent test overwrites across different material types.

Comment on lines +242 to +246
await mediaQueryBoilerplate(
tester,
locale: const Locale('fa', 'IR'),
materialType: MaterialType.material3,
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The test is named 'Material3 - formats 24-hour numbers correctly in Farsi', but alwaysUse24HourFormat: true is not explicitly passed to mediaQueryBoilerplate. To ensure the test is robust and explicitly tests the 24-hour format regardless of any default locale settings, please pass alwaysUse24HourFormat: true.

awaitmediaQueryBoilerplate(
tester,
alwaysUse24HourFormat:true,
locale:constLocale('fa', 'IR'),
materialType:MaterialType.material3,
);

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid suggestion, though I'm guessing the Farsi locale always uses 24-hours anyways? The test case as it is matches the original in flutter/flutter: https://github.com/flutter/flutter/blob/1abf8f42d372614c02ad78d1a00f3720f05af74c/packages/flutter/test/material/time_picker_test.dart#L239

Comment on lines +1541 to +1568
testWidgets(
'TimePicker dialog displays centered separator between hour and minute inputs for non-english locale',
(WidgetTester tester) async {
tester.view.physicalSize = const Size(400, 800);
tester.view.devicePixelRatio = 1.0;
addTearDown(tester.view.reset);

await tester.pumpWidget(
const MaterialApp(
localizationsDelegates: GlobalMaterialLocalizations.delegates,
supportedLocales: <Locale>[Locale('en'), Locale('es')],
locale: Locale('es'),
home: Material(
child: TimePickerDialog(
initialTime: TimeOfDay(hour: 12, minute: 0),
initialEntryMode: TimePickerEntryMode.input,
),
),
),
);
await tester.pumpAndSettle();

await expectLater(
find.byType(Dialog),
matchesGoldenFile('time_picker.dialog.separator.alignment.non_english_locale.png'),
);
},
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test is defined inside the for (final MaterialType materialType in MaterialType.values) loop, meaning it runs twice (once for Material 2 and once for Material 3). However, it currently has two issues:

  1. It pumps a MaterialApp without specifying a theme, so it always uses the default theme (Material 3) in both runs, leaving Material 2 untested.
  2. Both runs use the exact same golden file name ('time_picker.dialog.separator.alignment.non_english_locale.png'), which causes them to overwrite each other and can lead to flaky or incorrect test results.

To fix this, configure the MaterialApp's theme using materialType and parameterize the golden file name using ${materialType.name}.

testWidgets(
'TimePicker dialog displays centered separator between hour and minute inputs for non-english locale',
(WidgetTester tester) async {
tester.view.physicalSize =constSize(400, 800);
tester.view.devicePixelRatio =1.0;
addTearDown(tester.view.reset);
await tester.pumpWidget(
MaterialApp(
theme:ThemeData(useMaterial3: materialType ==MaterialType.material3),
localizationsDelegates:GlobalMaterialLocalizations.delegates,
supportedLocales:const<Locale>[Locale('en'), Locale('es')],
locale:constLocale('es'),
home:constMaterial(
child:TimePickerDialog(
initialTime:TimeOfDay(hour:12, minute:0),
initialEntryMode:TimePickerEntryMode.input,
),
),
),
);
await tester.pumpAndSettle();
awaitexpectLater(
find.byType(Dialog),
matchesGoldenFile('${materialType.name}_time_picker.dialog.separator.alignment.non_english_locale.png'),
);
},
);

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actionsgithub-actionsBot added triage-framework Should be looked at in framework triage p: material_ui labels Aug 7, 2026

@justinmcjustinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@ellietteelliette added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 7, 2026
@auto-submit
auto-submitBot merged commit a0e27cc into flutter:mainAug 7, 2026
12 checks passed
pullBot pushed a commit to Budda0ne/flutter that referenced this pull request Aug 10, 2026
…r#190844)
flutter/packages@fc22143...1861b68
2026-08-09 engine-flutter-autoroll@skia.org Manual roll Flutter from
2757a77 to e52f01c (47 revisions) (flutter/packages#12401)
2026-08-08 engine-flutter-autoroll@skia.org Manual roll Flutter from
2a230d1 to 2757a77 (14 revisions) (flutter/packages#12393)
2026-08-07 21270878+elliette@users.noreply.github.com [material_ui]
Enable localized `time_picker_test` cases (flutter/packages#12391)
2026-08-07 jmccandless@google.com [material_ui] Main example
(flutter/packages#12336)
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com on the revert to ensure that a
human
is aware of the problem.
To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose
To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmitMerge PR when tree becomes green via auto submit AppCICDRun CI/CDp: material_uitriage-frameworkShould be looked at in framework triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@elliette@justinmc