From a50c9c8e222c6bf792d0d8ae47ef37fffb7da9d1 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Tue, 18 Jul 2017 14:33:45 -0700 Subject: [PATCH] TextInput: `selection` property was unified Summary: This diff unifies `selection` prop between single line and multi line text inputs. Besides that, this diff improves the selection event handling, makes it more robust and predictable. (See inline comments.) Reviewed By: mmmulani Differential Revision: D5317652 fbshipit-source-id: db5b0d2c0b80268e479ba866980e14b444079386 --- .../Text/RCTBackedTextInputDelegateAdapter.h | 5 ++ .../Text/RCTBackedTextInputDelegateAdapter.m | 78 ++++++++++++------- .../Text/RCTBackedTextInputViewProtocol.h | 8 ++ Libraries/Text/RCTTextField.h | 2 - Libraries/Text/RCTTextField.m | 37 +-------- Libraries/Text/RCTTextInput.h | 5 +- Libraries/Text/RCTTextInput.m | 30 ++++++- Libraries/Text/RCTTextView.h | 1 - Libraries/Text/RCTTextView.m | 29 ++----- Libraries/Text/RCTUITextField.m | 17 ++++ Libraries/Text/RCTUITextView.m | 13 ++++ 11 files changed, 132 insertions(+), 93 deletions(-) diff --git a/Libraries/Text/RCTBackedTextInputDelegateAdapter.h b/Libraries/Text/RCTBackedTextInputDelegateAdapter.h index 261dc893a23..dce127bbbad 100644 --- a/Libraries/Text/RCTBackedTextInputDelegateAdapter.h +++ b/Libraries/Text/RCTBackedTextInputDelegateAdapter.h @@ -18,6 +18,9 @@ - (instancetype)initWithTextField:(UITextField *)backedTextInput; +- (void)skipNextTextInputDidChangeSelectionEventWithTextRange:(UITextRange *)textRange; +- (void)selectedTextRangeWasSet; + @end #pragma mark - RCTBackedTextViewDelegateAdapter (for UITextView) @@ -26,4 +29,6 @@ - (instancetype)initWithTextView:(UITextView *)backedTextInput; +- (void)skipNextTextInputDidChangeSelectionEventWithTextRange:(UITextRange *)textRange; + @end diff --git a/Libraries/Text/RCTBackedTextInputDelegateAdapter.m b/Libraries/Text/RCTBackedTextInputDelegateAdapter.m index 14c34830a48..f902f4fd46e 100644 --- a/Libraries/Text/RCTBackedTextInputDelegateAdapter.m +++ b/Libraries/Text/RCTBackedTextInputDelegateAdapter.m @@ -18,23 +18,18 @@ static void *TextFieldSelectionObservingContext = &TextFieldSelectionObservingCo @implementation RCTBackedTextFieldDelegateAdapter { __weak UITextField *_backedTextInput; - __unsafe_unretained UITextField *_unsafeBackedTextInput; BOOL _textDidChangeIsComing; + UITextRange *_previousSelectedTextRange; } - (instancetype)initWithTextField:(UITextField *)backedTextInput { if (self = [super init]) { _backedTextInput = backedTextInput; - _unsafeBackedTextInput = backedTextInput; backedTextInput.delegate = self; [_backedTextInput addTarget:self action:@selector(textFieldDidChange) forControlEvents:UIControlEventEditingChanged]; [_backedTextInput addTarget:self action:@selector(textFieldDidEndEditingOnExit) forControlEvents:UIControlEventEditingDidEndOnExit]; - - // We have to use `unsafe_unretained` pointer to `backedTextInput` for subscribing (and especially unsubscribing) for it - // because `weak` pointers do not KVO complient, unfortunately. - [_unsafeBackedTextInput addObserver:self forKeyPath:@"selectedTextRange" options:0 context:TextFieldSelectionObservingContext]; } return self; @@ -44,7 +39,6 @@ static void *TextFieldSelectionObservingContext = &TextFieldSelectionObservingCo { [_backedTextInput removeTarget:self action:nil forControlEvents:UIControlEventEditingChanged]; [_backedTextInput removeTarget:self action:nil forControlEvents:UIControlEventEditingDidEndOnExit]; - [_unsafeBackedTextInput removeObserver:self forKeyPath:@"selectedTextRange" context:TextFieldSelectionObservingContext]; } #pragma mark - UITextFieldDelegate @@ -85,38 +79,20 @@ static void *TextFieldSelectionObservingContext = &TextFieldSelectionObservingCo return result; } -- (BOOL)textFieldShouldReturn:(UITextField *)textField +- (BOOL)textFieldShouldReturn:(__unused UITextField *)textField { return [_backedTextInput.textInputDelegate textInputShouldReturn]; } -#pragma mark - Key Value Observing - -- (void)observeValueForKeyPath:(NSString *)keyPath - ofObject:(nullable id)object - change:(NSDictionary *)change - context:(void *)context -{ - if (context == TextFieldSelectionObservingContext) { - if ([keyPath isEqualToString:@"selectedTextRange"]) { - [_backedTextInput.textInputDelegate textInputDidChangeSelection]; - } - - return; - } - - [super observeValueForKeyPath:keyPath - ofObject:object - change:change - context:context]; -} - #pragma mark - UIControlEventEditing* Family Events - (void)textFieldDidChange { _textDidChangeIsComing = NO; [_backedTextInput.textInputDelegate textInputDidChange]; + + // `selectedTextRangeWasSet` isn't triggered during typing. + [self textFieldProbablyDidChangeSelection]; } - (void)textFieldDidEndEditingOnExit @@ -134,6 +110,30 @@ static void *TextFieldSelectionObservingContext = &TextFieldSelectionObservingCo return YES; } +#pragma mark - Public Interface + +- (void)skipNextTextInputDidChangeSelectionEventWithTextRange:(UITextRange *)textRange +{ + _previousSelectedTextRange = textRange; +} + +- (void)selectedTextRangeWasSet +{ + [self textFieldProbablyDidChangeSelection]; +} + +#pragma mark - Generalization + +- (void)textFieldProbablyDidChangeSelection +{ + if ([_backedTextInput.selectedTextRange isEqual:_previousSelectedTextRange]) { + return; + } + + _previousSelectedTextRange = _backedTextInput.selectedTextRange; + [_backedTextInput.textInputDelegate textInputDidChangeSelection]; +} + @end #pragma mark - RCTBackedTextViewDelegateAdapter (for UITextView) @@ -144,6 +144,7 @@ static void *TextFieldSelectionObservingContext = &TextFieldSelectionObservingCo @implementation RCTBackedTextViewDelegateAdapter { __weak UITextView *_backedTextInput; BOOL _textDidChangeIsComing; + UITextRange *_previousSelectedTextRange; } - (instancetype)initWithTextView:(UITextView *)backedTextInput @@ -211,6 +212,25 @@ static void *TextFieldSelectionObservingContext = &TextFieldSelectionObservingCo - (void)textViewDidChangeSelection:(__unused UITextView *)textView { + [self textViewProbablyDidChangeSelection]; +} + +#pragma mark - Public Interface + +- (void)skipNextTextInputDidChangeSelectionEventWithTextRange:(UITextRange *)textRange +{ + _previousSelectedTextRange = textRange; +} + +#pragma mark - Generalization + +- (void)textViewProbablyDidChangeSelection +{ + if ([_backedTextInput.selectedTextRange isEqual:_previousSelectedTextRange]) { + return; + } + + _previousSelectedTextRange = _backedTextInput.selectedTextRange; [_backedTextInput.textInputDelegate textInputDidChangeSelection]; } diff --git a/Libraries/Text/RCTBackedTextInputViewProtocol.h b/Libraries/Text/RCTBackedTextInputViewProtocol.h index 92acdff004e..79f70ce1bfa 100644 --- a/Libraries/Text/RCTBackedTextInputViewProtocol.h +++ b/Libraries/Text/RCTBackedTextInputViewProtocol.h @@ -23,4 +23,12 @@ @property (nonatomic, strong, nullable) UIView *inputAccessoryView; @property (nonatomic, weak, nullable) id textInputDelegate; +// This protocol disallows direct access to `selectedTextRange` property because +// unwise usage of it can break the `delegate` behavior. So, we always have to +// explicitly specify should `delegate` be notified about the change or not. +// If the change was initiated programmatically, we must NOT notify the delegate. +// If the change was a result of user actions (like typing or touches), we MUST notify the delegate. +- (void)setSelectedTextRange:(nullable UITextRange *)selectedTextRange NS_UNAVAILABLE; +- (void)setSelectedTextRange:(nullable UITextRange *)selectedTextRange notifyDelegate:(BOOL)notifyDelegate; + @end diff --git a/Libraries/Text/RCTTextField.h b/Libraries/Text/RCTTextField.h index 6e8d840b2e7..975c926b2ab 100644 --- a/Libraries/Text/RCTTextField.h +++ b/Libraries/Text/RCTTextField.h @@ -21,6 +21,4 @@ @property (nonatomic, assign) BOOL caretHidden; @property (nonatomic, strong) NSNumber *maxLength; -@property (nonatomic, copy) RCTDirectEventBlock onSelectionChange; - @end diff --git a/Libraries/Text/RCTTextField.m b/Libraries/Text/RCTTextField.m index 3773ae7ed2e..b10ae76c2da 100644 --- a/Libraries/Text/RCTTextField.m +++ b/Libraries/Text/RCTTextField.m @@ -86,35 +86,14 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithCoder:(NSCoder *)aDecoder) NSInteger offsetFromEnd = oldTextLength - offsetStart; NSInteger newOffset = text.length - offsetFromEnd; UITextPosition *position = [_backedTextInput positionFromPosition:_backedTextInput.beginningOfDocument offset:newOffset]; - _backedTextInput.selectedTextRange = [_backedTextInput textRangeFromPosition:position toPosition:position]; + [_backedTextInput setSelectedTextRange:[_backedTextInput textRangeFromPosition:position toPosition:position] + notifyDelegate:YES]; } } else if (eventLag > RCTTextUpdateLagWarningThreshold) { RCTLogWarn(@"Native TextInput(%@) is %zd events ahead of JS - try to make your JS faster.", _backedTextInput.text, eventLag); } } -#pragma mark - Events - -- (void)sendSelectionEvent -{ - if (_onSelectionChange && - _backedTextInput.selectedTextRange != _previousSelectionRange && - ![_backedTextInput.selectedTextRange isEqual:_previousSelectionRange]) { - - _previousSelectionRange = _backedTextInput.selectedTextRange; - - UITextRange *selection = _backedTextInput.selectedTextRange; - NSInteger start = [_backedTextInput offsetFromPosition:[_backedTextInput beginningOfDocument] toPosition:selection.start]; - NSInteger end = [_backedTextInput offsetFromPosition:[_backedTextInput beginningOfDocument] toPosition:selection.end]; - _onSelectionChange(@{ - @"selection": @{ - @"start": @(start), - @"end": @(end), - }, - }); - } -} - #pragma mark - RCTBackedTextInputDelegate - (BOOL)textInputShouldChangeTextInRange:(NSRange)range replacementText:(NSString *)string @@ -137,7 +116,8 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithCoder:(NSCoder *)aDecoder) // Collapse selection at end of insert to match normal paste behavior. UITextPosition *insertEnd = [_backedTextInput positionFromPosition:_backedTextInput.beginningOfDocument offset:(range.location + allowedLength)]; - _backedTextInput.selectedTextRange = [_backedTextInput textRangeFromPosition:insertEnd toPosition:insertEnd]; + [_backedTextInput setSelectedTextRange:[_backedTextInput textRangeFromPosition:insertEnd toPosition:insertEnd] + notifyDelegate:YES]; [self textInputDidChange]; } return NO; @@ -155,10 +135,6 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithCoder:(NSCoder *)aDecoder) text:_backedTextInput.text key:nil eventCount:_nativeEventCount]; - - // selectedTextRange observer isn't triggered when you type even though the - // cursor position moves, so we send event again here. - [self sendSelectionEvent]; } - (BOOL)textInputShouldEndEditing @@ -181,9 +157,4 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithCoder:(NSCoder *)aDecoder) eventCount:_nativeEventCount]; } -- (void)textInputDidChangeSelection -{ - [self sendSelectionEvent]; -} - @end diff --git a/Libraries/Text/RCTTextInput.h b/Libraries/Text/RCTTextInput.h index 294fb56b826..ac2e615343b 100644 --- a/Libraries/Text/RCTTextInput.h +++ b/Libraries/Text/RCTTextInput.h @@ -15,12 +15,12 @@ @class RCTBridge; @class RCTEventDispatcher; +@class RCTTextSelection; @interface RCTTextInput : RCTView { @protected RCTBridge *_bridge; RCTEventDispatcher *_eventDispatcher; - UITextRange *_previousSelectionRange; NSInteger _nativeEventCount; NSInteger _mostRecentEventCount; BOOL _blurOnSubmit; @@ -39,11 +39,13 @@ @property (nonatomic, assign, readonly) CGSize contentSize; @property (nonatomic, copy) RCTDirectEventBlock onContentSizeChange; +@property (nonatomic, copy) RCTDirectEventBlock onSelectionChange; @property (nonatomic, assign) NSInteger mostRecentEventCount; @property (nonatomic, assign) BOOL blurOnSubmit; @property (nonatomic, assign) BOOL selectTextOnFocus; @property (nonatomic, assign) BOOL clearTextOnFocus; +@property (nonatomic, copy) RCTTextSelection *selection; - (void)invalidateContentSize; @@ -53,5 +55,6 @@ - (void)textInputDidBeginEditing; - (BOOL)textInputShouldReturn; - (void)textInputDidReturn; +- (void)textInputDidChangeSelection; @end diff --git a/Libraries/Text/RCTTextInput.m b/Libraries/Text/RCTTextInput.m index 7ba1435a0df..c21426a00ca 100644 --- a/Libraries/Text/RCTTextInput.m +++ b/Libraries/Text/RCTTextInput.m @@ -62,6 +62,14 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithFrame:(CGRect)frame) [self setNeedsLayout]; } +- (RCTTextSelection *)selection +{ + id backedTextInput = self.backedTextInputView; + UITextRange *selectedTextRange = backedTextInput.selectedTextRange; + return [[RCTTextSelection new] initWithStart:[backedTextInput offsetFromPosition:backedTextInput.beginningOfDocument toPosition:selectedTextRange.start] + end:[backedTextInput offsetFromPosition:backedTextInput.beginningOfDocument toPosition:selectedTextRange.end]]; +} + - (void)setSelection:(RCTTextSelection *)selection { if (!selection) { @@ -70,15 +78,14 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithFrame:(CGRect)frame) id backedTextInput = self.backedTextInputView; - UITextRange *currentSelection = backedTextInput.selectedTextRange; + UITextRange *previousSelectedTextRange = backedTextInput.selectedTextRange; UITextPosition *start = [backedTextInput positionFromPosition:backedTextInput.beginningOfDocument offset:selection.start]; UITextPosition *end = [backedTextInput positionFromPosition:backedTextInput.beginningOfDocument offset:selection.end]; UITextRange *selectedTextRange = [backedTextInput textRangeFromPosition:start toPosition:end]; NSInteger eventLag = _nativeEventCount - _mostRecentEventCount; - if (eventLag == 0 && ![currentSelection isEqual:selectedTextRange]) { - _previousSelectionRange = selectedTextRange; - backedTextInput.selectedTextRange = selectedTextRange; + if (eventLag == 0 && ![previousSelectedTextRange isEqual:selectedTextRange]) { + [backedTextInput setSelectedTextRange:selectedTextRange notifyDelegate:NO]; } else if (eventLag > RCTTextUpdateLagWarningThreshold) { RCTLogWarn(@"Native TextInput(%@) is %zd events ahead of JS - try to make your JS faster.", backedTextInput.text, eventLag); } @@ -124,6 +131,21 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithFrame:(CGRect)frame) eventCount:_nativeEventCount]; } +- (void)textInputDidChangeSelection +{ + if (!_onSelectionChange) { + return; + } + + RCTTextSelection *selection = self.selection; + _onSelectionChange(@{ + @"selection": @{ + @"start": @(selection.start), + @"end": @(selection.end), + }, + }); +} + #pragma mark - Content Size (in Yoga terms, without any insets) - (CGSize)contentSize diff --git a/Libraries/Text/RCTTextView.h b/Libraries/Text/RCTTextView.h index 5c55e26e675..09c67faba6a 100644 --- a/Libraries/Text/RCTTextView.h +++ b/Libraries/Text/RCTTextView.h @@ -28,7 +28,6 @@ @property (nonatomic, strong) NSNumber *maxLength; @property (nonatomic, copy) RCTDirectEventBlock onChange; -@property (nonatomic, copy) RCTDirectEventBlock onSelectionChange; @property (nonatomic, copy) RCTDirectEventBlock onTextInput; @property (nonatomic, copy) RCTDirectEventBlock onScroll; diff --git a/Libraries/Text/RCTTextView.m b/Libraries/Text/RCTTextView.m index bb71acbc8b8..020c0acb01c 100644 --- a/Libraries/Text/RCTTextView.m +++ b/Libraries/Text/RCTTextView.m @@ -184,7 +184,8 @@ static NSAttributedString *removeReactTagFromString(NSAttributedString *string) NSInteger offsetFromEnd = oldTextLength - start; NSInteger newOffset = _backedTextInput.attributedText.length - offsetFromEnd; UITextPosition *position = [_backedTextInput positionFromPosition:_backedTextInput.beginningOfDocument offset:newOffset]; - _backedTextInput.selectedTextRange = [_backedTextInput textRangeFromPosition:position toPosition:position]; + [_backedTextInput setSelectedTextRange:[_backedTextInput textRangeFromPosition:position toPosition:position] + notifyDelegate:YES]; } [_backedTextInput layoutIfNeeded]; @@ -228,7 +229,8 @@ static NSAttributedString *removeReactTagFromString(NSAttributedString *string) NSInteger offsetFromEnd = oldTextLength - start; NSInteger newOffset = text.length - offsetFromEnd; UITextPosition *position = [_backedTextInput positionFromPosition:_backedTextInput.beginningOfDocument offset:newOffset]; - _backedTextInput.selectedTextRange = [_backedTextInput textRangeFromPosition:position toPosition:position]; + [_backedTextInput setSelectedTextRange:[_backedTextInput textRangeFromPosition:position toPosition:position] + notifyDelegate:YES]; } [self invalidateContentSize]; @@ -271,7 +273,8 @@ static NSAttributedString *removeReactTagFromString(NSAttributedString *string) // Collapse selection at end of insert to match normal paste behavior UITextPosition *insertEnd = [_backedTextInput positionFromPosition:_backedTextInput.beginningOfDocument offset:(range.location + allowedLength)]; - _backedTextInput.selectedTextRange = [_backedTextInput textRangeFromPosition:insertEnd toPosition:insertEnd]; + [_backedTextInput setSelectedTextRange:[_backedTextInput textRangeFromPosition:insertEnd toPosition:insertEnd] + notifyDelegate:YES]; [self textInputDidChange]; } @@ -309,26 +312,6 @@ static NSAttributedString *removeReactTagFromString(NSAttributedString *string) return YES; } -- (void)textInputDidChangeSelection -{ - if (_onSelectionChange && - _backedTextInput.selectedTextRange != _previousSelectionRange && - ![_backedTextInput.selectedTextRange isEqual:_previousSelectionRange]) { - - _previousSelectionRange = _backedTextInput.selectedTextRange; - - UITextRange *selection = _backedTextInput.selectedTextRange; - NSInteger start = [_backedTextInput offsetFromPosition:_backedTextInput.beginningOfDocument toPosition:selection.start]; - NSInteger end = [_backedTextInput offsetFromPosition:_backedTextInput.beginningOfDocument toPosition:selection.end]; - _onSelectionChange(@{ - @"selection": @{ - @"start": @(start), - @"end": @(end), - }, - }); - } -} - static BOOL findMismatch(NSString *first, NSString *second, NSRange *firstRange, NSRange *secondRange) { NSInteger firstMismatch = -1; diff --git a/Libraries/Text/RCTUITextField.m b/Libraries/Text/RCTUITextField.m index 188091959a3..82a4b58f5d0 100644 --- a/Libraries/Text/RCTUITextField.m +++ b/Libraries/Text/RCTUITextField.m @@ -112,6 +112,23 @@ #pragma mark - Overrides +- (void)setSelectedTextRange:(UITextRange *)selectedTextRange +{ + [super setSelectedTextRange:selectedTextRange]; + [_textInputDelegateAdapter selectedTextRangeWasSet]; +} + +- (void)setSelectedTextRange:(UITextRange *)selectedTextRange notifyDelegate:(BOOL)notifyDelegate +{ + if (!notifyDelegate) { + // We have to notify an adapter that following selection change was initiated programmatically, + // so the adapter must not generate a notification for it. + [_textInputDelegateAdapter skipNextTextInputDidChangeSelectionEventWithTextRange:selectedTextRange]; + } + + [super setSelectedTextRange:selectedTextRange]; +} + - (void)paste:(id)sender { [super paste:sender]; diff --git a/Libraries/Text/RCTUITextView.m b/Libraries/Text/RCTUITextView.m index 893594de0de..f9c3e5d34be 100644 --- a/Libraries/Text/RCTUITextView.m +++ b/Libraries/Text/RCTUITextView.m @@ -122,6 +122,19 @@ static UIColor *defaultPlaceholderColor() [self textDidChange]; } +#pragma mark - Overrides + +- (void)setSelectedTextRange:(UITextRange *)selectedTextRange notifyDelegate:(BOOL)notifyDelegate +{ + if (!notifyDelegate) { + // We have to notify an adapter that following selection change was initiated programmatically, + // so the adapter must not generate a notification for it. + [_textInputDelegateAdapter skipNextTextInputDidChangeSelectionEventWithTextRange:selectedTextRange]; + } + + [super setSelectedTextRange:selectedTextRange]; +} + - (void)paste:(id)sender { [super paste:sender];