Fix controlled TextInput with child nodes

Summary:
Changelog: [Internal]

# There are three changes in this diff

## _stateRevision is replaced with a BOOL
`_stateRevision` was protecting against setting attributed string that is already visible to the user. Previously this was ok because the change was only coming from native, any changes from JS were ignored.

Imagine following scenario:

1. User taps key.
2. Update state is called on component initiated by native.
3. New state is created with incremented revision by one.
4. `_stateRevision` gets set to new state's revision + 1.
5. Now JS wants to change something because it just learnt that user tapped the key.
6. New state is created again with incremented revision by one.
7. Update state is called on the component, but the change isn't applied to the text view because `_state->getRevision()` will equal `_stateRevision`.

By having a BOOL instead of number, we very explicitly mark the region in which we don't want state changes to be applied to text view.

## Calling [_backedTextInputView setAttributedText] move cursor to the end of text input
This is prevented by storing what the current selection is and applying it after `[_backedTextInputView setAttributedText]` is called.
This was previously invisible because JS wasn't changing contents of `_backedTextInputView`.

## Storing of previously applied JS attributed string in state

This is the mechanism used to detect when value of text input changes come from JavaScript. JavaScript sends text input value changes through props and as children of TextInput.
We compare what previously was set from JavaScript to what is currently being send from JavaScript and if they differ, this change is communicated to the component.
Previously only first attributed string send from JavaScript was send to the component.

# Problem

If children are used to set text input's value, then there is a case in which we can't tell what source of truth should be.

Let's take following example
We have a text field that allows only 4 characters, again this is only a problem if those 4 characters come as children, not as value.
This is a controller text input.

1. User types 1234.
2. User types 5th character.
3. JavaScript updates TextInput, saying that the content should stay 1234.
4. In `TextInputShadowNode` `hasJSUpdatedAttributedString` will be set to false, because previous JS value is the same as current JS value.

Reviewed By: shergin

Differential Revision: D20587681

fbshipit-source-id: 1b8a2efabbfa0fc87cba210570142d162efe61e6
This commit is contained in:
Samuel Susla
2020-03-23 04:42:09 -07:00
committed by Facebook GitHub Bot
parent 8bbb2ca44b
commit a40cfc05b8
3 changed files with 31 additions and 10 deletions
@@ -28,7 +28,7 @@ using namespace facebook::react;
@implementation RCTTextInputComponentView {
TextInputShadowNode::ConcreteState::Shared _state;
UIView<RCTBackedTextInputViewProtocol> *_backedTextInputView;
size_t _stateRevision;
BOOL _ignoreStateUpdate;
}
- (instancetype)initWithFrame:(CGRect)frame
@@ -41,7 +41,7 @@ using namespace facebook::react;
_backedTextInputView = props.traits.multiline ? [[RCTUITextView alloc] init] : [[RCTUITextField alloc] init];
_backedTextInputView.frame = self.bounds;
_backedTextInputView.textInputDelegate = self;
_stateRevision = State::initialRevisionValue;
_ignoreStateUpdate = NO;
[self addSubview:_backedTextInputView];
}
@@ -166,10 +166,9 @@ using namespace facebook::react;
return;
}
if (_state->getRevision() != _stateRevision) {
if (!_ignoreStateUpdate) {
auto data = _state->getData();
_stateRevision = _state->getRevision();
_backedTextInputView.attributedText = RCTNSAttributedStringFromAttributedStringBox(data.attributedStringBox);
[self _setAttributedString:RCTNSAttributedStringFromAttributedStringBox(data.attributedStringBox)];
}
}
@@ -184,12 +183,18 @@ using namespace facebook::react;
RCTUIEdgeInsetsFromEdgeInsets(layoutMetrics.contentInsets - layoutMetrics.borderWidth);
}
- (void)_setAttributedString:(NSAttributedString *)attributedString
{
UITextRange *selectedRange = [_backedTextInputView selectedTextRange];
_backedTextInputView.attributedText = attributedString;
[_backedTextInputView setSelectedTextRange:selectedRange notifyDelegate:NO];
}
- (void)prepareForRecycle
{
[super prepareForRecycle];
_backedTextInputView.attributedText = [[NSAttributedString alloc] init];
_state.reset();
_stateRevision = State::initialRevisionValue;
}
#pragma mark - RCTComponentViewProtocol
@@ -328,8 +333,9 @@ using namespace facebook::react;
auto data = _state->getData();
data.attributedStringBox = RCTAttributedStringBoxFromNSAttributedString(attributedString);
_ignoreStateUpdate = YES;
_state->updateState(std::move(data), EventPriority::SynchronousUnbatched);
_stateRevision = _state->getRevision() + 1;
_ignoreStateUpdate = NO;
}
- (AttributedString::Range)_selectionRange
@@ -379,7 +385,7 @@ using namespace facebook::react;
[[NSMutableAttributedString alloc] initWithAttributedString:_backedTextInputView.attributedText];
[mutableString replaceCharactersInRange:NSMakeRange(0, _backedTextInputView.attributedText.length)
withString:value];
_backedTextInputView.attributedText = mutableString;
[self _setAttributedString:mutableString];
[self _updateState];
}
@@ -69,9 +69,19 @@ void TextInputShadowNode::setTextLayoutManager(
void TextInputShadowNode::updateStateIfNeeded() {
ensureUnsealed();
if (!getState() || getState()->getRevision() == State::initialRevisionValue) {
auto attributedStringFromJS = getAttributedString();
bool hasJSUpdatedAttributedString = false;
if (getState()) {
hasJSUpdatedAttributedString =
attributedStringFromJS.compareTextAttributesWithoutFrame(
getStateData().lastAttributedStringFromJS);
}
if (!getState() || getState()->getRevision() == State::initialRevisionValue ||
hasJSUpdatedAttributedString) {
auto state = TextInputState{};
state.attributedStringBox = AttributedStringBox{getAttributedString()};
state.attributedStringBox = AttributedStringBox{attributedStringFromJS};
state.lastAttributedStringFromJS = attributedStringFromJS;
state.paragraphAttributes = getConcreteProps().paragraphAttributes;
state.layoutManager = textLayoutManager_;
setStateData(std::move(state));
@@ -28,6 +28,11 @@ class TextInputState final {
*/
AttributedStringBox attributedStringBox;
/*
* Last attributed string that came from JavaScript.
*/
AttributedString lastAttributedStringFromJS;
/*
* Represents all visual attributes of a paragraph of text represented as
* a ParagraphAttributes.