[Android] Create DatePickerDialog on demand
#25363
Conversation
|
Hey there @OvrBtn! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
OvrBtn
changed the title
DatePickerDialog on demandDatePickerDialog on demand
jsuarezruiz
added
platform/android
area-controls-datetimepicker
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
Failing tests don't seem related. |
mattleibow
left a comment
mattleibow
left a comment
There was a problem hiding this comment.
This PR has some good things, but I feel like it is working with bad source code. Maybe check to see if the whole dialog can be created on demand when it is about to appear.
| internal DatePickerDialog? DatePickerDialog { get { return _dialog; } } | ||
| internal DatePickerDialog? DatePickerDialog { get { return _dialog ??= CreateDefaultDatePickerDialog(); } } |
There was a problem hiding this comment.
Can we just create the dialog in ShowPickerDialog like the time picker? I also see that in time picker, we always re-create the dialog:
void ShowPickerDialog(int hour, int minute)
{
_dialog = CreateTimePickerDialog(hour, minute);
_dialog.Show();
}I see we do set some values in the mappers for the min/max dates, but I feel like that was wrong as we can totally set this in the setDateLater part of the ShowPickerDialog just like we set the actual date.
I think this whole setup should rather be a lazy loading thing and we set the min/max when the picker is about to appear.
| if (VirtualView != null) | ||
| { | ||
| dialog.DatePicker.MaxDate = (long)VirtualView.MaximumDate.ToUniversalTimeNative().Subtract(DateTime.MinValue.AddYears(1969)).TotalMilliseconds; | ||
| dialog.DatePicker.MinDate = (long)VirtualView.MinimumDate.ToUniversalTimeNative().Subtract(DateTime.MinValue.AddYears(1969)).TotalMilliseconds; | ||
| } |
There was a problem hiding this comment.
Well, maybe no need to the setLater as you do it here. In this case, we can use a totally lazy creating.
3 participants
Description of Change
This PR is an addition to #24948 and it further improves performance of first render of
DatePickerby creatingDatePickerDialogon demand instead of on control render.Issues Fixed
Fixes #24929
Blockers
This PR should be merged after #20547 is merged since it might break workarounds like this one #12899 (comment)