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

TextInput value can't be change during onChangeText [iOS] #18874

Closed
3 tasks done
jerameel opened this issue Apr 16, 2018 · 81 comments
Closed
3 tasks done

TextInput value can't be change during onChangeText [iOS] #18874

jerameel opened this issue Apr 16, 2018 · 81 comments
Labels
Component: TextInput Related to the TextInput component. Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: iOS iOS applications. Resolution: Fixed A PR that fixes this issue has been merged. Resolution: Locked This issue was locked by the bot.

Comments

@jerameel
Copy link

jerameel commented Apr 16, 2018

TextInput's value property does not seem to get in sync when changing state via onChangeText.

An example use case would be blocking non numerical characters from the input.

This works on Android though.

Environment

OS: macOS High Sierra 10.13.3
Node: 8.10.0
Yarn: 1.5.1
npm: 5.6.0
Watchman: 4.9.0
Xcode: Xcode 9.3 Build version 9E145
Android Studio: 3.1 AI-173.4697961

react: ^16.3.0-alpha.1 => 16.3.0-rc.0
react-native: 0.54.3 => 0.54.3

Expected Behavior

TextInput value should be equal to state value even when it is modified during onChangeText.

Actual Behavior

TextInput value is not being modified.

@react-native-bot

This comment has been minimized.

@react-native-bot react-native-bot added ❔Needs More Information Platform: iOS iOS applications. Component: TextInput Related to the TextInput component. labels Apr 16, 2018
@Kaniz92

This comment has been minimized.

@rplankenhorn
Copy link

rplankenhorn commented Apr 18, 2018

I am seeing a similar issue where I can't filter undesired characters from JS. I attempted to use the changes in the PR I tagged but I saw some issues:

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.

@jerameel
Copy link
Author

It seems on iOS, value property is not the one prioritized, instead it shows the actual value typed in. Isn't this supposed to be the same with both platforms?

OT: Why is this getting tag with "needs more information" while I already had it in the first place.

@rplankenhorn
Copy link

@shergin @PeteTheHeat @cbrevik Any thoughts? Looks like you are the three that contributed most to RCTBaseTextInputView.

@rplankenhorn
Copy link

I did some more digging and may have found a temporary fix for my project:

I noticed that onTextInput is a prop that isn't documented anywhere but it's still fired. In RCTBaseTextInputView.m, towards the end of - (BOOL)textInputShouldChangeTextInRange:(NSRange)range replacementText:(NSString *)text, it does an if check to see if this prop has been passed in and if so, fires the event and outside of the if statement, it returns YES to allow the characters to be added.

For my fix, I just returned NO inside of the if (_onTextInput) check to prevent characters added in before I could do validation. I also pass _predictedText for text inside of the event instead of text. This allowed for me to do data validation in the JS layer and then set the new text via props.

I don't think I will be PRing this to React Native since it doesn't seem like a universal fix. Here is the diff if anyone is interested:

