From 65184ec6b0ef2d136c0db239d65e0624efac8a2d Mon Sep 17 00:00:00 2001 From: Spencer Ahrens Date: Thu, 18 Jan 2018 13:51:43 -0800 Subject: [PATCH] rename and extend new maintain visible content position feature Summary: Builds off of https://github.com/facebook/react-native/commit/cae7179c9459f12b1cb5e1a1d998a9dc45f927dc - Make the prop a dictionary for more configuration options - Rename `maintainPositionAtOrBeyondIndex` -> `maintainVisibleContentPosition` + `minIndexForVisible` - Add autoscroll threshold feature Given the async native of RN JS and background layout, there is no way to trigger the scrollTo from JS without risking a delay, so we add the feature in native code. == Test Plan == ScrollViewExample: https://youtu.be/pmY8pxC9PRs Reviewed By: shergin Differential Revision: D6729160 fbshipit-source-id: 70f9bae460ce84567857a4f696da78ce9b3b834c --- Libraries/Components/ScrollView/ScrollView.js | 31 ++++++++++++++----- RNTester/js/ScrollViewExample.js | 14 ++++++--- React/Views/ScrollView/RCTScrollView.h | 2 +- React/Views/ScrollView/RCTScrollView.m | 29 ++++++++++++----- React/Views/ScrollView/RCTScrollViewManager.m | 2 +- 5 files changed, 56 insertions(+), 22 deletions(-) diff --git a/Libraries/Components/ScrollView/ScrollView.js b/Libraries/Components/ScrollView/ScrollView.js index 266e7283603d23..ff76758632ce59 100644 --- a/Libraries/Components/ScrollView/ScrollView.js +++ b/Libraries/Components/ScrollView/ScrollView.js @@ -234,18 +234,33 @@ const ScrollView = createReactClass({ */ keyboardShouldPersistTaps: PropTypes.oneOf(['always', 'never', 'handled', false, true]), /** - * When non-null, the scroll view will adjust the scroll position so that the content at or - * beyond the specified index that is currently visible will not change position. This is useful - * for lists that are loading content in both directions, e.g. a chat thread, where new messages - * coming in might otherwise cause the scroll position to jump. A value of 1 can be used to skip - * a spinner that does not need to maintain position. The default value is null. + * When set, the scroll view will adjust the scroll position so that the first child that is + * currently visible and at or beyond `minIndexForVisible` will not change position. This is + * useful for lists that are loading content in both directions, e.g. a chat thread, where new + * messages coming in might otherwise cause the scroll position to jump. A value of 0 is common, + * but other values such as 1 can be used to skip loading spinners or other content that should + * not maintain position. * - * Caveat: reordering elements in the scrollview with this enabled will probably cause jumpiness - * and jank. It can be fixed, but there are currently no plans to do so. + * The optional `autoscrollToTopThreshold` can be used to make the content automatically scroll + * to the top after making the adjustment if the user was within the threshold of the top before + * the adjustment was made. This is also useful for chat-like applications where you want to see + * new messages scroll into place, but not if the user has scrolled up a ways and it would be + * disruptive to scroll a bunch. + * + * Caveat 1: Reordering elements in the scrollview with this enabled will probably cause + * jumpiness and jank. It can be fixed, but there are currently no plans to do so. For now, + * don't re-order the content of any ScrollViews or Lists that use this feature. + * + * Caveat 2: This simply uses `contentOffset` and `frame.origin` in native code to compute + * visibility. Occlusion, transforms, and other complexity won't be taken into account as to + * whether content is "visible" or not. * * @platform ios */ - maintainPositionAtOrBeyondIndex: PropTypes.number, + maintainVisibleContentPosition: PropTypes.shape({ + minIndexForVisible: PropTypes.number.isRequired, + autoscrollToTopThreshold: PropTypes.number, + }), /** * The maximum allowed zoom scale. The default value is 1.0. * @platform ios diff --git a/RNTester/js/ScrollViewExample.js b/RNTester/js/ScrollViewExample.js index 2d0dcde3d88a31..49ea1fbefbe04b 100644 --- a/RNTester/js/ScrollViewExample.js +++ b/RNTester/js/ScrollViewExample.js @@ -131,7 +131,7 @@ if (Platform.OS === 'ios') { exports.examples.push({ title: ' smooth bi-directional content loading\n', description: - 'The `maintainPositionAtOrBeyondIndex` prop allows insertions to either end of the content ' + + 'The `maintainVisibleContentPosition` prop allows insertions to either end of the content ' + 'without causing the visible content to jump. Re-ordering is not supported.', render: function() { let itemCount = 6; @@ -146,7 +146,10 @@ if (Platform.OS === 'ios') { {this.state.items.map(item => @@ -156,9 +159,12 @@ if (Platform.OS === 'ios') { - + {this.state.items.map(item => React.cloneElement(item, {key: item.props.msg, style: null}), )} diff --git a/React/Views/ScrollView/RCTScrollView.h b/React/Views/ScrollView/RCTScrollView.h index cb3518bbf19d1a..dda27dfce04551 100644 --- a/React/Views/ScrollView/RCTScrollView.h +++ b/React/Views/ScrollView/RCTScrollView.h @@ -45,7 +45,7 @@ @property (nonatomic, assign) BOOL DEPRECATED_sendUpdatedChildFrames; @property (nonatomic, assign) NSTimeInterval scrollEventThrottle; @property (nonatomic, assign) BOOL centerContent; -@property (nonatomic, copy) NSNumber *maintainPositionAtOrBeyondIndex; +@property (nonatomic, copy) NSDictionary *maintainVisibleContentPosition; @property (nonatomic, assign) int snapToInterval; @property (nonatomic, copy) NSString *snapToAlignment; diff --git a/React/Views/ScrollView/RCTScrollView.m b/React/Views/ScrollView/RCTScrollView.m index 66cfa14f95a013..1f08f0c33747bd 100644 --- a/React/Views/ScrollView/RCTScrollView.m +++ b/React/Views/ScrollView/RCTScrollView.m @@ -911,16 +911,16 @@ - (void)updateContentOffsetIfNeeded } } -// maintainPositionAtOrBeyondIndex is used to allow seamless loading of content from both ends of +// maintainVisibleContentPosition is used to allow seamless loading of content from both ends of // the scrollview without the visible content jumping in position. -- (void)setMaintainPositionAtOrBeyondIndex:(NSNumber *)maintainPositionAtOrBeyondIndex +- (void)setMaintainVisibleContentPosition:(NSDictionary *)maintainVisibleContentPosition { - if (maintainPositionAtOrBeyondIndex != nil) { + if (maintainVisibleContentPosition != nil && _maintainVisibleContentPosition == nil) { [_eventDispatcher.bridge.uiManager.observerCoordinator addObserver:self]; - } else { + } else if (maintainVisibleContentPosition == nil && _maintainVisibleContentPosition != nil) { [_eventDispatcher.bridge.uiManager.observerCoordinator removeObserver:self]; } - _maintainPositionAtOrBeyondIndex = maintainPositionAtOrBeyondIndex; + _maintainVisibleContentPosition = maintainVisibleContentPosition; } #pragma mark - RCTUIManagerObserver @@ -930,7 +930,7 @@ - (void)uiManagerWillPerformMounting:(RCTUIManager *)manager RCTAssertUIManagerQueue(); [manager prependUIBlock:^(RCTUIManager *uiManager, NSDictionary *viewRegistry) { BOOL horz = [self isHorizontal:self->_scrollView]; - NSUInteger minIdx = [self->_maintainPositionAtOrBeyondIndex integerValue]; + NSUInteger minIdx = [self->_maintainVisibleContentPosition[@"minIndexForVisible"] integerValue]; for (NSUInteger ii = minIdx; ii < self->_contentView.subviews.count; ++ii) { // Find the first entirely visible view. This must be done after we update the content offset // or it will tend to grab rows that were made visible by the shift in position @@ -946,9 +946,10 @@ - (void)uiManagerWillPerformMounting:(RCTUIManager *)manager } }]; [manager addUIBlock:^(RCTUIManager *uiManager, NSDictionary *viewRegistry) { - if (self->_maintainPositionAtOrBeyondIndex == nil) { + if (self->_maintainVisibleContentPosition == nil) { return; // The prop might have changed in the previous UIBlocks, so need to abort here. } + NSNumber *autoscrollThreshold = self->_maintainVisibleContentPosition[@"autoscrollToTopThreshold"]; // TODO: detect and handle/ignore re-ordering if ([self isHorizontal:self->_scrollView]) { CGFloat deltaX = self->_firstVisibleView.frame.origin.x - self->_prevFirstVisibleFrame.origin.x; @@ -957,15 +958,27 @@ - (void)uiManagerWillPerformMounting:(RCTUIManager *)manager self->_scrollView.contentOffset.x + deltaX, self->_scrollView.contentOffset.y ); + if (autoscrollThreshold != nil) { + // If the offset WAS within the threshold of the start, animate to the start. + if (self->_scrollView.contentOffset.x - deltaX <= [autoscrollThreshold integerValue]) { + [self scrollToOffset:CGPointMake(0, self->_scrollView.contentOffset.y) animated:YES]; + } + } } } else { CGRect newFrame = self->_firstVisibleView.frame; CGFloat deltaY = newFrame.origin.y - self->_prevFirstVisibleFrame.origin.y; - if (ABS(deltaY) > 0.1 || deltaY != 0.0) { + if (ABS(deltaY) > 0.1) { self->_scrollView.contentOffset = CGPointMake( self->_scrollView.contentOffset.x, self->_scrollView.contentOffset.y + deltaY ); + if (autoscrollThreshold != nil) { + // If the offset WAS within the threshold of the start, animate to the start. + if (self->_scrollView.contentOffset.y - deltaY <= [autoscrollThreshold integerValue]) { + [self scrollToOffset:CGPointMake(self->_scrollView.contentOffset.x, 0) animated:YES]; + } + } } } }]; diff --git a/React/Views/ScrollView/RCTScrollViewManager.m b/React/Views/ScrollView/RCTScrollViewManager.m index f771fa551ff888..3c7d11405a74dd 100644 --- a/React/Views/ScrollView/RCTScrollViewManager.m +++ b/React/Views/ScrollView/RCTScrollViewManager.m @@ -62,7 +62,7 @@ - (UIView *)view RCT_EXPORT_VIEW_PROPERTY(bouncesZoom, BOOL) RCT_EXPORT_VIEW_PROPERTY(canCancelContentTouches, BOOL) RCT_EXPORT_VIEW_PROPERTY(centerContent, BOOL) -RCT_EXPORT_VIEW_PROPERTY(maintainPositionAtOrBeyondIndex, NSNumber) +RCT_EXPORT_VIEW_PROPERTY(maintainVisibleContentPosition, NSDictionary) RCT_EXPORT_VIEW_PROPERTY(automaticallyAdjustContentInsets, BOOL) RCT_EXPORT_VIEW_PROPERTY(decelerationRate, CGFloat) RCT_EXPORT_VIEW_PROPERTY(directionalLockEnabled, BOOL)