onChange event for TextField with Select options does not include field name

[gnapse]

On a regular TextField, the onChange event handler receives an event parameter that allows to know the name of the TextField as well as the value that was changed to:

onChange = ({ target: { name, value } }) => {
  console.log('textfield changed', name, value); // -> { 'username': 'john-doe' }
}

However, on a TextField with nested options, which renders internally a Select, this is no longer the case. The element given as target in the onChange event is the li of the dropdown list, which only has a value, but no name.

• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

The onChange event handler of a <TextField name="xyz">{options}</TextField> should receive an event object where event.target.name is "xyz".

Current Behavior

The onChange event handler of a <TextField name="xyz">{options}</TextField> receives an event object where event.target.name is undefined.

Steps to Reproduce (for bugs)

1. Visit https://codesandbox.io/s/nkq35z76wj
2. Open the codesandbox.io console on the right bottom area of the page
3. Change the name text field, see that it changes visually, and the name and value are present in the console
4. Now change the gender select field. See that it does not change, and the console prints out undefined in place of the actual field name.

Context

Having the name reflected in the event target is not only more compatible with how actual form input events work in html, it is also convenient because you can use the same onChange event handler for many input elements, instead of dynamically having to generate a different event handler for different input elements of a form.

Your Environment

Tech	Version
Material-UI	1.0.0-beta.17
React	16.0.0
browser	Chrome latest

[mbrookes]

I've just noticed that callback signatures aren't documented in the individual component API docs - it just says "signature", which is a bit meaningless. Worse, someone appears to have stripped the @param tags out of the source code!

[oliviertassinari]

@gnapse The event of the onChange callback is coming from a click event. The target is not supposed to have a name attribute. As far as I'm concerned, the current behavior is the expected one. I understand your simplification use case, but I don't see how it could be handled by Material-UI core. Notice that your expected behavior is the current behavior of the native mode of the Select.

@mbrookes Is right. I should have added the onChange API description when migrating the component. It's missing!

[gnapse]

@oliviertassinari ok, I get now that using the native select it will work as I expect, and therefore I'll be using that.

But just to be clear, for the other case, are you saying this does not need to be fixed? If the same component, TextField, has an onChange event that behaves differently in both different uses, I think that's confusing. I realized too that the actual event I'm getting is a mouse click event, but that is wrong. Internally there's a mouse click, but the handler is a onChange event handler. It does not care about the li that was clicked, it cares about the input value change, what was changed, and how it was changed.

[oliviertassinari]

@gnapse I'm saying that the documentation needs to be improved.

I think that's confusing

Yes, it's. I wish we could fix it, but I don't see how. It's structural to not using a native select.

[oliviertassinari]

Oh wait, I have forgotten that we are already playing with 🔥 with the onChange(event). It should be as easy as adding name here:
https://github.com/callemall/material-ui/blob/cdb98335169d3d740beb389f4b4707ac999551c1/src/Select/SelectInput.js#L157

[gnapse]

Good to know. In any case I'm leaning to always using these TextField-with-options as a native select. It integrates better with mobile, and now also this event signature mismatch compared to normal form input fields. But I'm happy if this is eventually fixable and fixed.

[mbrookes]

@oliviertassinari I was mistaken about @param having been removed, when I saw it was missing from Chip, which I knew had migrated it for, I did a global search, but had a previous file-type filter that meant it didn't return any results. 😊

I'd still like to know what the plan is for standardizing the ordering of the callback signature. It's a shame @alitaheri's proposal (#2957) wasn't implemented from the start for v1... but it isn't too late!

[oliviertassinari]

@mbrookes I'm not sure to understand your point about standardizing the callbacks arguments. I have tried to make them as standard as possible so far. Do you see some issues?

[mbrookes]

@oliviertassinari I hadn't noticed that we're adding children to the event to propagate additional attributes, rather than passing multiple args to the callback function. That's smart. 😄

I guess it might be helpful to document those?

[oliviertassinari]

@mbrookes I'm not sure how we can improve the docs. If you see, go ahead :). I have submitted a PR to add the name.