From a255204e3e7fddefd2d7b0de224101768757ca7a Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Mon, 11 Dec 2017 18:49:54 -0800 Subject: [PATCH] Removing `reactBridgeDidFinishTransaction` from RCTScrollView Summary: We are removing `reactBridgeDidFinishTransaction`. Why? * It is a performance drain. Supporting this requires dispatching main-thread block on every single transaction complete; * It has "too broad" non-conceptual semantic which encouraged using this as a "band-aid solution" for poorly designed components; * It is conceptually incompatible with new approaches that we are trying to implement to optimize the render layer; * It was deprecated for very long time. This diff removes `reactBridgeDidFinishTransaction` from RCTScrollView component. As I mentioned, because of the semantic of `reactBridgeDidFinishTransaction` is extremely broad, it's hard to capture what exact case it should handle. Based on comments and existing logic, it seems it tight to `contentSize` property and the size of RCTScrollContentView. Reviewed By: rsnara Differential Revision: D6538419 fbshipit-source-id: ccc6f5fea327471f10f1738d3da5214c0d362953 --- React/Views/RCTScrollContentView.h | 16 +++++++++++ React/Views/RCTScrollContentView.m | 35 +++++++++++++++++++++++ React/Views/RCTScrollContentViewManager.m | 6 ++++ React/Views/RCTScrollView.h | 6 ++++ React/Views/RCTScrollView.m | 9 +++++- 5 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 React/Views/RCTScrollContentView.h create mode 100644 React/Views/RCTScrollContentView.m diff --git a/React/Views/RCTScrollContentView.h b/React/Views/RCTScrollContentView.h new file mode 100644 index 00000000000..f25250cc5f8 --- /dev/null +++ b/React/Views/RCTScrollContentView.h @@ -0,0 +1,16 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#import + +#import + +@interface RCTScrollContentView : RCTView + +@end diff --git a/React/Views/RCTScrollContentView.m b/React/Views/RCTScrollContentView.m new file mode 100644 index 00000000000..a7fb9d2180e --- /dev/null +++ b/React/Views/RCTScrollContentView.m @@ -0,0 +1,35 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#import "RCTScrollContentView.h" + +#import +#import + +#import "RCTScrollView.h" + +@implementation RCTScrollContentView + +- (void)reactSetFrame:(CGRect)frame +{ + [super reactSetFrame:frame]; + + RCTScrollView *scrollView = (RCTScrollView *)self.superview.superview; + + if (!scrollView) { + return; + } + + RCTAssert([scrollView isKindOfClass:[RCTScrollView class]], + @"Unexpected view hierarchy of RCTScrollView component."); + + [scrollView updateContentOffsetIfNeeded]; +} + +@end diff --git a/React/Views/RCTScrollContentViewManager.m b/React/Views/RCTScrollContentViewManager.m index 5639b4fc9ef..18585fb88d1 100644 --- a/React/Views/RCTScrollContentViewManager.m +++ b/React/Views/RCTScrollContentViewManager.m @@ -10,11 +10,17 @@ #import "RCTScrollContentViewManager.h" #import "RCTScrollContentShadowView.h" +#import "RCTScrollContentView.h" @implementation RCTScrollContentViewManager RCT_EXPORT_MODULE() +- (RCTScrollContentView *)view +{ + return [RCTScrollContentView new]; +} + - (RCTShadowView *)shadowView { return [RCTScrollContentShadowView new]; diff --git a/React/Views/RCTScrollView.h b/React/Views/RCTScrollView.h index 091c8885b78..90b8e3700f0 100644 --- a/React/Views/RCTScrollView.h +++ b/React/Views/RCTScrollView.h @@ -59,6 +59,12 @@ @end +@interface RCTScrollView (Internal) + +- (void)updateContentOffsetIfNeeded; + +@end + @interface RCTEventDispatcher (RCTScrollView) /** diff --git a/React/Views/RCTScrollView.m b/React/Views/RCTScrollView.m index 96fea689c77..fb3d5f2efeb 100644 --- a/React/Views/RCTScrollView.m +++ b/React/Views/RCTScrollView.m @@ -466,6 +466,13 @@ static inline void RCTApplyTranformationAccordingLayoutDirection(UIView *view, U // Do nothing, as subviews are managed by `insertReactSubview:atIndex:` } +- (void)didSetProps:(NSArray *)changedProps +{ + if ([changedProps containsObject:@"contentSize"]) { + [self updateContentOffsetIfNeeded]; + } +} + - (BOOL)centerContent { return _scrollView.centerContent; @@ -882,7 +889,7 @@ RCT_SCROLL_EVENT_HANDLER(scrollViewDidZoom, onScroll) return _contentView.frame.size; } -- (void)reactBridgeDidFinishTransaction +- (void)updateContentOffsetIfNeeded { CGSize contentSize = self.contentSize; if (!CGSizeEqualToSize(_scrollView.contentSize, contentSize)) {