[pickers] Do not apply the current time when no date provided in DayPicker
#4649
Conversation
|
These are the results for the performance tests:
|
flaviendelangle
added
bug 🐛
| import { DayPicker } from './DayPicker'; | ||
| import { adapterToUse, createPickerRenderer } from '../../../../test/utils/pickers-utils'; | ||
|
|
||
| describe('<DayPicker />', () => { |
There was a problem hiding this comment.
I added the test directly on DayPicker, I think the structure of the pickers favors more unit tests on the smaller components.
| focusedDay={null} | ||
| onFocusedDayChange={() => {}} | ||
| onMonthSwitchingAnimationEnd={() => {}} | ||
| isDateDisabled={() => false} | ||
| isMonthSwitchingAnimating={false} | ||
| slideDirection="right" | ||
| reduceAnimations={false} |
There was a problem hiding this comment.
Why do you define all those props? Is it for performance improvement?
There was a problem hiding this comment.
No, it's because DayPicker being an internal component, it was built with almost all its props required and I did not want to change its api at all.
But some default value could probably be inside DayPicker instead of higher in the component chain.
| @@ -139,12 +139,20 @@ export function DayPicker<TDate>(props: PickersCalendarProps<TDate>) { | |||
| if (readOnly) { | |||
| return; | |||
| } | |||
|
|
|||
| // TODO possibly buggy line figure out and add tests | |||
There was a problem hiding this comment.
Now that it's tested, the comment could be removed, or at least remove 'line' otherwise in a few months we will wonder why let finalDate can be buggy
| // TODO possibly buggy line figure out and add tests | |
| // TODO possibly buggy figure out and add tests |
There was a problem hiding this comment.
I found the issue related to this comment ealier today #4703
I think to problem is that some date library represents dates as array so this check is problematic and we should pass an isRange boolean instead to differentiate ranges and mono dates.
Fixes #4449
If we use the
DatePickerorDateTimePicker, when clicking on a date for the 1st time, it should set the time to00:00:00not the current timeAdd tests