[pickers] Always use props.value as the source of truth when defined
#16853
Conversation
flaviendelangle
added
regression
flaviendelangle
force-pushed
the
picker-value-controlled
branch
from
bf91819 to
37abb52
Compare
|
Deploy preview: https://deploy-preview-16853--material-ui-x.netlify.app/ |
flaviendelangle
force-pushed
the
picker-value-controlled
branch
from
d7aec39 to
7714139
Compare
flaviendelangle
changed the title
props.value as the source of truth when defined
flaviendelangle
force-pushed
the
picker-value-controlled
branch
from
804a8f1 to
1977df2
Compare
There was a problem hiding this comment.
Great work! 👏 💙
Thank you for taking care of it. 🙏
I've updated my example with use cases that I checked in the original (fields) related PR.
Everyhing seems to check out. 👌
| onError: props.onError, | ||
| }); | ||
|
|
||
| const timezoneFromDraftValue = valueManager.getTimezone(utils, state.draft); | ||
| if (previousTimezoneProp !== timezoneProp) { |
There was a problem hiding this comment.
Nice cleanup allowed by refactor. 🙌 💙
| if (selectionState === 'shallow') { | ||
| setState((prev) => ({ | ||
| ...prev, | ||
| draft: newValue, | ||
| clockShallowValue: newValue, |
There was a problem hiding this comment.
Yeah... The Time Clock could really use a value management refactor. 🙈
Now, its behavior has to be "declared" in the common state management. 🤷
There was a problem hiding this comment.
AFAIK, the only reason why we can't just keep those "shallow" value inside the clock state, is to update the value in the toolbar while dragging, but without calling onChange.
If we decide that we are fine either:
A. Calling onChange during the drag
B. Not updating the toolbar during the drag
Then we could get rid of this state altogether.
There was a problem hiding this comment.
B. Not updating the toolbar during the drag
Personally, I don't think it's such a bad idea. 🤔
What do you think? 🤔
It might even be more transparent to the user, that they are only selecting (not have already selected) the new value, while dragging.
There was a problem hiding this comment.
I think it's very subjective tbh 😆
Given that the clock is used by way less people than before, maybe going for the simpler option makes sense here.
And I agree that it may clear out the expectations.
|
Hi @flaviendelangle |
|
Hi, |
Closes #15530
Follow up on #15875