Use OverflowBar instead of ButtonBar in TimePicker (flutter#62601)

Pragya007 · Aug 5, 2020 · 8ec3b54 · 8ec3b54
1 parent 5238638
commit 8ec3b54
Show file tree

Hide file tree

Showing 2 changed files with 112 additions and 63 deletions.
diff --git a/packages/flutter/lib/src/material/time_picker.dart b/packages/flutter/lib/src/material/time_picker.dart
@@ -11,8 +11,6 @@ import 'package:flutter/rendering.dart';
 import 'package:flutter/services.dart';
 import 'package:flutter/widgets.dart';
 
-import 'button_bar.dart';
-import 'button_theme.dart';
 import 'color_scheme.dart';
 import 'colors.dart';
 import 'constants.dart';
@@ -1827,19 +1825,24 @@ class _TimePickerDialogState extends State<_TimePickerDialog> {
               : MaterialLocalizations.of(context).dialModeButtonLabel,
         ),
         Expanded(
-          // TODO(rami-a): Move away from ButtonBar to avoid https://github.com/flutter/flutter/issues/53378.
-          child: ButtonBar(
-            layoutBehavior: ButtonBarLayoutBehavior.constrained,
-            children: <Widget>[
-              TextButton(
-                onPressed: _handleCancel,
-                child: Text(widget.cancelText ?? localizations.cancelButtonLabel),
-              ),
-              TextButton(
-                onPressed: _handleOk,
-                child: Text(widget.confirmText ?? localizations.okButtonLabel),
-              ),
-            ],
+          child: Container(
+            alignment: AlignmentDirectional.centerEnd,
+            constraints: const BoxConstraints(minHeight: 52.0),
+            padding: const EdgeInsets.symmetric(horizontal: 8),
+            child: OverflowBar(
+              spacing: 8,
+              overflowAlignment: OverflowBarAlignment.end,
+              children: <Widget>[
+                TextButton(
+                  onPressed: _handleCancel,
+                  child: Text(widget.cancelText ?? localizations.cancelButtonLabel),
+                ),
+                TextButton(
+                  onPressed: _handleOk,
+                  child: Text(widget.confirmText ?? localizations.okButtonLabel),
+                ),
+              ],
+            ),
           ),
         ),
       ],

diff --git a/packages/flutter/test/material/time_picker_test.dart b/packages/flutter/test/material/time_picker_test.dart
@@ -643,54 +643,100 @@ void _tests() {
     expect(find.text(helperText), findsOneWidget);
   });
 
