From 017997b9c142225e91c2c6afa7afa587c78e9ac6 Mon Sep 17 00:00:00 2001 From: Darren Austin Date: Mon, 6 May 2019 10:27:16 -0700 Subject: [PATCH] Increase size of touch regions in the Time Picker header (#32053) - Increased the AM/PM, minute and hour buttons to at least 48x48 - Added InkWells to all of them - Adjusted the landscape layout for the AM/PM buttons to be horizontal - Added a test to ensure the regions are at least 48x48 --- .../flutter/lib/src/material/time_picker.dart | 200 +++++++++++------- .../test/material/time_picker_test.dart | 32 +++ 2 files changed, 153 insertions(+), 79 deletions(-) diff --git a/packages/flutter/lib/src/material/time_picker.dart b/packages/flutter/lib/src/material/time_picker.dart index 05d9c7d7d4..b3834a8379 100644 --- a/packages/flutter/lib/src/material/time_picker.dart +++ b/packages/flutter/lib/src/material/time_picker.dart @@ -16,6 +16,8 @@ import 'debug.dart'; import 'dialog.dart'; import 'feedback.dart'; import 'flat_button.dart'; +import 'ink_well.dart'; +import 'material.dart'; import 'material_localizations.dart'; import 'text_theme.dart'; import 'theme.dart'; @@ -44,15 +46,7 @@ const double _kTimePickerHeightLandscape = 316.0; const double _kTimePickerHeightPortraitCollapsed = 484.0; const double _kTimePickerHeightLandscapeCollapsed = 304.0; -/// The horizontal gap between the day period fragment and the fragment -/// positioned next to it horizontally. -/// -/// Normally there's only one horizontal sibling, and it may appear on the left -/// or right depending on the current [TextDirection]. -const double _kPeriodGap = 8.0; - -/// The vertical gap between pieces when laid out vertically (in portrait mode). -const double _kVerticalGap = 8.0; +const BoxConstraints _kMinTappableRegion = BoxConstraints(minWidth: 48, minHeight: 48); enum _TimePickerHeaderId { hour, @@ -194,9 +188,11 @@ class _TimePickerHeaderFormat { class _DayPeriodControl extends StatelessWidget { const _DayPeriodControl({ @required this.fragmentContext, + @required this.orientation, }); final _TimePickerFragmentContext fragmentContext; + final Orientation orientation; void _togglePeriod() { final int newHour = (fragmentContext.selectedTime.hour + TimeOfDay.hoursPerPeriod) % TimeOfDay.hoursPerDay; @@ -238,41 +234,73 @@ class _DayPeriodControl extends StatelessWidget { final TextStyle pmStyle = headerTextTheme.subhead.copyWith( color: !amSelected ? activeColor: inactiveColor ); + final bool layoutPortrait = orientation == Orientation.portrait; - return Column( - mainAxisSize: MainAxisSize.min, - children: [ - GestureDetector( - excludeFromSemantics: true, - onTap: Feedback.wrapForTap(() { - _setAm(context); - }, context), - behavior: HitTestBehavior.opaque, - child: Semantics( - selected: amSelected, - onTap: () { - _setAm(context); - }, - child: Text(materialLocalizations.anteMeridiemAbbreviation, style: amStyle), + final Widget amButton = ConstrainedBox( + constraints: _kMinTappableRegion, + child: Material( + type: MaterialType.transparency, + child: InkWell( + onTap: Feedback.wrapForTap(() => _setAm(context), context), + child: Padding( + padding: layoutPortrait ? const EdgeInsets.only(bottom: 2.0) : const EdgeInsets.only(right: 4.0), + child: Align( + alignment: layoutPortrait ? Alignment.bottomCenter : Alignment.centerRight, + widthFactor: 1, + heightFactor: 1, + child: Semantics( + selected: amSelected, + child: Text(materialLocalizations.anteMeridiemAbbreviation, style: amStyle) + ), + ), ), ), - const SizedBox(width: 0.0, height: 4.0), // Vertical spacer - GestureDetector( - excludeFromSemantics: true, - onTap: Feedback.wrapForTap(() { - _setPm(context); - }, context), - behavior: HitTestBehavior.opaque, - child: Semantics( - selected: !amSelected, - onTap: () { - _setPm(context); - }, - child: Text(materialLocalizations.postMeridiemAbbreviation, style: pmStyle), - ), - ), - ], + ), ); + + final Widget pmButton = ConstrainedBox( + constraints: _kMinTappableRegion, + child: Material( + type: MaterialType.transparency, + textStyle: pmStyle, + child: InkWell( + onTap: Feedback.wrapForTap(() => _setPm(context), context), + child: Padding( + padding: layoutPortrait ? const EdgeInsets.only(top: 2.0) : const EdgeInsets.only(left: 4.0), + child: Align( + alignment: orientation == Orientation.portrait ? Alignment.topCenter : Alignment.centerLeft, + widthFactor: 1, + heightFactor: 1, + child: Semantics( + selected: !amSelected, + child: Text(materialLocalizations.postMeridiemAbbreviation, style: pmStyle), + ), + ), + ), + ), + ), + ); + + switch (orientation) { + case Orientation.portrait: + return Column( + mainAxisSize: MainAxisSize.min, + children: [ + amButton, + pmButton, + ], + ); + + case Orientation.landscape: + return Row( + mainAxisSize: MainAxisSize.min, + children: [ + amButton, + pmButton, + ], + ); + } + return null; } } @@ -326,22 +354,28 @@ class _HourControl extends StatelessWidget { alwaysUse24HourFormat: alwaysUse24HourFormat, ); - return GestureDetector( - onTap: Feedback.wrapForTap(() => fragmentContext.onModeChange(_TimePickerMode.hour), context), - child: Semantics( - hint: localizations.timePickerHourModeAnnouncement, - value: formattedHour, - excludeSemantics: true, - increasedValue: formattedNextHour, - onIncrease: () { - fragmentContext.onTimeChange(nextHour); - }, - decreasedValue: formattedPreviousHour, - onDecrease: () { - fragmentContext.onTimeChange(previousHour); - }, - child: Text(formattedHour, style: hourStyle), + return Semantics( + hint: localizations.timePickerHourModeAnnouncement, + value: formattedHour, + excludeSemantics: true, + increasedValue: formattedNextHour, + onIncrease: () { + fragmentContext.onTimeChange(nextHour); + }, + decreasedValue: formattedPreviousHour, + onDecrease: () { + fragmentContext.onTimeChange(previousHour); + }, + child: ConstrainedBox( + constraints: _kMinTappableRegion, + child: Material( + type: MaterialType.transparency, + child: InkWell( + onTap: Feedback.wrapForTap(() => fragmentContext.onModeChange(_TimePickerMode.hour), context), + child: Text(formattedHour, style: hourStyle, textAlign: TextAlign.end), + ), ), + ), ); } } @@ -390,22 +424,28 @@ class _MinuteControl extends StatelessWidget { ); final String formattedPreviousMinute = localizations.formatMinute(previousMinute); - return GestureDetector( - onTap: Feedback.wrapForTap(() => fragmentContext.onModeChange(_TimePickerMode.minute), context), - child: Semantics( - excludeSemantics: true, - hint: localizations.timePickerMinuteModeAnnouncement, - value: formattedMinute, - increasedValue: formattedNextMinute, - onIncrease: () { - fragmentContext.onTimeChange(nextMinute); - }, - decreasedValue: formattedPreviousMinute, - onDecrease: () { - fragmentContext.onTimeChange(previousMinute); - }, - child: Text(formattedMinute, style: minuteStyle), + return Semantics( + excludeSemantics: true, + hint: localizations.timePickerMinuteModeAnnouncement, + value: formattedMinute, + increasedValue: formattedNextMinute, + onIncrease: () { + fragmentContext.onTimeChange(nextMinute); + }, + decreasedValue: formattedPreviousMinute, + onDecrease: () { + fragmentContext.onTimeChange(previousMinute); + }, + child: ConstrainedBox( + constraints: _kMinTappableRegion, + child: Material( + type: MaterialType.transparency, + child: InkWell( + onTap: Feedback.wrapForTap(() => fragmentContext.onModeChange(_TimePickerMode.minute), context), + child: Text(formattedMinute, style: minuteStyle, textAlign: TextAlign.start), + ), ), + ), ); } } @@ -415,13 +455,17 @@ class _MinuteControl extends StatelessWidget { /// configuration. /// /// The [timeOfDayFormat] and [context] arguments must not be null. -_TimePickerHeaderFormat _buildHeaderFormat(TimeOfDayFormat timeOfDayFormat, _TimePickerFragmentContext context) { +_TimePickerHeaderFormat _buildHeaderFormat( + TimeOfDayFormat timeOfDayFormat, + _TimePickerFragmentContext context, + Orientation orientation +) { + // Creates an hour fragment. _TimePickerHeaderFragment hour() { return _TimePickerHeaderFragment( layoutId: _TimePickerHeaderId.hour, widget: _HourControl(fragmentContext: context), - startMargin: _kPeriodGap, ); } @@ -448,8 +492,7 @@ _TimePickerHeaderFormat _buildHeaderFormat(TimeOfDayFormat timeOfDayFormat, _Tim _TimePickerHeaderFragment dayPeriod() { return _TimePickerHeaderFragment( layoutId: _TimePickerHeaderId.period, - widget: _DayPeriodControl(fragmentContext: context), - startMargin: _kPeriodGap, + widget: _DayPeriodControl(fragmentContext: context, orientation: orientation), ); } @@ -508,7 +551,6 @@ _TimePickerHeaderFormat _buildHeaderFormat(TimeOfDayFormat timeOfDayFormat, _Tim fragment3: minute(), ), piece( - bottomMargin: _kVerticalGap, fragment1: dayPeriod(), ), ); @@ -529,7 +571,6 @@ _TimePickerHeaderFormat _buildHeaderFormat(TimeOfDayFormat timeOfDayFormat, _Tim case TimeOfDayFormat.a_space_h_colon_mm: return format( piece( - bottomMargin: _kVerticalGap, fragment1: dayPeriod(), ), piece( @@ -621,10 +662,11 @@ class _TimePickerHeaderLayout extends MultiChildLayoutDelegate { final _TimePickerHeaderPiece centrepiece = format.pieces[format.centrepieceIndex]; double y = (size.height - height) / 2.0; for (int pieceIndex = 0; pieceIndex < format.pieces.length; pieceIndex += 1) { + final double pieceVerticalCenter = y + pieceHeights[pieceIndex] / 2.0; if (pieceIndex != format.centrepieceIndex) - _positionPiece(size.width, y, childSizes, format.pieces[pieceIndex].fragments); + _positionPiece(size.width, pieceVerticalCenter, childSizes, format.pieces[pieceIndex].fragments); else - _positionPivoted(size.width, y, childSizes, centrepiece.fragments, centrepiece.pivotIndex); + _positionPivoted(size.width, pieceVerticalCenter, childSizes, centrepiece.fragments, centrepiece.pivotIndex); y += pieceHeights[pieceIndex] + format.pieces[pieceIndex].bottomMargin; } @@ -774,7 +816,7 @@ class _TimePickerHeader extends StatelessWidget { use24HourDials: use24HourDials, ); - final _TimePickerHeaderFormat format = _buildHeaderFormat(timeOfDayFormat, fragmentContext); + final _TimePickerHeaderFormat format = _buildHeaderFormat(timeOfDayFormat, fragmentContext, orientation); return Container( width: width, diff --git a/packages/flutter/test/material/time_picker_test.dart b/packages/flutter/test/material/time_picker_test.dart index 198a892de4..6e9cc73a35 100644 --- a/packages/flutter/test/material/time_picker_test.dart +++ b/packages/flutter/test/material/time_picker_test.dart @@ -542,6 +542,38 @@ void _tests() { semantics.dispose(); }); + testWidgets('header touch regions are large enough', (WidgetTester tester) async { + await mediaQueryBoilerplate(tester, false); + + final Size amSize = tester.getSize(find.ancestor( + of: find.text('AM'), + matching: find.byType(InkWell), + )); + expect(amSize.width, greaterThanOrEqualTo(48.0)); + expect(amSize.height, greaterThanOrEqualTo(48.0)); + + final Size pmSize = tester.getSize(find.ancestor( + of: find.text('PM'), + matching: find.byType(InkWell), + )); + expect(pmSize.width, greaterThanOrEqualTo(48.0)); + expect(pmSize.height, greaterThanOrEqualTo(48.0)); + + final Size hourSize = tester.getSize(find.ancestor( + of: find.text('7'), + matching: find.byType(InkWell), + )); + expect(hourSize.width, greaterThanOrEqualTo(48.0)); + expect(hourSize.height, greaterThanOrEqualTo(48.0)); + + final Size minuteSize = tester.getSize(find.ancestor( + of: find.text('00'), + matching: find.byType(InkWell), + )); + expect(minuteSize.width, greaterThanOrEqualTo(48.0)); + expect(minuteSize.height, greaterThanOrEqualTo(48.0)); + }); + testWidgets('builder parameter', (WidgetTester tester) async { Widget buildFrame(TextDirection textDirection) { return MaterialApp(