Some components have an onChange prop e.g. Searchbox, Slider, Checkbox while others use onChanged e.g. TextField, Toggle, DropDown
Prop types are not consistent
Prop types should be consistent
From recent PRs, like #817, it looks like onChange
is the preferred name. Let's deprecate any remaining instances of onChanged
and replace them with onChange
.
Yes. onChange
with ev being the first param and custom things after, all as optionals.
This confused me just now with the TextField because I was trying to use the onChange event, which doesn't get fired. It's also not a compilation error because it exists intrinsically.
@scriby I am having the same issue here. I wanna get the attribute name and the value from the TextField react component, but the onChange is not firing
I have got the same issue. OnChange is not firing. Instead onChanged is firing but only with the entered text as parameter.
Returning the component proxy would be better.
Any update on this?
I'm looking into the best way to do this without adding breaking changes. I'm thinking: on the components that accept onChanged
adding onChange
that falls back to onChanged
, then deprecating onChanged
for next major release.
@jordandrako thanks!
My first thoughts were:
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions to Fabric React!
We should fix this!
This issue has been automatically marked as stale because it has not activity for 30 days. It will be closed if no further activity occurs within 14 days of this comment. Thank you for your contributions to Fabric React!
Why am I receiving this notification?
I did a quick scan on which components use onChange
:
| Name |Signature |
|-------|----------|
| Checkbox | onChange: (ev, checked) => void;
|
| ChoiceGroup | onChange: (ev, option) => void;
|
| pickers | onChange: (items) => void;
|
| SearchBox | onChange: (newValue) => void;
|
| Slider | onChange: (value) => void
|
It seems that onChange
parameters are not consistently ev, value
. This is makes things rather confusing. We should stick with one or the other...
Regarding non-deprecated onChanged
instances to be cleaned up:
| Name |Signature |
|-------|----------|
| ColorSlider | onChanged: (newValue) => void
|
| ComboBox | onChanged: (option, index, value, submitPendingValueEvent) => void
|
| Dropdown | onChanged: (option, index) => void
|
| Rating | onChanged: (rating) => void
|
| TextField | onChanged: (newValue) => void
|
| Toggle | onChanged: (checked) => void
|
| SelectableDroppableText | onChanged: (option, index) => void` |
So the work here:
onChange(ev, value)
to components missing it, deprecate onChanged
.onChange
without ev
, this is way more sensitive. That would be a major breaking change. We can consider a codemod and major release for this.
Most helpful comment
@scriby I am having the same issue here. I wanna get the attribute name and the value from the TextField react component, but the onChange is not firing