Add missing space between DayPeriodControl and time control in time picker (flutter#162230)

MinSeungHyun · web-flow · commit bd055cf9606a · 2025-02-04T22:37:11.000Z
Fixes flutter#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
diff --git 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: <Widget>[
-                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) ...<Widget>[
-                  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: <Widget>[
-                  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: <Widget>[
-                        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: <Widget>[
+                      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
@@ -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<void> 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 <LocalizationsDelegate<dynamic>>[
-          DefaultMaterialLocalizations.delegate,
-          DefaultWidgetsLocalizations.delegate,
+          GlobalMaterialLocalizations.delegate,
+          GlobalWidgetsLocalizations.delegate,
         ],
         child: MediaQuery(
           data: MediaQueryData(
