Uh oh!
There was an error while loading. Please reload this page.
Fix autocorrect onChangeText firing (#22691) - #23666
Conversation
ericlewis
commented
Feb 27, 2019
fixes #22691 |
ericlewis
commented
Feb 27, 2019
Would appreciate some extra eyes on this, it appears to work, though. |
shergin
commented
Feb 28, 2019
cc @rigdern, @mandrigin That's really cool. I didn't know that the API that we use here exists! That also probably can help us to fix other Korean/Japanese/Chinese issues. I will test it and hopefully land soon. |
| didProcessEditing:(__unused NSTextStorageEditActions)editedMask | ||
| range:(__unused NSRange)editedRange | ||
| changeInLength:(__unused NSInteger)delta { | ||
| [_backedTextInputView.textInputDelegate textInputDidChange]; |
There was a problem hiding this comment.
I am worried a bit about over-firing. So, e.g. if only text attributes were changed, we don't want to send an event.
There was a problem hiding this comment.
I will check if this is the case & fix it if that does occur.
facebook-github-bot
left a comment
There was a problem hiding this comment.
@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
ericlewis
commented
Feb 28, 2019
@shergin I changed this PR- I originally removed code from the UITextField adapter implementation, this was a mistake I believe. So, listening to the NSTextStorage only applies to multiline TextInput components. I feel like it would make sense for us to use UITextView instead of UITextField for all TextInput's due to UITextView having lower level API access. |
zhongwuzw
left a comment
There was a problem hiding this comment.
Give some my opinions:
- textStorage:didProcessEditing:range:changeInLength:delegate would not be called, we should settextStorage.delegateto adapter.textStorageis created and managed byUITextView, so is it really safe to change the delegate?- textStorage:didProcessEditing:range:changeInLength:would be called when text attributes changed, I think it may not what we want.- textStorage:didProcessEditing:range:changeInLength:would always be called, for example, we setdefaultValueprops for initial value, it should't be calledonChange.- If all things above fixes, singleline textinput still has issues. Because this
PRtry to fix multiline textinput.
ericlewis
commented
Mar 1, 2019
@zhongwuzw ah yes, I forgot to set the delegate in my cleaned up version. I will fix that. Good point on the remaining issues. |
Summary
This fixes an issue where autocorrect changes may cause
onChangeTextto not fire. It subscribes to the underlyingtextStorageof the UITextView. It is possible this fixes a few other issues, due to the nature of listening to the underlying textStorage changes.Changelog
[iOS] [Fixed] - iOS firing onChangeText when choosing autocorrect options
Test Plan
Open RNTester, and interact with textfields.