From bd055cf9606a9aff55b0ae87a74f90da1be171ef Mon Sep 17 00:00:00 2001 From: Seunghyun Min Date: Wed, 5 Feb 2025 07:37:11 +0900 Subject: [PATCH] Add missing space between DayPeriodControl and time control in time picker (#162230) Fixes #162229 | Before | After | |--------|--------| | ![Simulator Screenshot - iPhone 14 Pro - 2025-01-26 at 22 01 55](https://github.com/user-attachments/assets/99034ccf-7006-44d9-9642-3ceb1ffd4fb5) ![Simulator Screenshot - iPhone 14 Pro - 2025-01-27 at 16 17 16](https://github.com/user-attachments/assets/93b233ec-e66f-4d06-bdd6-474a841dc1a8) | ![Simulator Screenshot - iPhone 14 Pro - 2025-01-26 at 21 59 04](https://github.com/user-attachments/assets/319e6c83-c63a-4415-8d19-62359b0a93a0) ![Simulator Screenshot - iPhone 14 Pro - 2025-01-27 at 16 17 42](https://github.com/user-attachments/assets/4753b5c6-46f5-4b21-80ef-bc568364198e) | ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- .../flutter/lib/src/material/time_picker.dart | 47 +++++++-------- .../test/material/time_picker_test.dart | 58 ++++++++++++++++++- 2 files changed, 75 insertions(+), 30 deletions(-) diff --git a/packages/flutter/lib/src/material/time_picker.dart b/packages/flutter/lib/src/material/time_picker.dart index 78fd6f702ff..e3472a14bb6 100644 --- a/packages/flutter/lib/src/material/time_picker.dart +++ b/packages/flutter/lib/src/material/time_picker.dart @@ -267,10 +267,12 @@ class _TimePickerHeader extends StatelessWidget { ), ), Row( + textDirection: + timeOfDayFormat == TimeOfDayFormat.a_space_h_colon_mm + ? TextDirection.rtl + : TextDirection.ltr, + spacing: 12, children: [ - if (hourDialType == _HourDialType.twelveHour && - timeOfDayFormat == TimeOfDayFormat.a_space_h_colon_mm) - const _DayPeriodControl(), Expanded( child: Row( // Hour/minutes should not change positions in RTL locales. @@ -282,11 +284,7 @@ class _TimePickerHeader extends StatelessWidget { ], ), ), - if (hourDialType == _HourDialType.twelveHour && - timeOfDayFormat != TimeOfDayFormat.a_space_h_colon_mm) ...[ - const SizedBox(width: 12), - const _DayPeriodControl(), - ], + if (hourDialType == _HourDialType.twelveHour) const _DayPeriodControl(), ], ), ], @@ -303,29 +301,24 @@ class _TimePickerHeader extends StatelessWidget { _TimePickerModel.defaultThemeOf(context).helpTextStyle, ), Column( + verticalDirection: + timeOfDayFormat == TimeOfDayFormat.a_space_h_colon_mm + ? VerticalDirection.up + : VerticalDirection.down, mainAxisAlignment: MainAxisAlignment.center, crossAxisAlignment: CrossAxisAlignment.start, + spacing: 12, children: [ - if (hourDialType == _HourDialType.twelveHour && - timeOfDayFormat == TimeOfDayFormat.a_space_h_colon_mm) - const _DayPeriodControl(), - Padding( - padding: EdgeInsets.only( - bottom: hourDialType == _HourDialType.twelveHour ? 12 : 0, - ), - child: Row( - // Hour/minutes should not change positions in RTL locales. - textDirection: TextDirection.ltr, - children: [ - const Expanded(child: _HourControl()), - _TimeSelectorSeparator(timeOfDayFormat: timeOfDayFormat), - const Expanded(child: _MinuteControl()), - ], - ), + Row( + // Hour/minutes should not change positions in RTL locales. + textDirection: TextDirection.ltr, + children: [ + const Expanded(child: _HourControl()), + _TimeSelectorSeparator(timeOfDayFormat: timeOfDayFormat), + const Expanded(child: _MinuteControl()), + ], ), - if (hourDialType == _HourDialType.twelveHour && - timeOfDayFormat != TimeOfDayFormat.a_space_h_colon_mm) - const _DayPeriodControl(), + if (hourDialType == _HourDialType.twelveHour) const _DayPeriodControl(), ], ), ], diff --git a/packages/flutter/test/material/time_picker_test.dart b/packages/flutter/test/material/time_picker_test.dart index 2b1e755bfda..bf694a89367 100644 --- a/packages/flutter/test/material/time_picker_test.dart +++ b/packages/flutter/test/material/time_picker_test.dart @@ -10,6 +10,7 @@ import 'dart:ui'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; +import 'package:flutter_localizations/flutter_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; import '../widgets/feedback_tester.dart'; @@ -2162,6 +2163,56 @@ void main() { expect(tester.getSize(findBorderPainter().first), const Size(96.0, 70.0)); }); + + // Regression test for https://github.com/flutter/flutter/issues/162229. + testWidgets( + 'Time picker spacing between time control and day period control for locales using "a h:mm" pattern', + (WidgetTester tester) async { + addTearDown(tester.view.reset); + + final Finder dayPeriodControlFinder = find.byWidgetPredicate( + (Widget w) => '${w.runtimeType}' == '_DayPeriodControl', + ); + final Finder timeControlFinder = + find.ancestor(of: find.text('7'), matching: find.byType(Row)).first; + + // Render in portrait mode. + tester.view.physicalSize = const Size(800, 800.5); + tester.view.devicePixelRatio = 1; + await mediaQueryBoilerplate( + tester, + materialType: MaterialType.material3, + locale: const Locale('ko', 'KR'), + ); + + expect( + tester.getBottomLeft(timeControlFinder).dx - + tester.getBottomRight(dayPeriodControlFinder).dx, + 12, + ); + + // Dismiss the dialog. + final MaterialLocalizations materialLocalizations = MaterialLocalizations.of( + tester.element(find.byType(TextButton).first), + ); + await tester.tap(find.text(materialLocalizations.okButtonLabel)); + await tester.pumpAndSettle(); + + // Render in landscape mode. + tester.view.physicalSize = const Size(800.5, 800); + tester.view.devicePixelRatio = 1; + await mediaQueryBoilerplate( + tester, + materialType: MaterialType.material3, + locale: const Locale('ko', 'KR'), + ); + + expect( + tester.getTopLeft(timeControlFinder).dy - tester.getBottomLeft(dayPeriodControlFinder).dy, + 12, + ); + }, + ); } final Finder findDialPaint = find.descendant( @@ -2204,15 +2255,16 @@ Future mediaQueryBoilerplate( bool tapButton = true, required MaterialType materialType, Orientation? orientation, + Locale locale = const Locale('en', 'US'), }) async { await tester.pumpWidget( Theme( data: ThemeData(useMaterial3: materialType == MaterialType.material3), child: Localizations( - locale: const Locale('en', 'US'), + locale: locale, delegates: const >[ - DefaultMaterialLocalizations.delegate, - DefaultWidgetsLocalizations.delegate, + GlobalMaterialLocalizations.delegate, + GlobalWidgetsLocalizations.delegate, ], child: MediaQuery( data: MediaQueryData(