search

halilb / react-native-textinput-effects

Text inputs with custom label and icon animations for iOS and android. Built with react native and inspired by Codrops.
MIT License
2.98k stars 291 forks source link

Cannot input for the uncontrolled components [0.1.2] #4

Closed mijia closed 8 years ago

mijia commented 8 years ago

Hi, I think there is a problem with the new 0.1.2 release for the uncontrolled components. With uncontrolled components I don't think the value props would be null or undefined in the componentWillReceiveProps api call which would be set as the new state of the text input.

I have made the comment in the commit. https://github.com/halilb/react-native-textinput-effects/commit/8e5e4c8494ee0314b1e9dcccb98cfacf9ff83dd1

Thank you very much.

halilb commented 8 years ago

Hi @mijia, thanks for your time and contribution.

So, you are passing value props to the input and the value you're using might be stale? I see your point if my assumption is true.

<Hoshi
   value={this.props.textValue}
/>

Would replacing line 48 with this help fixing the problem?

if (this.props.value !== newValue && newValue !== this.state.value) {

mijia commented 8 years ago

Hi @halilb , thank you very much for the time and awesome project.

According to the doc https://facebook.github.io/react/docs/forms.html,

An without a value property is an uncontrolled component: An uncontrolled component maintains its own internal state.

In textinput-effects: BaseInput, the TextInput has been a controlled component, but I think it should also provide uncontrolled approach to use as the doc described. Before 0.1.2, that is OK, we just use <Fumi label=... > without the value props since we don't want to control the state. But in 0.1.2, the componentWillReceiveProps includes some logic make the uncontrolled component broke, since the newProps.value === undefined and newProps.value !== this.state.value, so a undefined value would be set as the internal state which breaks the things.

The code you mentioned seems like to solve this, but I think if (newProps.hasOwnProperty('value') && newValue !== this.state.value) would make more sense for the uncontrolled input components.

Thank you very much.

halilb commented 8 years ago

Thanks for the clear explanation @mijia. I wasn't aware of controlled and uncontrolled components concept.

I followed your advice and pushed 57e9380. I'm also checking defaultProps to support uncontrolled components which has an initial value.

mijia commented 8 years ago

Awesome. I will verify this when new version pushed to the NPM. Thank you very much, 👍

halilb commented 8 years ago

I just published version 0.1.3 to NPM. Can you let me know when you verify?

mijia commented 8 years ago

I just verified, thank you very much. Both working for the controlled and uncontrolled components. Will close this.