[time picker] Only update time when updating input in TimePicker #4398
Conversation
flaviendelangle
added
bug 🐛
| @@ -18,6 +18,13 @@ const valueManager: PickerStateValueManager<unknown, unknown> = { | |||
| parseInput: parsePickerInputValue, | |||
| areValuesEqual: (utils: MuiPickersAdapter<unknown>, a: unknown, b: unknown) => | |||
| utils.isEqual(a, b), | |||
| updateValue: (utils, prevValue, newValue) => { | |||
There was a problem hiding this comment.
Optional method (to be named in a better way) that creates the new value from the previous valud and the new one.
Allows us in TimePicker to only take the time from the new value but keep the date of the old value.
TODO: Do the same for MobileTimePicker (maybe refacto to avoid duplication on the value managers at some point)
date-fns handles this behavior out of the box, you can pass a reference date and when updating a mask it only applies the change of the elements in the mask, the rest is taken from the reference date.
It would be perfect here, but the others date libs do not support it from what I see.
|
These are the results for the performance tests:
|
|
For me, it makes sense to track the last valid date. Since we are talking about a time picker, I assume the date can only be modified by the Why using a |
|
The ref was to avoid a re-render since we are only querying its value in a callback. I did a first version with a state and a test |
|
It sounds good. Do you want to do the |
|
Yes, I applied the same logic on the mobile |
| @@ -18,6 +18,13 @@ const valueManager: PickerStateValueManager<unknown, unknown> = { | |||
| parseInput: parsePickerInputValue, | |||
| areValuesEqual: (utils: MuiPickersAdapter<unknown>, a: unknown, b: unknown) => | |||
| utils.isEqual(a, b), | |||
| mergeOldAndNewValues: (utils, prevValue, newValue) => { | |||
There was a problem hiding this comment.
Another naming option: valueReducer(newValue, prevValue?, utils?)
There was a problem hiding this comment.
Renamed
I keep utils 1st like on the other methods.
But feel free to change it later 👍
Fixes #4397
@alexfauquette I would like your opinion on this one.
My logic is to get the time (hour, minute, seconds, milliseconds) from the new date parsed from the input. But to get the date (day, month, year) from the previous date.
It works great, except if you don't have the
ignoreInvalidInputsset totrue(it blocks the onChange firing when the date is invalid).Because as soon as you get an invalid date, your previous date becomes null on the next
onChangeand I can't merge it.What do you prefer ?
ignoreInvalidInputs=true