From 17d317977b5221d900315945f6867d1941a86eab Mon Sep 17 00:00:00 2001 From: Hans Muller Date: Mon, 3 Aug 2020 14:26:43 -0700 Subject: [PATCH] Use OverflowBar instead of ButtonBar in TimePicker (#62601) --- .../flutter/lib/src/material/time_picker.dart | 33 +++-- .../test/material/time_picker_test.dart | 128 ++++++++++++------ 2 files changed, 105 insertions(+), 56 deletions(-) diff --git a/packages/flutter/lib/src/material/time_picker.dart b/packages/flutter/lib/src/material/time_picker.dart index 394fd372da..7dca396ada 100644 --- 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: [ - 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: [ + 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 index 68dbf897ba..c5b4d019f3 100644 --- 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(); + 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, + ); + }, + ); + }, + ); + }, + ), + ), + ), + ); + } - final double minutesDisplayHeight = tester.getSize(find.text('41')).height; - final double amHeight = tester.getSize(find.text('AM')).height; + 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.tap(find.text('OK')); // dismiss the dialog - 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(); + }); - // 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(); + 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 amHeight2x = tester.getSize(find.text('AM')).height; - expect(tester.getSize(find.text('41')).height, equals(minutesDisplayHeight)); - expect(amHeight2x, greaterThanOrEqualTo(amHeight * 2)); + 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(); + 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(); + // 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(); - expect(tester.getSize(find.text('41')).height, equals(minutesDisplayHeight)); - expect(tester.getSize(find.text('AM')).height, equals(amHeight2x)); - }); + 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() {