Unverified Commit f29f05ed authored by Rami's avatar Rami Committed by GitHub

Address misc time picker design issues (#62803)

parent 84974968
...@@ -162,9 +162,17 @@ class _TimePickerHeader extends StatelessWidget { ...@@ -162,9 +162,17 @@ class _TimePickerHeader extends StatelessWidget {
), ),
const SizedBox(width: 12.0), const SizedBox(width: 12.0),
], ],
Expanded(child: _HourControl(fragmentContext: fragmentContext)), Expanded(
_StringFragment(timeOfDayFormat: timeOfDayFormat), child: Row(
Expanded(child: _MinuteControl(fragmentContext: fragmentContext)), // Hour/minutes should not change positions in RTL locales.
textDirection: TextDirection.ltr,
children: <Widget>[
Expanded(child: _HourControl(fragmentContext: fragmentContext)),
_StringFragment(timeOfDayFormat: timeOfDayFormat),
Expanded(child: _MinuteControl(fragmentContext: fragmentContext)),
],
),
),
if (!use24HourDials && timeOfDayFormat != TimeOfDayFormat.a_space_h_colon_mm) ...<Widget>[ if (!use24HourDials && timeOfDayFormat != TimeOfDayFormat.a_space_h_colon_mm) ...<Widget>[
const SizedBox(width: 12.0), const SizedBox(width: 12.0),
_DayPeriodControl( _DayPeriodControl(
...@@ -195,6 +203,8 @@ class _TimePickerHeader extends StatelessWidget { ...@@ -195,6 +203,8 @@ class _TimePickerHeader extends StatelessWidget {
Container( Container(
height: kMinInteractiveDimension * 2, height: kMinInteractiveDimension * 2,
child: Row( child: Row(
// Hour/minutes should not change positions in RTL locales.
textDirection: TextDirection.ltr,
children: <Widget>[ children: <Widget>[
Expanded(child: _HourControl(fragmentContext: fragmentContext)), Expanded(child: _HourControl(fragmentContext: fragmentContext)),
_StringFragment(timeOfDayFormat: timeOfDayFormat), _StringFragment(timeOfDayFormat: timeOfDayFormat),
...@@ -1126,7 +1136,7 @@ class _DialState extends State<_Dial> with SingleTickerProviderStateMixin { ...@@ -1126,7 +1136,7 @@ class _DialState extends State<_Dial> with SingleTickerProviderStateMixin {
]; ];
_TappableLabel _buildTappableLabel(TextTheme textTheme, Color color, int value, String label, VoidCallback onTap) { _TappableLabel _buildTappableLabel(TextTheme textTheme, Color color, int value, String label, VoidCallback onTap) {
final TextStyle style = textTheme.subtitle1.copyWith(color: color); final TextStyle style = textTheme.bodyText1.copyWith(color: color);
final double labelScaleFactor = math.min(MediaQuery.of(context).textScaleFactor, 2.0); final double labelScaleFactor = math.min(MediaQuery.of(context).textScaleFactor, 2.0);
return _TappableLabel( return _TappableLabel(
value: value, value: value,
...@@ -1201,8 +1211,8 @@ class _DialState extends State<_Dial> with SingleTickerProviderStateMixin { ...@@ -1201,8 +1211,8 @@ class _DialState extends State<_Dial> with SingleTickerProviderStateMixin {
final TimePickerThemeData pickerTheme = TimePickerTheme.of(context); final TimePickerThemeData pickerTheme = TimePickerTheme.of(context);
final Color backgroundColor = pickerTheme.dialBackgroundColor ?? themeData.colorScheme.onBackground.withOpacity(0.12); final Color backgroundColor = pickerTheme.dialBackgroundColor ?? themeData.colorScheme.onBackground.withOpacity(0.12);
final Color accentColor = pickerTheme.dialHandColor ?? themeData.colorScheme.primary; final Color accentColor = pickerTheme.dialHandColor ?? themeData.colorScheme.primary;
final Color primaryLabelColor = MaterialStateProperty.resolveAs(pickerTheme.dialTextColor, <MaterialState>{}); final Color primaryLabelColor = MaterialStateProperty.resolveAs(pickerTheme.dialTextColor, <MaterialState>{}) ?? themeData.colorScheme.onSurface;
final Color secondaryLabelColor = MaterialStateProperty.resolveAs(pickerTheme.dialTextColor, <MaterialState>{MaterialState.selected}); final Color secondaryLabelColor = MaterialStateProperty.resolveAs(pickerTheme.dialTextColor, <MaterialState>{MaterialState.selected}) ?? themeData.colorScheme.onPrimary;
List<_TappableLabel> primaryLabels; List<_TappableLabel> primaryLabels;
List<_TappableLabel> secondaryLabels; List<_TappableLabel> secondaryLabels;
int selectedDialValue; int selectedDialValue;
...@@ -1539,11 +1549,9 @@ class _HourMinuteTextFieldState extends State<_HourMinuteTextField> { ...@@ -1539,11 +1549,9 @@ class _HourMinuteTextFieldState extends State<_HourMinuteTextField> {
if (inputDecorationTheme != null) { if (inputDecorationTheme != null) {
inputDecoration = const InputDecoration().applyDefaults(inputDecorationTheme); inputDecoration = const InputDecoration().applyDefaults(inputDecorationTheme);
} else { } else {
final Color unfocusedFillColor = timePickerTheme.hourMinuteColor ?? colorScheme.onSurface.withOpacity(0.12);
inputDecoration = InputDecoration( inputDecoration = InputDecoration(
contentPadding: EdgeInsets.zero, contentPadding: EdgeInsets.zero,
filled: true, filled: true,
fillColor: focusNode.hasFocus ? Colors.transparent : unfocusedFillColor,
enabledBorder: const OutlineInputBorder( enabledBorder: const OutlineInputBorder(
borderSide: BorderSide(color: Colors.transparent), borderSide: BorderSide(color: Colors.transparent),
), ),
...@@ -1561,10 +1569,12 @@ class _HourMinuteTextFieldState extends State<_HourMinuteTextField> { ...@@ -1561,10 +1569,12 @@ class _HourMinuteTextFieldState extends State<_HourMinuteTextField> {
errorStyle: const TextStyle(fontSize: 0.0, height: 0.0), // Prevent the error text from appearing. errorStyle: const TextStyle(fontSize: 0.0, height: 0.0), // Prevent the error text from appearing.
); );
} }
final Color unfocusedFillColor = timePickerTheme.hourMinuteColor ?? colorScheme.onSurface.withOpacity(0.12);
inputDecoration = inputDecoration.copyWith( inputDecoration = inputDecoration.copyWith(
// Remove the hint text when focused because the centered cursor appears // Remove the hint text when focused because the centered cursor appears
// odd above the hint text. // odd above the hint text.
hintText: focusNode.hasFocus ? null : _formattedValue, hintText: focusNode.hasFocus ? null : _formattedValue,
fillColor: focusNode.hasFocus ? Colors.transparent : inputDecorationTheme?.fillColor ?? unfocusedFillColor,
); );
return SizedBox( return SizedBox(
...@@ -1574,10 +1584,13 @@ class _HourMinuteTextFieldState extends State<_HourMinuteTextField> { ...@@ -1574,10 +1584,13 @@ class _HourMinuteTextFieldState extends State<_HourMinuteTextField> {
child: TextFormField( child: TextFormField(
expands: true, expands: true,
maxLines: null, maxLines: null,
inputFormatters: <TextInputFormatter>[
LengthLimitingTextInputFormatter(2),
],
focusNode: focusNode, focusNode: focusNode,
textAlign: TextAlign.center, textAlign: TextAlign.center,
keyboardType: TextInputType.number, keyboardType: TextInputType.number,
style: widget.style.copyWith(color: colorScheme.onSurface), style: widget.style.copyWith(color: timePickerTheme.hourMinuteTextColor ?? colorScheme.onSurface),
controller: controller, controller: controller,
decoration: inputDecoration, decoration: inputDecoration,
validator: widget.validator, validator: widget.validator,
......
...@@ -797,7 +797,7 @@ void _testsInput() { ...@@ -797,7 +797,7 @@ void _testsInput() {
// Invalid minute. // Invalid minute.
await tester.enterText(find.byType(TextField).first, '8'); await tester.enterText(find.byType(TextField).first, '8');
await tester.enterText(find.byType(TextField).last, '150'); await tester.enterText(find.byType(TextField).last, '95');
await finishPicker(tester); await finishPicker(tester);
expect(result, null); expect(result, null);
......
...@@ -158,14 +158,16 @@ void main() { ...@@ -158,14 +158,16 @@ void main() {
final List<dynamic> primaryLabels = dialPainter.primaryLabels as List<dynamic>; final List<dynamic> primaryLabels = dialPainter.primaryLabels as List<dynamic>;
expect( expect(
primaryLabels.first.painter.text.style, primaryLabels.first.painter.text.style,
Typography.material2014().englishLike.subhead Typography.material2014().englishLike.bodyText1
.merge(Typography.material2014().black.subhead), .merge(Typography.material2014().black.bodyText1)
.copyWith(color: defaultTheme.colorScheme.onSurface),
); );
final List<dynamic> secondaryLabels = dialPainter.secondaryLabels as List<dynamic>; final List<dynamic> secondaryLabels = dialPainter.secondaryLabels as List<dynamic>;
expect( expect(
secondaryLabels.first.painter.text.style, secondaryLabels.first.painter.text.style,
Typography.material2014().englishLike.subhead Typography.material2014().englishLike.bodyText1
.merge(Typography.material2014().white.subhead), .merge(Typography.material2014().white.bodyText1)
.copyWith(color: defaultTheme.colorScheme.onPrimary),
); );
final Material hourMaterial = _textMaterial(tester, '7'); final Material hourMaterial = _textMaterial(tester, '7');
...@@ -297,15 +299,15 @@ void main() { ...@@ -297,15 +299,15 @@ void main() {
final List<dynamic> primaryLabels = dialPainter.primaryLabels as List<dynamic>; final List<dynamic> primaryLabels = dialPainter.primaryLabels as List<dynamic>;
expect( expect(
primaryLabels.first.painter.text.style, primaryLabels.first.painter.text.style,
Typography.material2014().englishLike.subhead Typography.material2014().englishLike.bodyText1
.merge(Typography.material2014().black.subhead) .merge(Typography.material2014().black.bodyText1)
.copyWith(color: _unselectedColor), .copyWith(color: _unselectedColor),
); );
final List<dynamic> secondaryLabels = dialPainter.secondaryLabels as List<dynamic>; final List<dynamic> secondaryLabels = dialPainter.secondaryLabels as List<dynamic>;
expect( expect(
secondaryLabels.first.painter.text.style, secondaryLabels.first.painter.text.style,
Typography.material2014().englishLike.subhead Typography.material2014().englishLike.bodyText1
.merge(Typography.material2014().white.subhead) .merge(Typography.material2014().white.bodyText1)
.copyWith(color: _selectedColor), .copyWith(color: _selectedColor),
); );
......
...@@ -78,6 +78,7 @@ void main() { ...@@ -78,6 +78,7 @@ void main() {
const Locale('es', 'ES'), //'H:mm' const Locale('es', 'ES'), //'H:mm'
const Locale('fr', 'CA'), //'HH \'h\' mm' const Locale('fr', 'CA'), //'HH \'h\' mm'
const Locale('zh', 'ZH'), //'ah:mm' const Locale('zh', 'ZH'), //'ah:mm'
const Locale('fa', 'IR'), //'H:mm' but RTL
]; ];
for (final Locale locale in locales) { for (final Locale locale in locales) {
...@@ -114,6 +115,12 @@ void main() { ...@@ -114,6 +115,12 @@ void main() {
expect(dayPeriodLeftOffset, lessThan(hourLeftOffset)); expect(dayPeriodLeftOffset, lessThan(hourLeftOffset));
expect(hourLeftOffset, lessThan(stringFragmentLeftOffset)); expect(hourLeftOffset, lessThan(stringFragmentLeftOffset));
expect(stringFragmentLeftOffset, lessThan(minuteLeftOffset)); expect(stringFragmentLeftOffset, lessThan(minuteLeftOffset));
} else if (locale == const Locale('fa', 'IR')) {
// Even though this is an RTL locale, the hours and minutes positions should remain the same.
expect(stringFragmentText.data, ':');
expect(hourLeftOffset, lessThan(stringFragmentLeftOffset));
expect(stringFragmentLeftOffset, lessThan(minuteLeftOffset));
expect(dayPeriodControlFinder, findsNothing);
} }
await tester.tapAt(Offset(center.dx, center.dy - 50.0)); await tester.tapAt(Offset(center.dx, center.dy - 50.0));
await finishPicker(tester); await finishPicker(tester);
...@@ -143,6 +150,7 @@ void main() { ...@@ -143,6 +150,7 @@ void main() {
const Locale('es', 'ES'), //'H:mm' const Locale('es', 'ES'), //'H:mm'
const Locale('fr', 'CA'), //'HH \'h\' mm' const Locale('fr', 'CA'), //'HH \'h\' mm'
const Locale('zh', 'ZH'), //'ah:mm' const Locale('zh', 'ZH'), //'ah:mm'
const Locale('fa', 'IR'), //'H:mm' but RTL
]; ];
for (final Locale locale in locales) { for (final Locale locale in locales) {
...@@ -184,6 +192,12 @@ void main() { ...@@ -184,6 +192,12 @@ void main() {
expect(stringFragmentLeftOffset, lessThan(minuteLeftOffset)); expect(stringFragmentLeftOffset, lessThan(minuteLeftOffset));
expect(hourLeftOffset, dayPeriodLeftOffset); expect(hourLeftOffset, dayPeriodLeftOffset);
expect(hourTopOffset, greaterThan(dayPeriodTopOffset)); expect(hourTopOffset, greaterThan(dayPeriodTopOffset));
} else if (locale == const Locale('fa', 'IR')) {
// Even though this is an RTL locale, the hours and minutes positions should remain the same.
expect(stringFragmentText.data, ':');
expect(hourLeftOffset, lessThan(stringFragmentLeftOffset));
expect(stringFragmentLeftOffset, lessThan(minuteLeftOffset));
expect(dayPeriodControlFinder, findsNothing);
} }
await tester.tapAt(Offset(center.dx, center.dy - 50.0)); await tester.tapAt(Offset(center.dx, center.dy - 50.0));
await finishPicker(tester); await finishPicker(tester);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment