Skip to content

Commit

Permalink
Removing reactBridgeDidFinishTransaction from RCTScrollView
Browse files Browse the repository at this point in the history
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
  • Loading branch information
shergin authored and facebook-github-bot committed Dec 12, 2017
1 parent cb49877 commit a255204
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 1 deletion.
16 changes: 16 additions & 0 deletions React/Views/RCTScrollContentView.h
Original file line number Diff line number Diff line change
@@ -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 <UIKit/UIKit.h>

#import <React/RCTView.h>

@interface RCTScrollContentView : RCTView

@end
35 changes: 35 additions & 0 deletions React/Views/RCTScrollContentView.m
Original file line number Diff line number Diff line change
@@ -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 <React/RCTAssert.h>
#import <React/UIView+React.h>

#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
6 changes: 6 additions & 0 deletions React/Views/RCTScrollContentViewManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
6 changes: 6 additions & 0 deletions React/Views/RCTScrollView.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@

@end

@interface RCTScrollView (Internal)

- (void)updateContentOffsetIfNeeded;

@end

@interface RCTEventDispatcher (RCTScrollView)

/**
Expand Down
9 changes: 8 additions & 1 deletion React/Views/RCTScrollView.m
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,13 @@ - (void)didUpdateReactSubviews
// Do nothing, as subviews are managed by `insertReactSubview:atIndex:`
}

- (void)didSetProps:(NSArray<NSString *> *)changedProps
{
if ([changedProps containsObject:@"contentSize"]) {
[self updateContentOffsetIfNeeded];
}
}

- (BOOL)centerContent
{
return _scrollView.centerContent;
Expand Down Expand Up @@ -882,7 +889,7 @@ - (CGSize)contentSize
return _contentView.frame.size;
}

- (void)reactBridgeDidFinishTransaction
- (void)updateContentOffsetIfNeeded
{
CGSize contentSize = self.contentSize;
if (!CGSizeEqualToSize(_scrollView.contentSize, contentSize)) {
Expand Down

0 comments on commit a255204

Please sign in to comment.