-
Notifications
You must be signed in to change notification settings - Fork 29.1k
Add full size Cupertino calendar date picker #93130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Gold has detected about 9 new digest(s) on patchset 5. |
Gold has detected about 18 new digest(s) on patchset 8. |
Gold has detected about 10 new digest(s) on patchset 9. |
890a389
to
a2d0cee
Compare
Gold has detected about 6 new digest(s) on patchset 9. |
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
Updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the pull request! Left comments on a few minor things. I noticed the implementation of the new date picker is very similar to the material date picker. It will be great if we can take advantage of the existing material date picker implementation and avoid introducing duplicated code.
Discussed this with @HansMuller and @darrenaustin, they suggested moving the implementation to the material library and expose the calendar picker widget via a CalendarDatePicker .adaptive
or DatePicker.adaptive
constructor. What do you think?
/// The latest allowable [DateTime] that the user can select. | ||
final DateTime? maximumDate; | ||
|
||
/// Called when the user change a date in the picker. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Called when the user change a date in the picker. | |
/// Called when the user selects a date in the picker. |
DateTime? initialDate, | ||
this.onDisplayedMonthChanged, | ||
}) : initialDate = initialDate ?? DateTime.now(), | ||
minimumDate = DateTime(1900), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 2 are not meant to be hard-coded I assume?
/// [maximumDate] must be after or equal to [minimumDate]. | ||
/// | ||
/// [initialDate] must be between [minimumDate] and [maximumDate] or equal to | ||
/// one of them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and defaults to DateTime.now()
?
/// to start in the year selection interface with [initialCalendarMode] set | ||
/// to [CalendarPickerMode.year]. | ||
/// | ||
/// The [onDateChanged], and [initialCalendarMode] must be non-null. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: The [onDateChanged] parameter
final CalendarPickerMode initialCalendarMode; | ||
|
||
/// The earliest allowable [DateTime] that the user can select. | ||
final DateTime initialDate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documentation seems to be describing minimumDate
const Widget _centerSelectionOverlay = CupertinoPickerDefaultSelectionOverlay(capLeftEdge: false, capRightEdge: false); | ||
const Widget _rightSelectionOverlay = CupertinoPickerDefaultSelectionOverlay(capLeftEdge: false); | ||
|
||
const int _maxDayPickerRowCount = 6; // A 31 day month that starts on Saturday. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm why is this 6 not 7? Also isn't this the same as DateTime.daysPerWeek
? I noticed you are using DateTime.daysPerWeek
somewhere else.
|
||
const int _maxDayPickerRowCount = 6; // A 31 day month that starts on Saturday. | ||
// One extra row for the day-of-week header. | ||
const double _dayPickerRowHeight = 47.0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these have to be hard coded?
late CalendarPickerMode _mode; | ||
late DateTime _currentDisplayedMonthDate; | ||
late DateTime _selectedDate; | ||
final GlobalKey _monthPickerKey = GlobalKey(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these global keys needed?
__MonthPickerState createState() => __MonthPickerState(); | ||
} | ||
|
||
class __MonthPickerState extends State<_MonthPicker> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's double _
before the class name. Is that intended?
} | ||
|
||
class __MonthPickerState extends State<_MonthPicker> { | ||
final GlobalKey _pageViewKey = GlobalKey(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need the global key here?
@LongCatIsLooong
I'm gonna take this approach adaptive approach. I'll take your all suggestions here into count and work on this implementation in the new PR to avoid adding that code to this bubbled PR. Closing this for now. |
@TahaTesser Do you have any timeline for this? Many people love this feature and are waiting for it! Thanks for your contribution. |
Any update here? I would really love having the iOS 14+ calendar picker in flutter. |
Updates? |
Related #63693
This introduces a standalone full-size Cupertino Calendar Picker, this will become one of the picker styles in
CupertinoDatePicker
. This is designed to be as close as possible to the native full-size calendar picker. Initial Implementation matches Material's calendar picker.This calendar picker has two views grid and wheel (the wheel is customized to show month/year).
Light mode
Dark mode
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.