[pickers] Use `usePickerContext()` and `usePickerActionsContext()` instead of passing props to the `shortcuts` and `toolbar` slots #15948

flaviendelangle · 2024-12-19T18:43:51Z

Closes #15495 ( 🙏 )

What?

Remove the last props passed to the shortcuts, layout and toolbar slots
Introduce a new useIsValidValue hook to avoid passing something to usePickerContext that updates on every render
Add value to usePickerContext
Add setValue to usePickerContext and usePickerActionContext. This should eventually be the only way for parts of the UI to update the value (it will replace the onChange prop passed to the views and to the field in the future).
Rename PickerShortcutChangeImportance into PickerChangeImportance since it's not only used for the shortcut anymore.

Changelog

Breaking changes

The component passed to the layout slot no longer receives the value, onChange and onSelectShortcut props — Learn more.
The component passed to the toolbar slot no longer receives the value, onChange and isLandscape props — Learn more.
The component passed to the shortcuts slot no longer receives the onChange, isValid and isLandscape props — Learn more.
The PickerShortcutChangeImportance type has been renamed PickerChangeImportance — Learn more.

mui-bot · 2024-12-19T18:47:31Z

Deploy preview: https://deploy-preview-15948--material-ui-x.netlify.app/

Updated pages:

docs/data/migration/migration-pickers-v7/migration-pickers-v7.md

Generated by 🚫 dangerJS against dcc6d9e

flaviendelangle · 2024-12-20T11:27:55Z

packages/x-date-pickers/src/hooks/useIsValidValue.tsx

