Fixed OSS scroll view bug caused by FBPullToRefresh

Summary: When I bridged FBPullToRefresh to RN, the integration with ScrollView caused a bug on OSS TLDR; assuming that a scrollview subview that implemented UIScrollViewDelegate protocol was a custom PTR was a bad idea. This caused some scrollviews to break in OSS. The solution is to define a more explicit protocol. Further details here: #20324 Reviewed By: mmmulani Differential Revision: D8953893 fbshipit-source-id: 98cdc7fcced41d9e98e77293a03934f10c798665
facebook · Jul 23, 2018 · fab5fff · fab5fff
1 parent 1f54574
commit fab5fff
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 21 deletions.
diff --git a/React/Views/RCTRefreshControl.h b/React/Views/RCTRefreshControl.h
@@ -8,8 +8,9 @@
 #import <UIKit/UIKit.h>
 
 #import <React/RCTComponent.h>
+#import <React/RCTScrollableProtocol.h>
 
-@interface RCTRefreshControl : UIRefreshControl
+@interface RCTRefreshControl : UIRefreshControl <RCTCustomRefreshContolProtocol>
 
 @property (nonatomic, copy) NSString *title;
 @property (nonatomic, copy) RCTDirectEventBlock onRefresh;

diff --git a/React/Views/ScrollView/RCTScrollView.m b/React/Views/ScrollView/RCTScrollView.m
@@ -155,7 +155,7 @@ @interface RCTCustomScrollView : UIScrollView<UIGestureRecognizerDelegate>
 
 @property (nonatomic, assign) BOOL centerContent;
 #if !TARGET_OS_TV
-@property (nonatomic, strong) RCTRefreshControl *rctRefreshControl;
+@property (nonatomic, strong) UIView<RCTCustomRefreshContolProtocol> *customRefreshControl;
 @property (nonatomic, assign) BOOL pinchGestureEnabled;
 #endif
 
@@ -329,13 +329,13 @@ - (void)setFrame:(CGRect)frame
 }
 
 #if !TARGET_OS_TV
-- (void)setRctRefreshControl:(RCTRefreshControl *)refreshControl
+- (void)setCustomRefreshControl:(UIView<RCTCustomRefreshContolProtocol> *)refreshControl
 {
-  if (_rctRefreshControl) {
-    [_rctRefreshControl removeFromSuperview];
+  if (_customRefreshControl) {
+    [_customRefreshControl removeFromSuperview];
   }
-  _rctRefreshControl = refreshControl;
-  [self addSubview:_rctRefreshControl];
+  _customRefreshControl = refreshControl;
+  [self addSubview:_customRefreshControl];
 }
 
 - (void)setPinchGestureEnabled:(BOOL)pinchGestureEnabled
@@ -443,13 +443,12 @@ - (void)insertReactSubview:(UIView *)view atIndex:(NSInteger)atIndex
 {
   [super insertReactSubview:view atIndex:atIndex];
 #if !TARGET_OS_TV
-  if ([view isKindOfClass:[RCTRefreshControl class]]) {
-    [_scrollView setRctRefreshControl:(RCTRefreshControl *)view];
-  }
-  else if ([view conformsToProtocol:@protocol(UIScrollViewDelegate)]) {
-    [self addScrollListener:(UIView<UIScrollViewDelegate> *)view];
-    [_scrollView addSubview:view];
-    [_scrollView sendSubviewToBack:view];
+  if ([view conformsToProtocol:@protocol(RCTCustomRefreshContolProtocol)]) {
+    [_scrollView setCustomRefreshControl:(UIView<RCTCustomRefreshContolProtocol> *)view];
+    if (![view isKindOfClass:[UIRefreshControl class]]
+        && [view conformsToProtocol:@protocol(UIScrollViewDelegate)]) {
+      [self addScrollListener:(UIView<UIScrollViewDelegate> *)view];
+    }
   } else
 #endif
   {
@@ -464,11 +463,12 @@ - (void)removeReactSubview:(UIView *)subview
 {
   [super removeReactSubview:subview];
 #if !TARGET_OS_TV
-  if ([subview isKindOfClass:[RCTRefreshControl class]]) {
-    [_scrollView setRctRefreshControl:nil];
-  } else if ([subview conformsToProtocol:@protocol(UIScrollViewDelegate)]) {
-    [self removeScrollListener:(UIView<UIScrollViewDelegate> *)subview];
-    [subview removeFromSuperview];
+  if ([subview conformsToProtocol:@protocol(RCTCustomRefreshContolProtocol)]) {
+    [_scrollView setCustomRefreshControl:nil];
+    if (![subview isKindOfClass:[UIRefreshControl class]]
+        && [subview conformsToProtocol:@protocol(UIScrollViewDelegate)]) {
+      [self removeScrollListener:(UIView<UIScrollViewDelegate> *)subview];
+    }
   } else
 #endif
   {
@@ -519,8 +519,8 @@ - (void)layoutSubviews
 
 #if !TARGET_OS_TV
   // Adjust the refresh control frame if the scrollview layout changes.
-  RCTRefreshControl *refreshControl = _scrollView.rctRefreshControl;
-  if (refreshControl && refreshControl.refreshing) {
+  UIView<RCTCustomRefreshContolProtocol> *refreshControl = _scrollView.customRefreshControl;
+  if (refreshControl && refreshControl.isRefreshing) {
     refreshControl.frame = (CGRect){_scrollView.contentOffset, {_scrollView.frame.size.width, refreshControl.frame.size.height}};
   }
 #endif

diff --git a/React/Views/ScrollView/RCTScrollableProtocol.h b/React/Views/ScrollView/RCTScrollableProtocol.h
@@ -6,6 +6,7 @@
  */
 
 #import <UIKit/UIKit.h>
+#import <React/RCTComponent.h>
 
 /**
  * Contains any methods related to scrolling. Any `RCTView` that has scrolling
@@ -28,3 +29,13 @@
 - (void)removeScrollListener:(NSObject<UIScrollViewDelegate> *)scrollListener;
 
 @end
+
+/**
+ * Denotes a view which implements custom pull to refresh functionality.
+ */
+@protocol RCTCustomRefreshContolProtocol
+
+@property (nonatomic, copy) RCTDirectEventBlock onRefresh;
+@property (nonatomic, readonly, getter=isRefreshing) BOOL refreshing;
+
+@end