[pickers] Adds new source property to onChange and onAccept context object
#20234
Conversation
…tation Signed-off-by: michel <jsnerdic@gmail.com>
michelengelen
requested review from
LukasTy,
flaviendelangle,
mj12albert and
siriwatknp
michelengelen
added
scope: pickers
|
Deploy preview: https://deploy-preview-20234--material-ui-x.netlify.app/ Updated pages: Bundle size report
|
| shortcut?: PickersShortcutsItemContext; | ||
| /** | ||
| * Source of the change. | ||
| * If not provided, it will be inferred as "picker" when a `shortcut` is present, otherwise defaults to "field". |
There was a problem hiding this comment.
Are you sure about this inferrance?
There was a problem hiding this comment.
I did test this with the demos from the docs, so I think it should be picking up everything as it is intended to do. Or did I miss something?
Screen.Recording.2025-11-06.at.14.03.35.mov
There was a problem hiding this comment.
do you think it would make sense to also include 'shortcut' as an option?
There was a problem hiding this comment.
Oh I see
It feels fragile to me. If tomorrow we add a new UI that updates the value, we can easily have it flagged as source: "field" by mistake.
WDYT about instead making this property required in SetValueActionOptions?
Or at least put picker as the default since it's the one triggered by all UIs except the field, and then have the field explicitely put source: "field".
Side note, maybe it's worth being more precise thant picker (have source: "field" | "view" | "shortcut" | ...
But no strong preference here.
There was a problem hiding this comment.
yes, that makes sense ... so, top get this straight the proposal would be this:
export interface SetValueActionOptions<TError = string | null> {
...
/**
* Source of the change.
* @default 'picker'
*/
source: 'picker' | 'field' | 'view' | 'shortcut';
...
}Correct?
There was a problem hiding this comment.
@flaviendelangle I have added a new commit with a similar approach to what you suggested, but with this it is even possible to create more general handling if the granular approach is too much. So basically I kept the source as is, but extended that with more granular sub-sources, e.g. 'picker:actionBar:ok'. That way the user can determine easily where that change came from. WDYT?
There was a problem hiding this comment.
const clearValue = useEventCallback(() =>
setValue(valueManager.emptyValue, { source: 'picker:actionBar:clear' }),
);This is wrong, clearValue can be called from anywhere, not only from the action bar.
WDYT about keeping "field" | "view" | "unknown"
For those two, we know that the change has to come from the requested place (for field it's the field that sets the source, for the view, the method is passed directly to the view and is not accessible elsewhere).
And then we can see if people ask for other places and react accordingly.
I have no idea if actionBar:ok is usefull, or if actionBar would be enough, so I propose to wait and see what users want 👍
There was a problem hiding this comment.
@michelengelen looks good to me 👌
Just one last question: what is the reasoning behind using "picker" rather than "view"? Can it be used from elsewhere than the view?
There was a problem hiding this comment.
No particular reason. Maybe because it is a "Picker" overlay? Not against naming it view though.
There was a problem hiding this comment.
My main struggle with "picker" is that everything is "picker" since the component itself is a picker.
At least it's the nomenclature that we are using. We have the field (the input to type the date with the keyboard) and the view (the UI to select the date with the mouse). Together (and with the shortcuts, the toolbar, the tabs and the action bar) they form the picker component.
…ated related documentation Signed-off-by: michel <jsnerdic@gmail.com>
Signed-off-by: michel <jsnerdic@gmail.com>
Signed-off-by: michel <jsnerdic@gmail.com>
… documentation Signed-off-by: michel <jsnerdic@gmail.com>
Signed-off-by: michel <jsnerdic@gmail.com>
…cceptances Signed-off-by: michel <jsnerdic@gmail.com>
Signed-off-by: michel <jsnerdic@gmail.com>
Signed-off-by: michel <jsnerdic@gmail.com>
Co-authored-by: Flavien DELANGLE <flaviendelangle@gmail.com> Signed-off-by: Michel Engelen <32863416+michelengelen@users.noreply.github.com>
Signed-off-by: michel <jsnerdic@gmail.com>
3 participants
This pull request introduces a new
sourceproperty to the date/time picker lifecycle and event handler context, enabling clearer differentiation between changes initiated by the picker UI and those from direct field input.The documentation is updated to explain this addition, and the codebase is refactored to consistently set and propagate the
sourceproperty throughout picker interactions and event handlers.Fixes #19830