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

Libraries/Text/RCTBackedTextInputDelegateAdapter.h

@@ -18,6 +18,9 @@
 
 - (instancetype)initWithTextField:(UITextField<RCTBackedTextInputViewProtocol> *)backedTextInput;
 
+- (void)skipNextTextInputDidChangeSelectionEventWithTextRange:(UITextRange *)textRange;
+- (void)selectedTextRangeWasSet;
+
 @end
 
 #pragma mark - RCTBackedTextViewDelegateAdapter (for UITextView)
@@ -26,4 +29,6 @@
 
 - (instancetype)initWithTextView:(UITextView<RCTBackedTextInputViewProtocol> *)backedTextInput;
 
+- (void)skipNextTextInputDidChangeSelectionEventWithTextRange:(UITextRange *)textRange;
+
 @end

Libraries/Text/RCTBackedTextInputDelegateAdapter.m

@@ -18,23 +18,18 @@ static void *TextFieldSelectionObservingContext = &TextFieldSelectionObservingCo
 
 @implementation RCTBackedTextFieldDelegateAdapter {
   __weak UITextField<RCTBackedTextInputViewProtocol> *_backedTextInput;
-  __unsafe_unretained UITextField<RCTBackedTextInputViewProtocol> *_unsafeBackedTextInput;
   BOOL _textDidChangeIsComing;
+  UITextRange *_previousSelectedTextRange;
 }
 
 - (instancetype)initWithTextField:(UITextField<RCTBackedTextInputViewProtocol> *)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<RCTBackedTextInputViewProtocol> *_backedTextInput;
   BOOL _textDidChangeIsComing;
+  UITextRange *_previousSelectedTextRange;
 }
 
 - (instancetype)initWithTextView:(UITextView<RCTBackedTextInputViewProtocol> *)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];
 }
 

Libraries/Text/RCTBackedTextInputViewProtocol.h

@@ -23,4 +23,12 @@
 @property (nonatomic, strong, nullable) UIView *inputAccessoryView;
 @property (nonatomic, weak, nullable) id<RCTBackedTextInputDelegate> 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

Libraries/Text/RCTTextField.h

@@ -21,6 +21,4 @@
 @property (nonatomic, assign) BOOL caretHidden;
 @property (nonatomic, strong) NSNumber *maxLength;
 
-@property (nonatomic, copy) RCTDirectEventBlock onSelectionChange;
-
 @end

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

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

Libraries/Text/RCTTextInput.m

@@ -62,6 +62,14 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithFrame:(CGRect)frame)
   [self setNeedsLayout];
 }
 
+- (RCTTextSelection *)selection
+{
+  id<RCTBackedTextInputViewProtocol> 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<RCTBackedTextInputViewProtocol> 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

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;
 

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;

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];

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];