Skip to content

Commit 671b975

Browse files
janicduplessisfacebook-github-bot-6
authored andcommitted
Fix a bug with ListView with sticky headers + RefreshControl
Summary:The bug is caused by a weird race condition. What happens is that when calling `UIRefreshControl#endRefreshing` the `UIScrollView` delegate `scrollViewDidScroll` function is called synchronously and then `dockClosestSectionHeader` crashes because the sticky header indexes are updated but not the contentView children. I fixed it by adding an updating property on `RCTRefreshControl` and setting it before calling `endRefreshing` so we can know not to call `dockClosestSectionHeader` at that moment. Tested with both `RefreshControl` and `onRefreshStart` prop. I reproduced the bug by replacing ListViewExample.js in UIExplorer with https://gist.github.com/janicduplessis/05fc58e852f3e80e51b9 Fixes react#5440 cc nicklockwood Closes react#5445 Differential Revision: D2953984 Pulled By: nicklockwood fb-gh-sync-id: c17a6a75ab31ef54d478706ed17a8115a11d734e shipit-source-id: c17a6a75ab31ef54d478706ed17a8115a11d734e
1 parent 183d6a0 commit 671b975

2 files changed

Lines changed: 20 additions & 9 deletions

File tree

React/Views/RCTRefreshControl.m

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ - (instancetype)init
3030
- (void)layoutSubviews
3131
{
3232
[super layoutSubviews];
33-
33+
3434
// If the control is refreshing when mounted we need to call
3535
// beginRefreshing in layoutSubview or it doesn't work.
3636
if (_isInitialRender && _initialRefreshingState) {
@@ -46,9 +46,7 @@ - (void)beginRefreshing
4646
CGPoint offset = {scrollView.contentOffset.x, scrollView.contentOffset.y - self.frame.size.height};
4747
// Don't animate when the prop is set initialy.
4848
if (_isInitialRender) {
49-
// Must use `[scrollView setContentOffset:offset animated:NO]` instead of just setting
50-
// `scrollview.contentOffset` or it doesn't work, don't ask me why!
51-
[scrollView setContentOffset:offset animated:NO];
49+
scrollView.contentOffset = offset;
5250
[super beginRefreshing];
5351
} else {
5452
// `beginRefreshing` must be called after the animation is done. This is why it is impossible

React/Views/RCTScrollView.m

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ @interface RCTCustomScrollView : UIScrollView<UIGestureRecognizerDelegate>
145145

146146
@property (nonatomic, copy) NSIndexSet *stickyHeaderIndices;
147147
@property (nonatomic, assign) BOOL centerContent;
148-
@property (nonatomic, strong) UIRefreshControl *refreshControl;
148+
@property (nonatomic, strong) RCTRefreshControl *refreshControl;
149149

150150
@end
151151

@@ -287,10 +287,12 @@ - (void)dockClosestSectionHeader
287287
__block UIView *nextHeader = nil;
288288
NSUInteger subviewCount = contentView.reactSubviews.count;
289289
[_stickyHeaderIndices enumerateIndexesWithOptions:0 usingBlock:
290-
^(NSUInteger idx, __unused BOOL *stop) {
290+
^(NSUInteger idx, BOOL *stop) {
291291

292+
// If the subviews are out of sync with the sticky header indices don't
293+
// do anything.
292294
if (idx >= subviewCount) {
293-
RCTLogError(@"Sticky header index %zd was outside the range {0, %zd}", idx, subviewCount);
295+
*stop = YES;
294296
return;
295297
}
296298

@@ -365,7 +367,7 @@ - (UIView *)hitTest:(CGPoint)point withEvent:(UIEvent *)event
365367
return hitView ?: [super hitTest:point withEvent:event];
366368
}
367369

368-
- (void)setRefreshControl:(UIRefreshControl *)refreshControl
370+
- (void)setRefreshControl:(RCTRefreshControl *)refreshControl
369371
{
370372
if (_refreshControl) {
371373
[_refreshControl removeFromSuperview];
@@ -826,6 +828,17 @@ - (void)reactBridgeDidFinishTransaction
826828
_scrollView.contentSize = contentSize;
827829
_scrollView.contentOffset = newOffset;
828830
}
831+
832+
if (RCT_DEBUG) {
833+
// Validate that sticky headers are not out of range.
834+
NSUInteger subviewCount = _scrollView.contentView.reactSubviews.count;
835+
NSUInteger lastIndex = _scrollView.stickyHeaderIndices.lastIndex;
836+
if (lastIndex != NSNotFound && lastIndex >= subviewCount) {
837+
RCTLogWarn(@"Sticky header index %zd was outside the range {0, %zd}",
838+
lastIndex, subviewCount);
839+
}
840+
}
841+
829842
[_scrollView dockClosestSectionHeader];
830843
}
831844

@@ -888,7 +901,7 @@ - (void)setOnRefreshStart:(RCTDirectEventBlock)onRefreshStart
888901
_onRefreshStart = [onRefreshStart copy];
889902

890903
if (!_scrollView.refreshControl) {
891-
UIRefreshControl *refreshControl = [[UIRefreshControl alloc] init];
904+
RCTRefreshControl *refreshControl = [[RCTRefreshControl alloc] init];
892905
[refreshControl addTarget:self action:@selector(refreshControlValueChanged) forControlEvents:UIControlEventValueChanged];
893906
_scrollView.refreshControl = refreshControl;
894907
}

0 commit comments

Comments
 (0)