From c8108bdbe13590660d7f575d0747e100ea7bb866 Mon Sep 17 00:00:00 2001 From: Brent Vatne Date: Mon, 14 Dec 2015 06:41:45 -0800 Subject: [PATCH] Fixed controlled component on iOS and remove unnecessary code Summary: Closes #4290 `mostRecentEventCount` was always being set after `text` on iOS, so let's be really explicit about the order here as we were doing on Android: always call `setNativeProps` providing the `mostRecentEventCount` before we call `onChange` or `onChangeText`. I also ripped out storing `mostRecentEventCount` in the state, which isn't necessary since we're always doing it through `setNativeProps`. Closes https://github.com/facebook/react-native/pull/4588 Reviewed By: svcscm Differential Revision: D2754565 Pulled By: nicklockwood fb-gh-sync-id: a1401f39b4e19248095517c2a3503cd2af59fa47 --- Examples/UIExplorer/TextInputExample.ios.js | 27 +++++++++++++ Libraries/Components/TextInput/TextInput.js | 45 ++++++++------------- 2 files changed, 43 insertions(+), 29 deletions(-) diff --git a/Examples/UIExplorer/TextInputExample.ios.js b/Examples/UIExplorer/TextInputExample.ios.js index 2479e1d61..f8324c51a 100644 --- a/Examples/UIExplorer/TextInputExample.ios.js +++ b/Examples/UIExplorer/TextInputExample.ios.js @@ -125,6 +125,27 @@ class RewriteExample extends React.Component { } } +class RewriteExampleInvalidCharacters extends React.Component { + constructor(props) { + super(props); + this.state = {text: ''}; + } + render() { + return ( + + { + this.setState({text: text.replace(/\s/g, '')}); + }} + style={styles.default} + value={this.state.text} + /> + + ); + } +} + class TokenizedTextExample extends React.Component { constructor(props) { super(props); @@ -313,6 +334,12 @@ exports.examples = [ return ; } }, + { + title: "Live Re-Write (no spaces allowed)", + render: function() { + return ; + } + }, { title: 'Auto-capitalize', render: function() { diff --git a/Libraries/Components/TextInput/TextInput.js b/Libraries/Components/TextInput/TextInput.js index 9f8f44ed7..fe62ec7fb 100644 --- a/Libraries/Components/TextInput/TextInput.js +++ b/Libraries/Components/TextInput/TextInput.js @@ -329,12 +329,6 @@ var TextInput = React.createClass({ React.findNodeHandle(this.refs.input); }, - getInitialState: function() { - return { - mostRecentEventCount: 0, - }; - }, - contextTypes: { onFocusRequested: React.PropTypes.func, focusEmitter: React.PropTypes.instanceOf(EventEmitter), @@ -431,7 +425,6 @@ var TextInput = React.createClass({ onSelectionChange={onSelectionChange} onSelectionChangeShouldSetResponder={emptyFunction.thatReturnsTrue} text={this._getText()} - mostRecentEventCount={this.state.mostRecentEventCount} />; } else { for (var propKey in notMultiline) { @@ -460,7 +453,6 @@ var TextInput = React.createClass({ ref="input" {...props} children={children} - mostRecentEventCount={this.state.mostRecentEventCount} onFocus={this._onFocus} onBlur={this._onBlur} onChange={this._onChange} @@ -506,7 +498,7 @@ var TextInput = React.createClass({ textAlign={textAlign} textAlignVertical={textAlignVertical} keyboardType={this.props.keyboardType} - mostRecentEventCount={this.state.mostRecentEventCount} + mostRecentEventCount={0} multiline={this.props.multiline} numberOfLines={this.props.numberOfLines} maxLength={this.props.maxLength} @@ -548,29 +540,24 @@ var TextInput = React.createClass({ }, _onChange: function(event: Event) { - if (Platform.OS === 'android') { - // Android expects the event count to be updated as soon as possible. - this.refs.input.setNativeProps({ - mostRecentEventCount: event.nativeEvent.eventCount, - }); - } + // Make sure to fire the mostRecentEventCount first so it is already set on + // native when the text value is set. + this.refs.input.setNativeProps({ + mostRecentEventCount: event.nativeEvent.eventCount, + }); + var text = event.nativeEvent.text; - var eventCount = event.nativeEvent.eventCount; this.props.onChange && this.props.onChange(event); this.props.onChangeText && this.props.onChangeText(text); - this.setState({mostRecentEventCount: eventCount}, () => { - // NOTE: this doesn't seem to be needed on iOS - keeping for now in case it's required on Android - if (Platform.OS === 'android') { - // This is a controlled component, so make sure to force the native value - // to match. Most usage shouldn't need this, but if it does this will be - // more correct but might flicker a bit and/or cause the cursor to jump. - if (text !== this.props.value && typeof this.props.value === 'string') { - this.refs.input.setNativeProps({ - text: this.props.value, - }); - } - } - }); + + // This is necessary in case native updates the text and JS decides + // that the update should be ignored and we should stick with the value + // that we have in JS. + if (text !== this.props.value && typeof this.props.value === 'string') { + this.refs.input.setNativeProps({ + text: this.props.value, + }); + } }, _onBlur: function(event: Event) {