+/**
+ * Returns a function to check if a value is valid according to the validation props passed to the parent picker.
+ */
+export function useIsValidValue<TValue extends PickerValidValue>() {


For now it's a super dumb hook.
It re-renders everytime the picker render and just calls the function that the picker gives him (which calls the validation).

But introducing this hook will allow us in the future to add it to the Date Calendar and Digital Clock as well and potentially to add some caching mechanism to avoid testing several times the same date.

For now it's only needed for the shortcut slot because we were passing this isValid prop.
When the view and the field will use it, it will probably impact the shape of the context(s) related to validation, but I'm pretty sure we will still have a hook with this exact signature 👌

flaviendelangle · 2024-12-20T11:57:05Z

packages/x-date-pickers/src/internals/hooks/usePicker/usePickerValue.types.ts

+   * The shortcut that triggered this change.
+   * Should not be defined if the change does not come from a shortcut.
+   */
+  shortcut?: PickersShortcutsItemContext;


In the Base UI version we won't have a notion of shortcut, so this field will probably only live in the MUI version (since some people needed it).

The other will be handled by the Base UI layer.

michelengelen

Looks good i general ... only concern is that we could simplify all places where we check for value == null || !utils.isValid(value) into utils.isValid(value) ... agree?

michelengelen · 2024-12-20T15:18:12Z

docs/data/migration/migration-pickers-v7/migration-pickers-v7.md

+   // This contains a small behavior change.
+   // If the picker receives an invalid date,
+   // the old value would equal `null`.
+   // the new value would equal the invalid date received.


this feels like something is missing, no?

michelengelen · 2024-12-20T15:18:42Z

docs/data/migration/migration-pickers-v7/migration-pickers-v7.md

+   // This contains a small behavior change.
+   // If the picker receives an invalid date,
+   // the old value would equal `null`.
+   // the new value would equal the invalid date received.


same as above

michelengelen · 2024-12-20T15:25:37Z

packages/x-date-pickers-pro/src/internals/utils/date-range-manager.ts

+  const start = range[0] == null || !utils.isValid(range[0]) ? null : range[0];
+  const end = range[1] == null || !utils.isValid(range[1]) ? null : range[1];


couldn't we reverse the condition here? I did not check, but I think isValid on the adapters already include a if (value == null) return false, no?

Suggested change

const start = range[0] == null || !utils.isValid(range[0]) ? null : range[0];

const end = range[1] == null || !utils.isValid(range[1]) ? null : range[1];

const start = utils.isValid(range[0]) ? range[0] : null;

const end = utils.isValid(range[1]) ? range[1] : null;

michelengelen · 2024-12-20T15:26:01Z

packages/x-date-pickers/src/DatePicker/DatePickerToolbar.tsx

  const translations = usePickerTranslations();
  const ownerState = useToolbarOwnerState();
  const classes = useUtilityClasses(classesProp);

  const dateText = React.useMemo(() => {
-    if (!value) {
+    if (value == null || !utils.isValid(value)) {


same as above ☝🏼

flaviendelangle added breaking change component: pickers This is the name of the generic UI component, not the React module! labels Dec 19, 2024

flaviendelangle self-assigned this Dec 19, 2024

flaviendelangle force-pushed the layout-last-props branch 4 times, most recently from 2f4fb12 to 6fe5cfa Compare December 20, 2024 10:32

flaviendelangle changed the title ~~[pickers] Remove the last props passed to the layout slot~~ [pickers] Use usePickerContext() and usePickerActionsContext() to get the propspassed to the shortcuts and toolbar slots Dec 20, 2024

flaviendelangle force-pushed the layout-last-props branch 3 times, most recently from 59ca6f1 to bfb0786 Compare December 20, 2024 11:24

flaviendelangle commented Dec 20, 2024

View reviewed changes

flaviendelangle force-pushed the layout-last-props branch from bfb0786 to aed6a76 Compare December 20, 2024 11:34

flaviendelangle marked this pull request as ready for review December 20, 2024 11:53

flaviendelangle requested a review from michelengelen December 20, 2024 11:53

flaviendelangle commented Dec 20, 2024

View reviewed changes

[pickers] Remove the last props passed to the layout slot

dcc6d9e

flaviendelangle force-pushed the layout-last-props branch from aed6a76 to dcc6d9e Compare December 20, 2024 13:10

michelengelen reviewed Dec 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pickers] Use `usePickerContext()` and `usePickerActionsContext()` instead of passing props to the `shortcuts` and `toolbar` slots #15948

[pickers] Use `usePickerContext()` and `usePickerActionsContext()` instead of passing props to the `shortcuts` and `toolbar` slots #15948

flaviendelangle commented Dec 19, 2024 •

edited

Loading

mui-bot commented Dec 19, 2024 •

edited

Loading

flaviendelangle Dec 20, 2024 •

edited

Loading

flaviendelangle Dec 20, 2024

michelengelen left a comment

michelengelen Dec 20, 2024

michelengelen Dec 20, 2024

michelengelen Dec 20, 2024

michelengelen Dec 20, 2024

		const start = range[0] == null \|\| !utils.isValid(range[0]) ? null : range[0];
		const end = range[1] == null \|\| !utils.isValid(range[1]) ? null : range[1];

[pickers] Use usePickerContext() and usePickerActionsContext() instead of passing props to the shortcuts and toolbar slots #15948

Are you sure you want to change the base?

[pickers] Use usePickerContext() and usePickerActionsContext() instead of passing props to the shortcuts and toolbar slots #15948

Conversation

flaviendelangle commented Dec 19, 2024 • edited Loading

What?

Changelog

Breaking changes

mui-bot commented Dec 19, 2024 • edited Loading

flaviendelangle Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

flaviendelangle Dec 20, 2024

Choose a reason for hiding this comment

michelengelen left a comment

Choose a reason for hiding this comment

michelengelen Dec 20, 2024

Choose a reason for hiding this comment

michelengelen Dec 20, 2024

Choose a reason for hiding this comment

michelengelen Dec 20, 2024

Choose a reason for hiding this comment

michelengelen Dec 20, 2024

Choose a reason for hiding this comment

[pickers] Use `usePickerContext()` and `usePickerActionsContext()` instead of passing props to the `shortcuts` and `toolbar` slots #15948

[pickers] Use `usePickerContext()` and `usePickerActionsContext()` instead of passing props to the `shortcuts` and `toolbar` slots #15948

flaviendelangle commented Dec 19, 2024 •

edited

Loading

mui-bot commented Dec 19, 2024 •

edited

Loading

flaviendelangle Dec 20, 2024 •

edited

Loading