diff --git a/Libraries/Text/TextInput/RCTBaseTextInputView.m b/Libraries/Text/TextInput/RCTBaseTextInputView.m
index d9c47ff16..494d1715e 100644
--- a/Libraries/Text/TextInput/RCTBaseTextInputView.m
+++ b/Libraries/Text/TextInput/RCTBaseTextInputView.m
@@ -276,7 +276,7 @@ - (BOOL)textInputShouldChangeTextInRange:(NSRange)range replacementText:(NSStrin
 
   if (_onTextInput) {
     _onTextInput(@{
-      @"text": text,
+      @"text": _predictedText,
       @"previousText": previousText,
       @"range": @{
         @"start": @(range.location),
@@ -284,6 +284,8 @@ - (BOOL)textInputShouldChangeTextInRange:(NSRange)range replacementText:(NSStrin
       },
       @"eventCount": @(_nativeEventCount),
     });
+    
+    return NO;
   }
 
   return YES;

@imanderson

This comment has been minimized.

@zaguiini
Copy link

Thanks, @rplankenhorn . It works for now. You should submit a PR...

@rplankenhorn
Copy link

@zaguini Glad it works for you! I am hesitant to submit a PR because it doesn't fix the underlying issue. onTextInput is undocumented and has been around since very very early on in the project.

I know TextInput was just re-written but I think we all need to think about what is the best solution. It could be something as simple as adding an additional prop to TextInput called requiresValidation that forces textInputShouldChangeTextInRange to return no and wait for the text to be updated via the text prop. I am not sure.

I can try to come up with a better solution and submit a PR but I am swamped at work currently.

@zaguiini
Copy link

zaguiini commented Apr 25, 2018

Yeah it should be ideal to mantain the React Native's TextInput API... onChangeText and onChange instead of onTextInput... A lot of compos rely on those previous cited methods.

@rplankenhorn
Copy link

Upon further testing, I found that modifying textInputShouldChangeTextInRange with the above diff was the wrong approach. It caused two errors in my project:

  1. TextInputs now missed some characters when they were rapidly entered.
  2. Clearing text using the "X" on the right now resulted in a crash.

The above fix is more robust as it allows for setting the text to an empty string if it's empty because it's been filtered.

@yairopro

This comment has been minimized.

1 similar comment
@Victorams

This comment has been minimized.

@react-native-bot

This comment has been minimized.

@yairopro
Copy link

yairopro commented May 16, 2018

Here's a demo about this problem. In the demo, only numbers are accepted in the input.
It works fine on Android. Not in iOS.
https://snack.expo.io/SJsmZ4DRM

@JongLim107

This comment has been minimized.

@vovkasm
Copy link
Contributor

vovkasm commented May 21, 2018

@shergin This issue very serious, controlled mode of TextInput really broken. Can you please look at this? You the only one who create new Text implementation and probably the best person who can fix this properly in align with philosophy of new implementation!

@benevbright
Copy link

benevbright commented Aug 1, 2018

@ycai2 @vdlindenmark
Hi, guys.
I have to say that I'm sorry about it.
I misunderstood the original problem.
After I noticed, I tried to find a workaround with above example for hours.
Nothing is working.

I thought it's the same problem with Korean character problem. (it's another issue)
I marked my previous comment as the wrong comment.

@ycai2
Copy link

ycai2 commented Aug 1, 2018

@benevbright Oh I guess that makes sense. I also thought it was related. I saw a bunch issues that are similar, and some of them were referenced in this thread, so I thought it's the same problem. Thanks for the help though!

@hramos
Copy link
Contributor

hramos commented Aug 2, 2018

Just checking, does 892212b fix the issue described in this thread?

@dylansmurray
Copy link

@hramos Tested applying these changes on 55.3 and did not fix this issue.

@zhongwuzw
Copy link
Contributor

@yairopro Please see my PR #20634 , I tested and it works.

@yairopro
Copy link

@zhongwuzw I think you picked the wrong guy. I am not able to merge your PR .

@zhongwuzw
Copy link
Contributor

@yairopro 😂 I mean my PR may solved the demo you provided.

@yairopro
Copy link

@zhongwuzw ok. Good word dude 👍. Now let's see how this issue will be resolved with your PR😄

@abelcha
Copy link

abelcha commented Aug 22, 2018

As of RN56:
@rplankenhorn solution breaks onChange/onChangeText, but allow full controlled input (using onKeyPress)

I started of @rplankenhorn solution, and then forwarded a "controlled" flag to RCTBaseTextInputView
which only block textInputShouldChangeTextInRange on controlled input.

I then created a <ControlledTextInput/> module which recreate onChangeText behavior using onKeyPress and onSelectionChange

You can find the module with doc and example here 😀

https://github.com/abelcha/react-native-controlled-input
https://www.npmjs.com/package/react-native-controlled-input

@VicEsquivel
Copy link

VicEsquivel commented Aug 29, 2018

@shergin @PeteTheHeat @cbrevik Still no response? In almost 5 months having this major bug open... Everyone in the community experimenting workarounds and messing their own projects doing regressions because the version released is simply not doing its job correctly.

Its normal to have bugs when features or code restructuring iterations are made, but this 5 month bug resolving threshold, projects absent commitment.

Exactly as @vovkasm said, this is indeed a very strong message for community...

@plimerence
Copy link

plimerence commented Aug 30, 2018

if the textInput text length is longer than state text length,the text can't be change ,I don't want to change objective-c code,so I try a hack code,like this.

onChangeText={(text)=>{ console.log('输入的text',text); if ( parseInt(text)> 999){ setTimeout(()=>{this.ref.clear()},10); setTimeout(()=>{this.setState({phoneNo:'999'})},50); }else{ this.setState({phoneNo:text}); } }}

the core method is clear method, if I clear textInput,the textInput text length is shorter than state text length ,it is workaround for me.

@ycai2
Copy link

ycai2 commented Sep 6, 2018

@abelcha thank you for the module, but if this is the way everyone will have to end up using react-native, it would be ridiculous...

@hramos
Copy link
Contributor

hramos commented Sep 6, 2018

It would seem like the source for this bug is in RCTBaseTextInputView.m, if anyone wants to take a stab at fixing this.

Frankly, this is not an issue that has shown up in our own apps, therefore it hasn't become a priority for us to dedicate time to fixing it. Please do send a PR if you'd like to propose a fix.

We're also happy with taking @abelcha's approach of using third party modules; in fact, we're working towards a slimmer React Native core, where components such as Text Inputs might all be imported from npm packages outside of the RN core.

@isaacloft
Copy link

I've been experiencing the same issue, here's a workaround I'm using:

validateUsername(username) {

    // only alphanumerical and underscore
    const filteredUsername = username.replace(/[^\w]/gi, '');
   
    if(filteredUsername !== username) {
      console.log('forcing update');
      this.setState({ username: filteredUsername + '✕' }); // this character is not alphanumerical 'x', it's a forbidden character '✕' (cross)
      setTimeout(() => {
        this.setState((previousState) => {
          return { ...previousState, username: previousState.username.replace(/[^\w]/gi, '') };
        });
      }, 0);
    } else {
      this.setState({ username: filteredUsername });
    }
}

I set onChangeText={validateUsername}. I've noticed that TextInput update doesn't work when new string's length is shorter than the one typed in by the user but it does work when the new string is the same length but different or when it is longer. When a forbidded character is detected, the code above briefly adds and then removes another forbidden character. This forces TextInput to update where setState alone wouldn't. I'm using a state update function to avoid the state getting out of sync when the user is typing fast.

works for me

@fernandofranca
Copy link

fernandofranca commented Sep 12, 2018

I'm concatenating alternated invisible characters to workaround this bug.
Ugly hack:

const inputWorkaround = (() => {
  let workaroundIncrement = 0
  const invisibleCharsArr = [
    String.fromCharCode(28), 
    String.fromCharCode(29),
    String.fromCharCode(30),
    String.fromCharCode(31),
  ]
  return {
    getWorkaroundChar: () => {
      workaroundIncrement += 1
      const mod = workaroundIncrement % invisibleCharsArr.length
      return invisibleCharsArr[mod]
    }
  }
})()

class MyComponent extends React.Component {
  render() {
    const prefixedValue = inputWorkaround.getWorkaroundChar() + this.props.value
    return (<TextInput value={prefixedValue} />)
  }
}

You'll probably want to avoid doing this on Android. Some OS versions will print the invisible chars.

@zaguiini
Copy link

I'm concatenating alternated invisible characters to workaround this bug.
Ugly hack:

const inputWorkaround = (() => {
  let workaroundIncrement = 0
  const invisibleCharsArr = [
    String.fromCharCode(28), 
    String.fromCharCode(29),
    String.fromCharCode(30),
    String.fromCharCode(31),
  ]
  return {
    getWorkaroundChar: () => {
      workaroundIncrement += 1
      const mod = workaroundIncrement % invisibleCharsArr.length
      return invisibleCharsArr[mod]
    }
  }
})()

class MyComponent extends React.Component {
  render() {
    const prefixedValue = inputWorkaround.getWorkaroundChar() + this.props.value
    return (<TextInput value={prefixedValue} />)
  }
}

You'll probably want to avoid doing this on Android. Some OS versions will print the invisible chars.

Holy crap.
Anyways, bug's fixed by the RN Team.

@fernandofranca
Copy link

Anyways, bug's fixed by the RN Team.

Fixed in which version @zaguiini ?

@zaguiini
Copy link

zaguiini commented Sep 12, 2018

Latest (v0.57.0), @fernandofranca. Check the CHANGELOG.

@vovkasm
Copy link
Contributor

vovkasm commented Sep 12, 2018

I can confirm, that this bug fixed in RN 0.57, tested with this simple application (branch rn-0.57): https://github.com/vovkasm/react-native-textinput-bug/tree/rn-0.57

Also some flicker exists, it's currently unavoidable due to inter threads communication with controlled text inputs (information should move between UI and JS threads).

@hramos hramos closed this as completed Sep 13, 2018
@paramadeep
Copy link

I too have the issue with 0.57

@vovkasm
Copy link
Contributor

vovkasm commented Sep 16, 2018

@paramadeep Can you create reproducible example as in https://github.com/vovkasm/react-native-textinput-bug/ ?

@danReynolds
Copy link
Contributor

Can someone point me to the commit that fixed it?

@vovkasm
Copy link
Contributor

vovkasm commented Sep 24, 2018

@danReynolds, I think it is 2307ea6

@hramos hramos added the Resolution: Fixed A PR that fixes this issue has been merged. label Sep 24, 2018
@hramos
Copy link
Contributor

hramos commented Sep 24, 2018

This is in 0.57.1.

@facebook facebook locked as resolved and limited conversation to collaborators Sep 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Component: TextInput Related to the TextInput component. Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: iOS iOS applications. Resolution: Fixed A PR that fixes this issue has been merged. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests