Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pickers] Create a single setValue method used by all the parts of the picker to update the value #16059

Open
flaviendelangle opened this issue Jan 2, 2025 · 0 comments
Assignees
Labels
breaking change component: pickers This is the name of the generic UI component, not the React module!

Comments

@flaviendelangle
Copy link
Member

flaviendelangle commented Jan 2, 2025

Problem

Currently, we have tons of way to update the value of the picker, each with its own signature:

  • View update:

    const setValueFromView = (newValue: TValue, selectionState: 'partial' | 'shallow' | 'finish' = 'partial') => { /** ... */ };
  • Field update:

    const setValueFromField = (newValue: TValue, context: { validationError: TError }) => { /** ... */ };
  • Action update:

    const actions.onClear = () => { /** ... */ };
    const actions.onAccept = () => { /** ... */ };
    const actions.onDismiss = () => { /** ... */ };
    const actions.onCancel =  () => { /** ... */ };
    const actions.onSetToday =  () => { /** ... */ };
  • Update from the toolbar (same as the view update)

    const setValueFromToolbar = (newValue: TValue, selectionState: 'partial' | 'shallow' | 'finish' = 'partial') => { /** ... */ };
  • Update from the shortcut

    const setValueFromShortcut = (newValue: TValue, changeImportance: 'set' | 'accept', shortcut: PickersShortcutsItemContext) => { /** ... */ }

Proposal

Create a single method setValue that would be exposed by the picker context and that would replace all of the current
methods.
It would have an options methods that allow to handle all the behaviors currently supported:

const setValue = (newValue, options: SetValueActionsOptions<TError>) => { /** ... */ };

export interface SetValueActionOptions<TError = string> {
  changeImportance?: PickerChangeImportance;
  validationError?: TError;
  shortcut?: PickersShortcutsItemContext;
  skipPublicationIfPristine?: boolean;
}

The exact shape of SetValueActionOptions might evolve as we migrate all the methods, but the current one seems to solve all the use cases (except the shallow update of the Time Clock, but I don't think it should use setValue).

Side notes

  • Some methods can be replaced on the public API during the alpha, other can be implemented internally using setValue but keep there current signature depending on the migration pace.

  • The actions (onClear, onAccept, ...) are still exposed because people currently don't have a way to access the various date in the state (the last published value, the last comitted value, ...) so they can't reproduce their behavior. But those methods all take 0 parameter so it's not a problem for the consistency of our API. We can discuss later if we prefer to always enforce the usage of setValue and provide the tools to do it.

Excluded work

This issue does not cover any change to the onChange props of the component that can be used as standalone.
If a component cannot be used as standalone (e.g: the toolbar and shortcuts slots), then they always use setValue from the context and don't have an onChange prop anymore.

But if they can be used as standalone (e.g: <DateCalendar />, <TimField />, etc...), then they need to still have an onChange prop.

I would like to align all those onChange prop to be similar to the onChange of the picker (like the field is today), but it's outside of the scope of this issue and can wait for v9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: pickers This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

1 participant