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

[iOS][TextInput] Fix controlled TextInput for Chinese (and other languages) #18456

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
@magicien
Contributor

magicien commented Mar 20, 2018

Motivation

Fix #18403. It should support multistage text input (marked text).

In v0.54.2, when TextInput component has defaultValue or value prop, marked text is confirmed as soon as typed, and it doesn't show candidate words.

Test Plan

  • All tests of yarn run test are passed
  • Test with the sample app 1.
    • It worked. (It shows candidate words corresponding to the marked text)

react-native 0.54.2 (with bug) (from #18403 @danielsuo)

app1_before

with bugfix

app1_after

  • Test with the sample app 2.
    • It worked with both singleline and multiline TextInput.
    • When blur event was emitted, the marked text was confirmed. (#12599)

react-native 0.54.2 (with bug)

app2_before

with bugfix

app2_after

Release Notes

[IOS] [BUGFIX] [TextInput] - Fix controlled TextInput for Chinese (and other languages)

magicien added some commits Mar 8, 2018

Add text property to RCTBaseTextInputView
- Add text VIEW_PROPERTY to RCTBaseTextInputViewManager.m
- Add text accessor to RCTBaseTextInputView.m
Change textInputDidChange handling
Change textInputDidChange handling not to send onChange event while building Chinese characters

@magicien magicien changed the title from [WIP][iOS][TextInput] Fix controlled TextInput for Chinese (and other languages) to [iOS][TextInput] Fix controlled TextInput for Chinese (and other languages) Mar 21, 2018

magicien added a commit to CHANOMA/react-native that referenced this pull request Mar 21, 2018

yuya-fujimoto added a commit to CHANOMA/react-native that referenced this pull request Mar 23, 2018

Merge pull request #3 from CHANOMA/feature/fix_textinput_bugs
Apply facebook#18456 Fix controlled TextInput for Chinese (and other languages)

chrisnojima added a commit to keybase/react-native that referenced this pull request Mar 27, 2018

vshia pushed a commit to humandx/react-native that referenced this pull request Apr 2, 2018

yuya-fujimoto added a commit to CHANOMA/react-native that referenced this pull request Apr 8, 2018

@johnxie

This comment has been minimized.

johnxie commented Apr 16, 2018

Hi

Is this resolved in the latest release?

erikchan002 pushed a commit to erikchan002/react-native that referenced this pull request Apr 17, 2018

@rplankenhorn

This comment has been minimized.

rplankenhorn commented Apr 18, 2018

I was looking into this fix as a possible fix for #18874.

The problem is that if you are using onChangeText to do text validation, it will briefly show the undesired character before removing it from the TextInput.

I don't know much about the React Native source code but after taking a brief look at the code, I think there might be a problem in textInputShouldChangeTextInRange in RCTBaseTextInputView.m. I think we should be preventing the text from being added until it comes in via the prop. Not entirely sure.

@bunnyc1986

This comment has been minimized.

bunnyc1986 commented Apr 19, 2018

What's the plan merging this PR?

@rplankenhorn

This comment has been minimized.

rplankenhorn commented Apr 19, 2018

@bunnyc1986 Given my above comment, I don't think this should be merged in at all. It may fix #18403 but it causes more issues for #18874.

@facebook-github-bot

This comment has been minimized.

facebook-github-bot commented Apr 19, 2018

@magicien I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

@ssssssssssss

This comment has been minimized.

ssssssssssss commented Apr 20, 2018

iwasaki-d added a commit to iwasaki-d/react-native that referenced this pull request Apr 22, 2018

Update RCTBaseTextInputShadowView.m
[iOS][TextInput] Fix controlled TextInput for Chinese (and other languages) facebook#18456
@jamsch

This comment has been minimized.

Contributor

jamsch commented Apr 23, 2018

Confirmed. React Native treats every text entered separately which shouldn't be the case for languages like Chinese and Japanese since there's a large amount of characters that just won't fit on a keyboard.

In the instance of Japanese, you're only able to write in the single letter phonetics. For example:

  • a => あ
  • i => い
  • u => う
  • ka => kあ - supposed to be:
  • shi => shい - supposed to be
  • ku => kう - supposed to be
  • tsu => tsう - supposed to be

Manually applying the fix above (from @iwasaki-d) seems to have solved the issue.

@maharjanaman

This comment has been minimized.

maharjanaman commented Jun 12, 2018

@watanabeyu the workaround is great. However I'm unable to implement it with redux-form. Is there a way to do this with redux-form?

@chrisnojima chrisnojima referenced this pull request Jun 22, 2018

Closed

WIP: rn56 #12477

0 of 9 tasks complete

chrisnojima added a commit to keybase/react-native that referenced this pull request Jun 22, 2018

@hramos hramos requested a review from shergin Jun 28, 2018

@elliscwc

This comment has been minimized.

elliscwc commented Jul 3, 2018

hey guys, is this going to be reviewed and merged?

rodolphefauquez pushed a commit to Clintal/react-native that referenced this pull request Jul 5, 2018

@chrisnojima chrisnojima referenced this pull request Jul 5, 2018

Closed

NOMERGE: Keybase fixes off 560 #5

2 of 2 tasks complete
@huowenxuan

This comment has been minimized.

huowenxuan commented Jul 13, 2018

@lhcn
设置Info.plist->Localization native development region,改为cn

hisokakei pushed a commit to hisokakei/react-native that referenced this pull request Jul 15, 2018

Kevin
Fix webSocket idle force close
facebook#19489

Fix TextInput chinese char input issue
facebook#18456
@zerosrat

This comment has been minimized.

zerosrat commented Jul 16, 2018

I modify files in node_modules/react-native/... as file changes in this pull request, but the bug is still here. I wonder what I did wrong.
react-native: 0.55.4

@roycclu

This comment has been minimized.

roycclu commented Jul 17, 2018

@magicien thanks this works magically!
@zerosrat did you recompile the app after making the changes?

@zerosrat

This comment has been minimized.

zerosrat commented Jul 17, 2018

@roycclu I did not compile react native code after I modify. Should I follow this guide to make it work? Thanks for your reply!

@codegoblins

This comment has been minimized.

codegoblins commented Jul 18, 2018

@zerosrat if you are using react native in your project you can create a postinstall script in package.json that applies this PR as a patch, then you can commit it to your project.

@randomor

This comment has been minimized.

randomor commented Jul 23, 2018

I've integrated this fix. It is an improvement, however, since the TextInput value could sometimes not equal to what's displayed on the screen, it will result in this undesirable bug, e.g.:
img_4322

When I press the plus button at this point (which normally should log the text in input and then set the text value to "" right away), I will only get "我正在“, not “我正在tijiao” as I would expect, and to make things worse, the text input is not cleared, and "我正在tijiao“ is still left in the input.

@sunnylqm

This comment has been minimized.

Contributor

sunnylqm commented Jul 24, 2018

@facebook-github-bot

@mmmulani has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@mmmulani

This comment has been minimized.

Contributor

mmmulani commented Jul 25, 2018

@randomor thanks for that example. Could you provide it as a Expo Snack or change to RNTester? I'm trying to fix this and want to make sure this problem is also solved

@dwyanelin

This comment has been minimized.

dwyanelin commented Jul 26, 2018

inspired by watanabeyu.

`
import React, {Component} from 'react';
import {TextInput, Platform} from 'react-native';

export default class MyTextInput extends Component{
state={
fakeValue:this.props.value,//pure display here
refresh:false,//right clear
}

componentDidUpdate(prevProps){//right clear
	if(prevProps.value!==this.props.value&&this.props.value===''&&Platform.OS==='ios'){
		this.setState({fakeValue:'', refresh:true}, ()=>this.setState({refresh: false}, ()=>this.input.focus()));//render null make blur perhaps, so do focus.
	}
}

focus=()=>{
	this.input.focus();
}

blur=()=>{
	this.input.blur();
}

clear=()=>{
	this.input.clear();
}

setFakeValue=fakeValue=>this.setState({fakeValue});//when parent setState({input}), call this too.

render(){
	if(this.state.refresh){//right clear
		return null;
	}

	return(
		<TextInput
			{...this.props}
			ref={ref=>this.input=ref}
			value={Platform.OS==='ios'?this.state.fakeValue:this.props.value}//only ios need this.
		/>
	);
}

}
`

@mmmulani

This comment has been minimized.

Contributor

mmmulani commented Jul 30, 2018

I instead fixed this in 892212b. The approach I took still sends onChange events, allowing you to process the input in RN if you wish.

@mmmulani mmmulani closed this Jul 30, 2018

@kelset

This comment has been minimized.

Collaborator

kelset commented Jul 30, 2018

Sweet!

@magicien

This comment has been minimized.

Contributor

magicien commented Jul 30, 2018

Thank you!! Finally I can use Japanese with React Native.

@johnxie

This comment has been minimized.

johnxie commented Jul 30, 2018

This is great, thank you.

@watanabeyu

This comment has been minimized.

Contributor

watanabeyu commented Jul 31, 2018

Thank

@bunnyc1986

This comment has been minimized.

bunnyc1986 commented Jul 31, 2018

Thank you! Look forward to seeing it in next release!

MarkRunWu added a commit to MarkRunWu/react-native that referenced this pull request Jul 31, 2018

Update RCTBaseTextInputShadowView.m
[iOS][TextInput] Fix controlled TextInput for Chinese (and other languages) facebook#18456
@ganlanshu0211

This comment has been minimized.

ganlanshu0211 commented Aug 14, 2018

Thanks!!!

1 similar comment
@471685249

This comment has been minimized.

471685249 commented Sep 22, 2018

Thanks!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment