Input component issues about fluentui HOT 9 CLOSED

Thanks for the response. Good to hear that you are dropping the naming/coding conventions in favour of HTML conventions 👍.

Regarding the components, you can have look at my components (https://github.com/react-fabric/react-fabric/tree/development/src), they're all completely dump and rely only on props (expect SearchBox because the css needs a hover state). I'm also thinking about deprecating the project and start contributing here since they basically accomplish the same thing.

For the onChanged callback, how about renaming it to onValueChange: (value:any) => void and supporting the original onChange: (e:React.InputEvent) => void.

from fluentui.

As @kmees mentioned, the standard onChange: (e:React.InputEvent) => void callback implementation is important. There is no way we can reference the changed element without event: React.InputEvent (or let me know if there is a way).

from fluentui.

Thanks for the feedback, it's helpful to get another pair of eyes on this. I agree with pretty much all of your feedback. A couple of things here:

Regarding "as dumb as possible" 100% yes. We are learning this with many of the controls to extract out dumber sub components; there are a lot of changes coming to reduce/refactor some of these to make cleaner implementations. In some cases we can get away with stateless if we can have React's underlying DOM abstraction do it for us, but not always.

First we are actively cleaning up props that feel "different" from the default HTML attributes without a good reason. Checkbox is definitely an example that hasn't been updated to this standard yet. Originally, Toggle had an isToggled property that we renamed to "checked" to be consistent with the default input HTML element. Toggle has been scrubbed but there are still a few left overs to make consistent; we'll do it.

For background: we have a general javascript coding convention to use the "is, are, should" qualifier prefix for boolean flags, but HTML standards conflict here which is to not use qualifiers. We also have a general rule to use positive variable names like "isVisible" instead of negative "isHidden", but this again conflicts with HTML which uses "disabled" and "hidden" flags. So we've decided that for flags, we will try to follow the HTML convention as much as possible. Not everything has been updated, so if you see contract changes and want to submit an issue, or even better a PR, we'll try to get that patched up.

The onChange vs onChanged naming convention was intentional, but we're open to making it more consistently use onChange. The issue is that in these onChanged callbacks, we're passing back additional metadata to the caller. Toggle for example passes back the checked state. TextField may pass in the value. We don't want the caller to assume ev.target.value is the way to read the value. So by changing the definition we may be changing the expectations. E.g. a user expects onChange to take in a React.InputEvent , but now the event is the 3rd argument. But... well, there is also ambiguity here with onChange vs onChanged so I can see it being confusing either way.

I will look into the controlled/uncontrolled switching. We have a separate issue logged to address and will see if I can scrub these in that process.

Factoring out the validation from text field has definitely been discussed already and agreed on. We should open a separate issue to track this.

from fluentui.

I am currently trying to implement a TextField and I can't figure out how to reference the current element when updating the appropriate values in the state. The old way, like mentioned above, was to use onChange and event.target.name. What is the current workaround for this?

Edit: Thanks to kmees for the workaround.

Create a OnChangeHandler for the TextField component. Like so...

const createOnChangedHandler = (name) => (value) => { /* do something with name and value here, i.e. */ }

Then call the onChangedHandler from your onChanged event.

<TextField onChanged={createOnChangedHandler('foo')} value={/* value for foo */} />

from fluentui.

@gabrielruss did you find a way to do it?

from fluentui.

@notgiorgi Yes, I am currently using the method I described in my previous comment. I believe you could also create a parent component for the TextField (or any component type) and specify a name property on the parent component. Then on the child component's onChanged method, call this.props.onChanged and pass this.props.name. This is a strange method...but it works currently.

Here is the onChanged method I have for my child component.

private _handleChange (value: IChoiceGroupOption) { this.props.onChanged(this.props.name, value.text); }

from fluentui.

@gabrielruss It got fixed #809

from fluentui.

It got fixed in Jan so closing it.

from fluentui.

The above workaround does the job. This is a littler clearer:

handleInputChange = name => value => {
  this.setState({
    [name]: value
  });
}

---

<TextField
  label='Last Name'
  value={this.state.lastName}
  onChanged={this.handleInputChange('lastName')}
/>

from fluentui.