Skip to content

Commit

Permalink
[Material] Relanding fix to ensure time picker input mode lays out co…
Browse files Browse the repository at this point in the history 
…rrectly in RTL (flutter#64097)
  • Loading branch information
@rami-a @mingwandroid
rami-a authored and mingwandroid committed Sep 6, 2020
1 parent ab4c4be commit df3976f
Show file tree
Hide file tree
Showing 3 changed files with 199 additions and 49 deletions.
162 changes: 114 additions & 48 deletions packages/flutter/lib/src/material/time_picker.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1414,58 +1414,69 @@ class _TimePickerInputState extends State<_TimePickerInput> {
),
const SizedBox(width: 12.0),
],
Expanded(child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: <Widget>[
const SizedBox(height: 8.0),
_HourMinuteTextField(
selectedTime: _selectedTime,
isHour: true,
style: hourMinuteStyle,
validator: _validateHour,
onSavedSubmitted: _handleHourSavedSubmitted,
onChanged: _handleHourChanged,
),
const SizedBox(height: 8.0),
if (!hourHasError && !minuteHasError)
ExcludeSemantics(
child: Text(
MaterialLocalizations.of(context).timePickerHourLabel,
style: theme.textTheme.caption,
maxLines: 1,
overflow: TextOverflow.ellipsis,
Expanded(
child: Row(
crossAxisAlignment: CrossAxisAlignment.start,
// Hour/minutes should not change positions in RTL locales.
textDirection: TextDirection.ltr,
children: <Widget>[
Expanded(
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: <Widget>[
const SizedBox(height: 8.0),
_HourTextField(
selectedTime: _selectedTime,
style: hourMinuteStyle,
validator: _validateHour,
onSavedSubmitted: _handleHourSavedSubmitted,
onChanged: _handleHourChanged,
),
const SizedBox(height: 8.0),
if (!hourHasError && !minuteHasError)
ExcludeSemantics(
child: Text(
MaterialLocalizations.of(context).timePickerHourLabel,
style: theme.textTheme.caption,
maxLines: 1,
overflow: TextOverflow.ellipsis,
),
),
],
),
),
],
)),
Container(
margin: const EdgeInsets.only(top: 8.0),
height: _kTimePickerHeaderControlHeight,
child: _StringFragment(timeOfDayFormat: timeOfDayFormat),
),
Expanded(child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: <Widget>[
const SizedBox(height: 8.0),
_HourMinuteTextField(
selectedTime: _selectedTime,
isHour: false,
style: hourMinuteStyle,
validator: _validateMinute,
onSavedSubmitted: _handleMinuteSavedSubmitted,
),
const SizedBox(height: 8.0),
if (!hourHasError && !minuteHasError)
ExcludeSemantics(
child: Text(
MaterialLocalizations.of(context).timePickerMinuteLabel,
style: theme.textTheme.caption,
maxLines: 1,
overflow: TextOverflow.ellipsis,
Container(
margin: const EdgeInsets.only(top: 8.0),
height: _kTimePickerHeaderControlHeight,
child: _StringFragment(timeOfDayFormat: timeOfDayFormat),
),
Expanded(
child: Column(
crossAxisAlignment: CrossAxisAlignment.start,
children: <Widget>[
const SizedBox(height: 8.0),
_MinuteTextField(
selectedTime: _selectedTime,
style: hourMinuteStyle,
validator: _validateMinute,
onSavedSubmitted: _handleMinuteSavedSubmitted,
),
const SizedBox(height: 8.0),
if (!hourHasError && !minuteHasError)
ExcludeSemantics(
child: Text(
MaterialLocalizations.of(context).timePickerMinuteLabel,
style: theme.textTheme.caption,
maxLines: 1,
overflow: TextOverflow.ellipsis,
),
),
],
),
),
],
)),
],
),
),
if (!use24HourDials && timeOfDayFormat != TimeOfDayFormat.a_space_h_colon_mm) ...<Widget>[
const SizedBox(width: 12.0),
_DayPeriodControl(
Expand All @@ -1489,6 +1500,61 @@ class _TimePickerInputState extends State<_TimePickerInput> {
}
}

class _HourTextField extends StatelessWidget {
const _HourTextField({
Key key,
@required this.selectedTime,
@required this.style,
@required this.validator,
@required this.onSavedSubmitted,
@required this.onChanged,
}) : super(key: key);

final TimeOfDay selectedTime;
final TextStyle style;
final FormFieldValidator<String> validator;
final ValueChanged<String> onSavedSubmitted;
final ValueChanged<String> onChanged;

@override
Widget build(BuildContext context) {
return _HourMinuteTextField(
selectedTime: selectedTime,
isHour: true,
style: style,
validator: validator,
onSavedSubmitted: onSavedSubmitted,
onChanged: onChanged,
);
}
}

class _MinuteTextField extends StatelessWidget {
const _MinuteTextField({
Key key,
@required this.selectedTime,
@required this.style,
@required this.validator,
@required this.onSavedSubmitted,
}) : super(key: key);

final TimeOfDay selectedTime;
final TextStyle style;
final FormFieldValidator<String> validator;
final ValueChanged<String> onSavedSubmitted;

@override
Widget build(BuildContext context) {
return _HourMinuteTextField(
selectedTime: selectedTime,
isHour: false,
style: style,
validator: validator,
onSavedSubmitted: onSavedSubmitted,
);
}
}

class _HourMinuteTextField extends StatefulWidget {
const _HourMinuteTextField({
Key key,
Expand Down
10 changes: 10 additions & 0 deletions packages/flutter/test/material/time_picker_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,16 @@ void _testsInput() {
await finishPicker(tester);
expect(result, equals(const TimeOfDay(hour: 8, minute: 15)));
});

// Fixes regression that was reverted in https://github.com/flutter/flutter/pull/64094#pullrequestreview-469836378.
testWidgets('Ensure hour/minute fields are top-aligned with the separator', (WidgetTester tester) async {
await startPicker(tester, (TimeOfDay time) { }, entryMode: TimePickerEntryMode.input);
final double hourFieldTop = tester.getTopLeft(find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_HourTextField')).dy;
final double minuteFieldTop = tester.getTopLeft(find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_MinuteTextField')).dy;
final double separatorTop = tester.getTopLeft(find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_StringFragment')).dy;
expect(hourFieldTop, separatorTop);
expect(minuteFieldTop, separatorTop);
});
}

final Finder findDialPaint = find.descendant(
Expand Down
76 changes: 75 additions & 1 deletion packages/flutter_localizations/test/material/time_picker_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@ import 'package:flutter_localizations/flutter_localizations.dart';
import 'package:flutter_test/flutter_test.dart';

class _TimePickerLauncher extends StatelessWidget {
const _TimePickerLauncher({ Key key, this.onChanged, this.locale }) : super(key: key);
const _TimePickerLauncher({
Key key,
this.onChanged,
this.locale,
this.entryMode = TimePickerEntryMode.dial,
}) : super(key: key);

final ValueChanged<TimeOfDay> onChanged;
final Locale locale;
final TimePickerEntryMode entryMode;

@override
Widget build(BuildContext context) {
Expand All @@ -28,6 +34,7 @@ class _TimePickerLauncher extends StatelessWidget {
onPressed: () async {
onChanged(await showTimePicker(
context: context,
initialEntryMode: entryMode,
initialTime: const TimeOfDay(hour: 7, minute: 0),
));
},
Expand Down Expand Up @@ -207,6 +214,73 @@ void main() {
tester.binding.window.devicePixelRatioTestValue = null;
});

testWidgets('can localize input mode in all known formats', (WidgetTester tester) async {
final Finder stringFragmentTextFinder = find.descendant(
of: find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_StringFragment'),
matching: find.byType(Text),
).first;
final Finder hourControlFinder = find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_HourTextField');
final Finder minuteControlFinder = find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_MinuteTextField');
final Finder dayPeriodControlFinder = find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_DayPeriodControl');

// TODO(yjbanov): also test `HH.mm` (in_ID), `a h:mm` (ko_KR) and `HH:mm น.` (th_TH) when we have .arb files for them
final List<Locale> locales = <Locale>[
const Locale('en', 'US'), //'h:mm a'
const Locale('en', 'GB'), //'HH:mm'
const Locale('es', 'ES'), //'H:mm'
const Locale('fr', 'CA'), //'HH \'h\' mm'
const Locale('zh', 'ZH'), //'ah:mm'
const Locale('fa', 'IR'), //'H:mm' but RTL
];

for (final Locale locale in locales) {
await tester.pumpWidget(_TimePickerLauncher(onChanged: (TimeOfDay time) { }, locale: locale, entryMode: TimePickerEntryMode.input));
await tester.tap(find.text('X'));
await tester.pumpAndSettle(const Duration(seconds: 1));

final Text stringFragmentText = tester.widget(stringFragmentTextFinder);
final double hourLeftOffset = tester.getTopLeft(hourControlFinder).dx;
final double minuteLeftOffset = tester.getTopLeft(minuteControlFinder).dx;
final double stringFragmentLeftOffset = tester.getTopLeft(stringFragmentTextFinder).dx;

if (locale == const Locale('en', 'US')) {
final double dayPeriodLeftOffset = tester.getTopLeft(dayPeriodControlFinder).dx;
expect(stringFragmentText.data, ':');
expect(hourLeftOffset, lessThan(stringFragmentLeftOffset));
expect(stringFragmentLeftOffset, lessThan(minuteLeftOffset));
expect(minuteLeftOffset, lessThan(dayPeriodLeftOffset));
} else if (locale == const Locale('en', 'GB')) {
expect(stringFragmentText.data, ':');
expect(hourLeftOffset, lessThan(stringFragmentLeftOffset));
expect(stringFragmentLeftOffset, lessThan(minuteLeftOffset));
expect(dayPeriodControlFinder, findsNothing);
} else if (locale == const Locale('es', 'ES')) {
expect(stringFragmentText.data, ':');
expect(hourLeftOffset, lessThan(stringFragmentLeftOffset));
expect(stringFragmentLeftOffset, lessThan(minuteLeftOffset));
expect(dayPeriodControlFinder, findsNothing);
} else if (locale == const Locale('fr', 'CA')) {
expect(stringFragmentText.data, 'h');
expect(hourLeftOffset, lessThan(stringFragmentLeftOffset));
expect(stringFragmentLeftOffset, lessThan(minuteLeftOffset));
expect(dayPeriodControlFinder, findsNothing);
} else if (locale == const Locale('zh', 'ZH')) {
final double dayPeriodLeftOffset = tester.getTopLeft(dayPeriodControlFinder).dx;
expect(stringFragmentText.data, ':');
expect(dayPeriodLeftOffset, lessThan(hourLeftOffset));
expect(hourLeftOffset, lessThan(stringFragmentLeftOffset));
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 finishPicker(tester);
}
});

testWidgets('uses single-ring 24-hour dial for all formats', (WidgetTester tester) async {
const List<Locale> locales = <Locale>[
Locale('en', 'US'), // h
Expand Down

0 comments on commit df3976f

Please sign in to comment.