Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Controlled TextInput component breaks Pinyin autocompletion in v0.26.0 #8265

Closed
bnham opened this issue Jun 21, 2016 · 4 comments
Closed

Controlled TextInput component breaks Pinyin autocompletion in v0.26.0 #8265

bnham opened this issue Jun 21, 2016 · 4 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@bnham
Copy link
Contributor

bnham commented Jun 21, 2016

There is a regression in React Native v0.26.0. Specifically, a simple controlled TextInput component (both single and multiline) like this:

var ControlledTextInput = React.createClass({
  getInitialState: function() {
    return {value: ''};
  },
  _onChangeText: function(text) {
    this.setState({value: text});
  },
  render: function() {
    return <TextInput
      value={this.state.value}
      style={styles.default}
      onChangeText={this._onChangeText.bind(this)}
      {...this.props}
      />;
  }
});

breaks the Chinese Pinyin keyboard's autocompletion feature. For instance, after typing the "n" here, the keyboard should offer an autocompletion to "你", but instead the keyboard offers an autocompletion to ",":

pinyin_autocompletion

I bisected and traced this regression down to 9be37ca, which is the commit that updated us to using react 15.0.2-alpha.2.

The reason for the breakage is that after the problematic diff, -[RCTTextView setText:] gets called twice for every keypress: once with the old text and once with the new text with the new character appended. By calling setText: with the old text, we end up calling -[UITextView setText:], which clears the marked autocompletion state of the text view and breaks multi-character autocomplete keyboards like Chinese Pinyin.

See #8140 for a PR that tries to work around this issue. I'm going to close that PR for now and work on trying to fix the regression rather than applying a workaround.

@wasabia
Copy link
Contributor

wasabia commented Jun 21, 2016

will be fixed in v0.28.0?

@bnham
Copy link
Contributor Author

bnham commented Jun 21, 2016

The intention is to fix it as soon as we can. I'm not sure what release the fix will be in until we actually fix it...

@bnham
Copy link
Contributor Author

bnham commented Jun 21, 2016

This commit in react seems to be the root cause of the duplicate -setText: calls: facebook/react#6572

@bnham
Copy link
Contributor Author

bnham commented Jun 23, 2016

It turns out this bug was fixed by another diff: 26aa27d. This diff is included in v0.28.0 so this bug is fixed.

@bnham bnham closed this as completed Jun 23, 2016
@facebook facebook locked as resolved and limited conversation to collaborators Jun 23, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

3 participants