-  // TODO(rami-a): Re-enable and fix test.
-  testWidgets('text scale affects certain elements and not others',
-          (WidgetTester tester) async {
-        await mediaQueryBoilerplate(
-          tester,
-          false,
-          textScaleFactor: 1.0,
-          initialTime: const TimeOfDay(hour: 7, minute: 41),
-        );
-        await tester.tap(find.text('X'));
-        await tester.pumpAndSettle();
-
-        final double minutesDisplayHeight = tester.getSize(find.text('41')).height;
-        final double amHeight = tester.getSize(find.text('AM')).height;
-
-        await tester.tap(find.text('OK')); // dismiss the dialog
-        await tester.pumpAndSettle();
-
-        // Verify that the time display is not affected by text scale.
-        await mediaQueryBoilerplate(
-          tester,
-          false,
-          textScaleFactor: 2.0,
-          initialTime: const TimeOfDay(hour: 7, minute: 41),
-        );
-        await tester.tap(find.text('X'));
-        await tester.pumpAndSettle();
-
-        final double amHeight2x = tester.getSize(find.text('AM')).height;
-        expect(tester.getSize(find.text('41')).height, equals(minutesDisplayHeight));
-        expect(amHeight2x, greaterThanOrEqualTo(amHeight * 2));
-
-        await tester.tap(find.text('OK')); // dismiss the dialog
-        await tester.pumpAndSettle();
-
-        // Verify that text scale for AM/PM is at most 2x.
-        await mediaQueryBoilerplate(
-          tester,
-          false,
-          textScaleFactor: 3.0,
-          initialTime: const TimeOfDay(hour: 7, minute: 41),
-        );
-        await tester.tap(find.text('X'));
-        await tester.pumpAndSettle();
-
-        expect(tester.getSize(find.text('41')).height, equals(minutesDisplayHeight));
-        expect(tester.getSize(find.text('AM')).height, equals(amHeight2x));
-      });
+  testWidgets('OK Cancel button layout', (WidgetTester tester) async {
+    Widget buildFrame(TextDirection textDirection) {
+      return MaterialApp(
+        home: Material(
+          child: Center(
+            child: Builder(
+              builder: (BuildContext context) {
+                return ElevatedButton(
+                  child: const Text('X'),
+                  onPressed: () {
+                    showTimePicker(
+                      context: context,
+                      initialTime: const TimeOfDay(hour: 7, minute: 0),
+                      builder: (BuildContext context, Widget child) {
+                        return Directionality(
+                          textDirection: textDirection,
+                          child: child,
+                        );
+                      },
+                    );
+                  },
+                );
+              },
+            ),
+          ),
+        ),
+      );
+    }
+
+    await tester.pumpWidget(buildFrame(TextDirection.ltr));
+    await tester.tap(find.text('X'));
+    await tester.pumpAndSettle();
+    expect(tester.getBottomRight(find.text('OK')).dx, 638);
+    expect(tester.getBottomLeft(find.text('OK')).dx, 610);
+    expect(tester.getBottomRight(find.text('CANCEL')).dx, 576);
+    await tester.tap(find.text('OK'));
+    await tester.pumpAndSettle();
+
+    await tester.pumpWidget(buildFrame(TextDirection.rtl));
+    await tester.tap(find.text('X'));
+    await tester.pumpAndSettle();
+    expect(tester.getBottomLeft(find.text('OK')).dx, 162);
+    expect(tester.getBottomRight(find.text('OK')).dx, 190);
+    expect(tester.getBottomLeft(find.text('CANCEL')).dx, 224);
+    await tester.tap(find.text('OK'));
+    await tester.pumpAndSettle();
+  });
+
+  testWidgets('text scale affects certain elements and not others', (WidgetTester tester) async {
+    await mediaQueryBoilerplate(
+      tester,
+      false,
+      textScaleFactor: 1.0,
+      initialTime: const TimeOfDay(hour: 7, minute: 41),
+    );
+    await tester.tap(find.text('X'));
+    await tester.pumpAndSettle();
+
+    final double minutesDisplayHeight = tester.getSize(find.text('41')).height;
+    final double amHeight = tester.getSize(find.text('AM')).height;
+
+    await tester.tap(find.text('OK')); // dismiss the dialog
+    await tester.pumpAndSettle();
+
+    // Verify that the time display is not affected by text scale.
+    await mediaQueryBoilerplate(
+      tester,
+      false,
+      textScaleFactor: 2.0,
+      initialTime: const TimeOfDay(hour: 7, minute: 41),
+    );
+    await tester.tap(find.text('X'));
+    await tester.pumpAndSettle();
+
+    final double amHeight2x = tester.getSize(find.text('AM')).height;
+    expect(tester.getSize(find.text('41')).height, equals(minutesDisplayHeight));
+    expect(amHeight2x, greaterThanOrEqualTo(amHeight * 2));
+
+    await tester.tap(find.text('OK')); // dismiss the dialog
+    await tester.pumpAndSettle();
+
+    // Verify that text scale for AM/PM is at most 2x.
+    await mediaQueryBoilerplate(
+      tester,
+      false,
+      textScaleFactor: 3.0,
+      initialTime: const TimeOfDay(hour: 7, minute: 41),
+    );
+    await tester.tap(find.text('X'));
+    await tester.pumpAndSettle();
+
+    expect(tester.getSize(find.text('41')).height, equals(minutesDisplayHeight));
+    expect(tester.getSize(find.text('AM')).height, equals(amHeight2x));
+  });
 }
 
 void _testsInput() {