Skip to content

Commit

Permalink
Fix controlled <TextInput> on iOS when inputting in Chinese/Japanese
Browse files Browse the repository at this point in the history
Summary:
@public
This should fix #18403.
When the user is inputting in Chinese/Japanese with <TextInput> in a controlled manner, the RCTBaseTextInputView will compare the JS-generated attributed string against the TextInputView attributed string and repeatedly overwrite the TextInputView one. This is because the native TextInputView will provide extra styling to show that some text is provisional.
My solution is to do a plain text string comparison at this point, like how we do for dictation.

Expected behavior when typing in a language that has "multistage" text input: For instance, in Chinese/Japanese it's common to type out the pronunciation for a word and then choose the appropriate word from above the keyboard. In this model, the "pronunciation" shows up in the text box first and then is replaced with the chosen word.
Using the word Japan which is written 日本 but first typed as にほん. It takes 4 key-presses to get to 日本, since に, ほ, ん, are all typed and then 日本 is selected. So here is what should happen:

1. enter に, onChange fires with 'に', markedTextRange covers 'に'
2. enter ほ, onChange fires with 'にほ', markedTextRange covers 'にほ'
3. enter ん, onChange fires with 'にほん', markedTextRange covers 'にほん'
4. user selects 日本 from the menu above the keyboard (provided by the keyboard/OS), onChange fires with '日本', markedTextRange is removed

previously we were overwriting the attributed text which would remove the markedTextRange, preventing the user from selecting 日本 from above the keyboard.

Cheekily, I've also fixed an issue with secure text entry as it's the same type of problem.

Reviewed By: PeteTheHeat

Differential Revision: D9002295

fbshipit-source-id: 7304ede055f301dab9ce1ea70f65308f2a4b4a8f
  • Loading branch information
Mehdi Mulani authored and facebook-github-bot committed Jul 30, 2018
1 parent bda84a3 commit 892212b
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 16 deletions.
18 changes: 13 additions & 5 deletions Libraries/Text/TextInput/RCTBaseTextInputView.m
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,19 @@ - (NSAttributedString *)attributedText
}

- (BOOL)textOf:(NSAttributedString*)newText equals:(NSAttributedString*)oldText{
UITextInputMode *currentInputMode = self.backedTextInputView.textInputMode;
if ([currentInputMode.primaryLanguage isEqualToString:@"dictation"]) {
// When the dictation is running we can't update the attibuted text on the backed up text view
// because setting the attributed string will kill the dictation. This means that we can't impose
// the settings on a dictation.
// When the dictation is running we can't update the attibuted text on the backed up text view
// because setting the attributed string will kill the dictation. This means that we can't impose
// the settings on a dictation.
// Similarly, when the user is in the middle of inputting some text in Japanese/Chinese, there will be styling on the
// text that we should disregard. See https://developer.apple.com/documentation/uikit/uitextinput/1614489-markedtextrange?language=objc
// for more info.
// Lastly, when entering a password, etc., there will be additional styling on the field as the native text view
// handles showing the last character for a split second.
BOOL shouldFallbackToBareTextComparison =
[self.backedTextInputView.textInputMode.primaryLanguage isEqualToString:@"dictation"] ||
self.backedTextInputView.markedTextRange ||
self.backedTextInputView.isSecureTextEntry;
if (shouldFallbackToBareTextComparison) {
return ([newText.string isEqualToString:oldText.string]);
} else {
return ([newText isEqualToAttributedString:oldText]);
Expand Down
60 changes: 49 additions & 11 deletions RNTester/js/TextInputExample.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,48 @@ class RewriteExampleInvalidCharacters extends React.Component<
}
}

class RewriteExampleKana extends React.Component<$FlowFixMeProps, any> {
constructor(props) {
super(props);
this.state = {text: ''};
}
render() {
return (
<View style={styles.rewriteContainer}>
<TextInput
multiline={false}
onChangeText={text => {
this.setState({text: text.replace(//g, '日')});
}}
style={styles.default}
value={this.state.text}
/>
</View>
);
}
}

class SecureEntryExample extends React.Component<$FlowFixMeProps, any> {
constructor(props) {
super(props);
this.state = {text: ''};
}
render() {
return (
<View>
<TextInput
secureTextEntry={true}
style={styles.default}
defaultValue="abc"
onChangeText={text => this.setState({text})}
value={this.state.text}
/>
<Text>Current text is: {this.state.text}</Text>
</View>
);
}
}

class TokenizedTextExample extends React.Component<$FlowFixMeProps, any> {
constructor(props) {
super(props);
Expand Down Expand Up @@ -524,6 +566,12 @@ exports.examples = [
return <RewriteExampleInvalidCharacters />;
},
},
{
title: 'Live Re-Write (ひ -> 日)',
render: function() {
return <RewriteExampleKana />;
},
},
{
title: 'Keyboard Accessory View',
render: function() {
Expand Down Expand Up @@ -677,17 +725,7 @@ exports.examples = [
{
title: 'Secure text entry',
render: function() {
return (
<View>
<WithLabel label="true">
<TextInput
secureTextEntry={true}
style={styles.default}
defaultValue="abc"
/>
</WithLabel>
</View>
);
return <SecureEntryExample />;
},
},
{
Expand Down

10 comments on commit 892212b

@johnxie
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@jamsch
Copy link
Contributor

@jamsch jamsch commented on 892212b Jul 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I hope this is cherry picked in the next release.

@soyanakagawa
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@elliscwc
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you 💛

@watanabeyu
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@hankzhuo
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks , it helps

@imaimiami
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great 👍

@hernancorigliano
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Kennytian
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, man! Thanks do this for Chinese input.

@aiynmm
Copy link

@aiynmm aiynmm commented on 892212b Nov 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's awesome! I have seen this in v0.57.0! Thanks!

Please sign in to comment.