docs/react-v9/contributing/rfcs/react-components/convergence/event-handlers-arguments.md
@dzearing @layershifter
We should provide guidance on how events handlers (onChange, onOpen, etc.) are exposed across components. Today we have a discrepancy between @fluentui/react-northstar (Fluent UI Northstar) and @fluentui/react (Fluent UI v8) components:
// v8
// "newValue" is the new value of the component's main data, depending on the component type
onChange: (ev: React.FormEvent, newValue: ___);
// Northstar
// "data" is props, with the new value mixed in on top
// const data = { ...props, value: newValue }
onChange: (ev: React.FormEvent, data: TProps);
For converged components (Fluent UI vNext) it's implemented in different ways 😪
The primary data has changed, and I need the new value to support controlled component cases or to forward the data to a state management system.
When the data changes I not only need the new value, but I need the original props of the component, so that I can differentiate which instance fired the change event to a general handler.
There are a few types of components which might fire an onChange event:
value = "foo"): Input and Togglevalue = { name: 'foo-item' }): RadioGroup, Selectvalue = ["foo", "bar"]): multiselect Dropdown, DetailsList, TreeWe need to standardize event handlers across components and ensure that API is consistent to avoid confusion for customers. To clarify, we should avoid this case:
// ❌ Each component in this example has completely different signature for event handlers
<>
<Checkbox onChange={(ev: React.FormEvent, newChecked: boolean) => {}} />
<Input onChange={(ev: React.FormEvent, data: { value: string }) => {}} />
</>
Semantic UI React's thread for onChange which provided some background on thinking for Option B below:
https://github.com/Semantic-Org/Semantic-UI-React/issues/623#issuecomment-261018287
Material UI follows a similar approach to v8 code: the second argument represents the new value being changed. See Select API as an example of this. However the components aren't consistent - See Input API or Switch API where neither have a second argument.
The same as "Option A", but without access to props, see sample interfaces below.
null (when an event object is missing)data object where we provide minimum propertiesinterface InputOnChangeData {
value: string;
// other metadata can be included on demand
}
interface CheckboxOnChangeData {
// 👇 The name "value" doesn't always make sense and could possibly be confusing in some cases. E.g. for "Checkbox",
// the "onChange" event happens when the "checked" prop changes, not the "value" prop
checked: string;
}
function App() {
return (
<>
<Input
onChange={(ev: React.ChangeEvent, data: InputOnChangeData) => {
const { value } = data;
// I can access the new value
console.log(`The new value is ${value}`);
}}
/>
<Checkbox onChange={(ev: React.ChangeEvent, data: CheckboxOnChangeData) => console.log(data.checked)} />
</>
);
}
value (or other meaningful property, for example checked) is always predictable in the value propdata object is extendable without ever accidentally overriding a potentially needed prop [value]props are not accessible, but we can reconsider this decision lateronChange handlerFor consistency, all inputs (form controls) should expose a customized onChange handler with the (ev, data) => void signature described above.
We can add a conformance test to verify that an onChange prop exists for all input components. (We already have another test confirming it has the correct signature.) The main challenge for the new test will be whether there's a way to programmatically determine what counts as an input component. If that's not reasonable, we'll have to add a prop to enable the test and ensure it's used in all relevant components.
value (or other meaningful property, for example checked) is always predictable in the value prop, rather than having to know where to look in the event objectevent.target doesn't have a specific enough element type, or the value on the target element has an overly-broad type that doesn't reflect the usageInput, Checkbox, Slider) where the value could be accessed on the underlying <input> as event.target.valuedata object where we provide the following minimum props:| Prop | Description |
|---|---|
active, checked, value, etc. | new value or state for the data of the component |
props | the original props given to the parent component |
Example:
interface InputOnChangeData {
value: string;
props: InputProps;
}
interface CheckboxOnChangeData {
// 👇 The name "value" doesn't always make sense and could possibly be confusing in some cases. E.g. for "Checkbox",
// the "onChange" event happens when the "checked" prop changes, not the "value" prop
checked: string;
props: CheckboxProps;
}
function App() {
return (
<>
<Input
onChange={(ev: React.ChangeEvent, data: InputOnChangeData) => {
const { props, value } = data;
// I can access the new value
console.log(`The new value is ${value}`);
// I can still access the props of the parent component
console.log(`The input (#${props.id}) user's passed in props are ${JSON.stringify(props)}`);
// I can even access additional metadata specific to the change if needed
const { id } = props;
// 😈 I can even access `props.value`
const valueFromProps = props.value;
}}
/>
<Checkbox onChange={(ev: React.ChangeEvent, data: CheckboxOnChangeData) => console.log(data.checked)} />
</>
);
}
value (or other meaningful property, for example checked) is always predictable in the value propprops are accessible and don't overlap other things in the data object[value]data.props[value] and data.props[defaultValue] are accessible, leaving multiple ways to see value which might be confusing as they'll represent the current props rather than the new value. But it should be obvious that these are user inputs and not the new value. Could consider calling new value newValue to be clear, but that seems a bit unpredictable.data.props.iddata is { ...props, value }Example:
const onChange = (ev: React.FormEvent, data: InputProps) => {
const { value } = data;
// I can access the new value.
console.log(`The new value is ${value}`);
// I can still access the props of the parent component.
console.log(`The input (#${id}) user's passed in props are ${JSON.stringify(data)}`);
// I can access additional metadata specific to the change if needed, but it may overwrite
// original props values because of the lack of namepacing.
const { id } = props;
};
value (defaultValue could still be there)index prop of the parent vs index of the new selected item)props value)Example:
const onChange = (ev: React.FormEvent, value: string) => {
// I can access the new value.
console.log(`The new value is ${value}`);
// I can not access the props of the originating component; `ev.target` may handle some of this:
console.log(`The input (#${ev.target.id}) change its value to ${value}.`);
// I can not access additional metadata without baking it into `value`, which may require a breaking change
const { index } = value;
};
TextField uses string for the newValue type.
Toggle uses boolean for the newValue type.
ChoiceGroup uses IChoiceGroupOption as newValue type.
props of parent component from callback, so to do things like sharing one callback implementation which maintains a form hash table requires accessing ev.target.id to know which component changed.index or key is a breaking change. Infinitely expanding callback signatures (new optional params to avoid breaks) as new things are needed. For example, /ComboBox/ComboBox.types.ts in v8:onChange?: (event: React.FormEvent<IComboBox>, option?: IComboBoxOption, index?: number, value?: string) => void;