Skip to content
This repository has been archived by the owner on Mar 4, 2022. It is now read-only.

Commit

Permalink
fix ios bug where <TextInput> ignored value prop
Browse files Browse the repository at this point in the history
The value in the native text input would sometimes be respected over the
value passed in. This was specifically an issue for text normalization:
for example, if we wanted to only allow digits in a text field and a
user enters "123a", the attributedText passed in from JS is "123"
(since it gets normalized), _previousAttributedText is "123" (since
that was the last value updated from the JS, but
baseTextInputView.attributedText is "123a" (since that gets updated by
the native event). In that scenario, the text in the input never gets
normalized.

There's a comment warning about not notifying the view of a text change
that isn't a real update, but as far as I can tell, this is a non-issue
here because RCTBaseTextInputView does its own checks to see if values
have changed.

This implementation was inspired by PR facebook#19087, but simplifies by
removing `_previousAttributedText` entirely and avoids breaking
uncontrolled text inputs.
  • Loading branch information
sidnair committed Aug 13, 2018
1 parent bb1c488 commit 7238193
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 11 deletions.
3 changes: 3 additions & 0 deletions Libraries/Components/TextInput/TextInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,7 @@ const TextInput = createReactClass({
onSelectionChange={this._onSelectionChange}
onSelectionChangeShouldSetResponder={emptyFunction.thatReturnsTrue}
text={this._getText()}
isControlled={typeof this.props.value === 'string'}
/>
);
} else {
Expand Down Expand Up @@ -958,6 +959,7 @@ const TextInput = createReactClass({
onTextInput={this._onTextInput}
onSelectionChangeShouldSetResponder={emptyFunction.thatReturnsTrue}
text={this._getText()}
isControlled={typeof this.props.value === 'string'}
dataDetectorTypes={this.props.dataDetectorTypes}
onScroll={this._onScroll}
/>
Expand Down Expand Up @@ -1010,6 +1012,7 @@ const TextInput = createReactClass({
onTextInput={this._onTextInput}
onSelectionChangeShouldSetResponder={emptyFunction.thatReturnsTrue}
text={this._getText()}
isControlled={typeof this.props.value === 'string'}
dataDetectorTypes={this.props.dataDetectorTypes}
onScroll={this._onScroll}
/>
Expand Down
1 change: 1 addition & 0 deletions Libraries/Text/TextInput/RCTBaseTextInputShadowView.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ NS_ASSUME_NONNULL_BEGIN
- (instancetype)initWithBridge:(RCTBridge *)bridge;

@property (nonatomic, copy, nullable) NSString *text;
@property (nonatomic, assign) BOOL isControlled;
@property (nonatomic, copy, nullable) NSString *placeholder;
@property (nonatomic, assign) NSInteger maximumNumberOfLines;
@property (nonatomic, copy, nullable) RCTDirectEventBlock onContentSizeChange;
Expand Down
16 changes: 5 additions & 11 deletions Libraries/Text/TextInput/RCTBaseTextInputShadowView.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
@implementation RCTBaseTextInputShadowView
{
__weak RCTBridge *_bridge;
NSAttributedString *_Nullable _previousAttributedText;
BOOL _needsUpdateView;
NSAttributedString *_Nullable _localAttributedText;
CGSize _previousContentSize;
Expand Down Expand Up @@ -137,15 +136,6 @@ - (void)uiManagerWillPerformMounting
[attributedText insertAttributedString:propertyAttributedText atIndex:0];
}

BOOL isAttributedTextChanged = NO;
if (![_previousAttributedText isEqualToAttributedString:attributedText]) {
// We have to follow `set prop` pattern:
// If the value has not changed, we must not notify the view about the change,
// otherwise we may break local (temporary) state of the text input.
isAttributedTextChanged = YES;
_previousAttributedText = [attributedText copy];
}

NSNumber *tag = self.reactTag;

[_bridge.uiManager addUIBlock:^(RCTUIManager *uiManager, NSDictionary<NSNumber *, UIView *> *viewRegistry) {
Expand All @@ -158,7 +148,11 @@ - (void)uiManagerWillPerformMounting
baseTextInputView.reactBorderInsets = borderInsets;
baseTextInputView.reactPaddingInsets = paddingInsets;

if (isAttributedTextChanged) {
// For controlled inputs attempt to update on any changes; an event lag
// check in the Libraries/Text/TextInput/RCTBaseTextInputView.m takes care
// of cases where the JS-provided value (attributedText.string) lags behind
// the one in the native text input.
if (self.isControlled && ![attributedText.string isEqualToString:baseTextInputView.attributedText.string]) {
baseTextInputView.attributedText = attributedText;
}
}];
Expand Down
1 change: 1 addition & 0 deletions Libraries/Text/TextInput/RCTBaseTextInputViewManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ @implementation RCTBaseTextInputViewManager
RCT_EXPORT_VIEW_PROPERTY(mostRecentEventCount, NSInteger)

RCT_EXPORT_SHADOW_PROPERTY(text, NSString)
RCT_EXPORT_SHADOW_PROPERTY(isControlled, BOOL)
RCT_EXPORT_SHADOW_PROPERTY(placeholder, NSString)
RCT_EXPORT_SHADOW_PROPERTY(onContentSizeChange, RCTBubblingEventBlock)

Expand Down

0 comments on commit 7238193

Please sign in to comment.