TimePicker moves to minute mode after hour selection #29876
Conversation
remove whitespace
4429e38 to
024c773
Compare
|
@HansMuller I checked this with the TimePicker tests and modified them so they all pass with (and also test) the new behavior, any chance of it landing seperately from the larger updates? |
ghost
left a comment
There was a problem hiding this comment.
Thanks for working on this. Looks good. I have a couple of issues for you to consider, which I describe in comments below.
| final _TimePickerMode mode; | ||
| final bool use24HourDials; | ||
| final ValueChanged<TimeOfDay> onChanged; | ||
| final VoidCallback onHourDragEnd; |
There was a problem hiding this comment.
These two new callbacks really are describing the same abstract event: the user is done editing one of the units for the time. How they did that (dragging or tapping) isn't really important to the client of this widget.
I would suggest having only one of these and instead of tying it to just the hour, have it called for either the hour or the minutes. This would allow more flexibility. So something like;
final VoidCallback onTimeUnitFinished;And then let the client check for the mode and do whatever it wants for either the hour being set or the minutes (which in this case would only be checking for the hour and switching modes).
| @required this.mode, | ||
| @required this.use24HourDials, | ||
| @required this.onChanged, | ||
| @required this.onHourDragEnd, |
There was a problem hiding this comment.
Minor nit, but this probably doesn't need to be required, but as this is an internal widget, probably not critical.
| }); | ||
| } | ||
|
|
||
| void _handleHourDragEnd() { |
There was a problem hiding this comment.
These both need to be doing the same thing as _handleModeChanged(). It would make sense to refactor the guts of _handleModeChanged() into a _setMode(_TimePickerMode mode) and then have _handleModeChanged() and these (or a new _handleTimeUnitFinished() from the above comment) call _setMode().
Adds a feature of the native Android Time Picker to our Material Time Picker. When the user selects an hour, it automatically switches to minute mode. This is a merging of two pull requests: Code changes from @sdolski #24677 Tests from @lucaslcode #29876 Thanks to both of you for your contributions!
Description
Updates TimePicker so both dragging and tapping in hour mode will change to minute mode, as is native android behavior.
Related Issues
#17276
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?