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.

Sign up for GitHub

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

Jump to bottom

[0.26.0-rc] autocorrect broken #7496

Closed
sjmueller opened this issue May 10, 2016 · 9 comments
Closed

[0.26.0-rc] autocorrect broken #7496

sjmueller opened this issue May 10, 2016 · 9 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@sjmueller
Copy link
Contributor

sjmueller commented May 10, 2016 •
edited
Loading

When setting up a TextInput to keep in sync with state (as per documentation), autocorrect breaks. For example:

<TextInput
  style={styles.title}
  placeholder="Title"
  onChangeText={title => this.setState({title})}
  value={this.state.title}
/>

It seems that the value of the native control is now getting overridden during every onChangeText event that updates state, because even things like double space no longer results in a period.

autocorrect

For reference, I'm also on react 15.0.2
@sjmueller
Copy link
Contributor Author

Did some investigation, and found this commit:

Fix autocomplete in rich-text editing mode for CJK text input 91dcc9a

I rolled these changes out of my build, but still looks like the same problems persist. Any suggestions?

@mpretty-cyro
Copy link

@sjmueller It looks like the 'setText:' method of RCTTextField is getting called twice, once with the old value then again with the new value. Having it set twice is what is causing the problem.

@mpretty-cyro
Copy link

Actually, it looks like the problem is this if statement

react-native/Libraries/Components/TextInput/TextInput.js

Line 619 in dd8caf4

if (text !== this.props.value && typeof this.props.value === 'string') {

@mpretty-cyro
Copy link

@nicklockwood It seems to me that autocorrect could never work if the TextInput is a controlled component. Unfortunately I can't think of a nice way to fix this without having something return that the text change is valid and hence shouldn't call setNativeProps. Do you have any ideas which might help?

@mpretty-cyro mpretty-cyro mentioned this issue May 20, 2016
Closed
@mpretty-cyro
Copy link

@sjmueller I've created a PR which seems to fix the autocorrect on iOS (haven't tested the effects on Android yet). If you decide to try it out then let me know if this introduces any bug.

@rigdern
Copy link
Contributor

rigdern commented May 22, 2016 •
edited
Loading

This is a regression caused by a change to batch event handling in React Native: facebook/react@9f11f8c

cc @spicyj

@sophiebits
Copy link
Contributor

Not at a computer, but you can look at a recent change I did to PickerAndroid to fix a similar issue.

rigdern pushed a commit to rigdern/react-native that referenced this issue May 22, 2016
Fix TextInput autocorrect (facebook#7496)
a801020 
TextInput autocorrect was broken by a change to batch event handling in React Native:
facebook/react@9f11f8c

This fix uses the same approach as
facebook@0cd2904

The problem is that TextInput's _onChange handler relied on this.props.value
being updated synchronously when calling this.props.onChangeText(text). However,
this assumption was broken when React Native event handling started being batched.

The fix is to move the code that relies on this.props.value being up-to-date to
componentDidUpdate.
ghost pushed a commit that referenced this issue May 26, 2016
Fix TextInput autocorrect (#7496)
26aa27d 
Summary:
Autocorrect was broken for controlled TextInput components by a change to batch event handling in React Native:
facebook/react@9f11f8c

For example, a TextInput like this would be affected by this bug:

```javascript
<TextInput
  autoCorrect={true}
  style={{height: 26, width: 100}}
  onChangeText={(text) => this.setState({ text })}
  value={this.state.text}
/>
```
This fix uses the same approach as
0cd2904

The problem is that TextInput's _onChange handler relied on this.props.value being updated synchronously when calling this.props.onChangeText(text). However, this assumption was broken when React Native event handling started being batched.

The fix is to move the code that relies on this.props.value being up-to-date to componentDidUpdate.

**Test plan (required)**

Tested autocorrect now works on iOS in a small app and a large app. Also tested t
Closes #7676

Differential Revision: D3346221

Pulled By: nicklockwood

fbshipit-source-id: 715df3e8a03aa58cb0a462de4add02289d42782f
@rigdern
Copy link
Contributor

rigdern commented May 26, 2016

@nicklockwood Should this issue be closed since you merged my PR? 26aa27d

@mpretty-cyro
Copy link

I can confirm the 26aa27d fix works for me as well.

@gcanti gcanti mentioned this issue May 30, 2016
Closed
bubblesunyum pushed a commit to iodine/react-native that referenced this issue Aug 23, 2016
@bubblesunyum
Fix TextInput autocorrect (facebook#7496)
6df0270 
Summary:
Autocorrect was broken for controlled TextInput components by a change to batch event handling in React Native:
facebook/react@9f11f8c

For example, a TextInput like this would be affected by this bug:

```javascript
<TextInput
  autoCorrect={true}
  style={{height: 26, width: 100}}
  onChangeText={(text) => this.setState({ text })}
  value={this.state.text}
/>
```
This fix uses the same approach as
facebook@0cd2904

The problem is that TextInput's _onChange handler relied on this.props.value being updated synchronously when calling this.props.onChangeText(text). However, this assumption was broken when React Native event handling started being batched.

The fix is to move the code that relies on this.props.value being up-to-date to componentDidUpdate.

**Test plan (required)**

Tested autocorrect now works on iOS in a small app and a large app. Also tested t
Closes facebook#7676

Differential Revision: D3346221

Pulled By: nicklockwood

fbshipit-source-id: 715df3e8a03aa58cb0a462de4add02289d42782f
mpretty-cyro pushed a commit to HomePass/react-native that referenced this issue Aug 25, 2016
Fix TextInput autocorrect (facebook#7496)
1fcd1f8 
Summary:
Autocorrect was broken for controlled TextInput components by a change to batch event handling in React Native:
facebook/react@9f11f8c

For example, a TextInput like this would be affected by this bug:

```javascript
<TextInput
  autoCorrect={true}
  style={{height: 26, width: 100}}
  onChangeText={(text) => this.setState({ text })}
  value={this.state.text}
/>
```
This fix uses the same approach as
facebook@0cd2904

The problem is that TextInput's _onChange handler relied on this.props.value being updated synchronously when calling this.props.onChangeText(text). However, this assumption was broken when React Native event handling started being batched.

The fix is to move the code that relies on this.props.value being up-to-date to componentDidUpdate.

**Test plan (required)**

Tested autocorrect now works on iOS in a small app and a large app. Also tested t
Closes facebook#7676

Differential Revision: D3346221

Pulled By: nicklockwood

fbshipit-source-id: 715df3e8a03aa58cb0a462de4add02289d42782f
dryganets pushed a commit to dryganets/react-native that referenced this issue Oct 3, 2017
Fix TextInput autocorrect
d4f9a46 
TextInput autocorrect was broken by a change to batch event handling in React Native:
facebook/react@9f11f8c

This fix uses the same approach as
facebook@0cd2904

The problem is that TextInput's _onChange handler relied on this.props.value
being updated synchronously when calling this.props.onChangeText(text). However,
this assumption was broken when React Native event handling started being batched.

The fix is to move the code that relies on this.props.value being up-to-date to
componentDidUpdate.

Fixes Skype4Life bug:
https://skype.visualstudio.com/DefaultCollection/SCC/Skype4Life/_workitems?id=554823&triage=true&_a=edit

Fixes React Native bug:
facebook#7496
@danielsuo danielsuo mentioned this issue Mar 16, 2018
Closed
@facebook facebook locked as resolved and limited conversation to collaborators May 30, 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.
Assignees
No one assigned
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@sophiebits @sjmueller @rigdern @nicklockwood @mpretty-cyro @react-native-bot