Skip to content

Fix invalid onScroll events breaking FlatList #1873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 13, 2023
Merged

Fix invalid onScroll events breaking FlatList #1873

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions React/Views/ScrollView/RCTScrollView.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ @implementation RCTScrollView {
BOOL _allowNextScrollNoMatterWhat;
#if TARGET_OS_OSX // [macOS
BOOL _notifyDidScroll;
NSPoint _lastDocumentOrigin;
NSPoint _lastScrollPosition;
#endif // macOS]
CGRect _lastClippedToRect;
uint16_t _coalescingKey;
Expand Down Expand Up @@ -476,7 +476,7 @@ - (instancetype)initWithEventDispatcher:(id<RCTEventDispatcherProtocol>)eventDis
_scrollView.delaysContentTouches = NO;
#else // [macOS
_scrollView.postsBoundsChangedNotifications = YES;
_lastDocumentOrigin = NSZeroPoint;
_lastScrollPosition = NSZeroPoint;
#endif // macOS]

#if !TARGET_OS_OSX // [macOS]
Expand Down Expand Up @@ -877,9 +877,7 @@ - (void)scrollViewDocumentViewBoundsDidChange:(__unused NSNotification *)notific
[_scrollView setContentOffset:_scrollView.contentOffset];
}

// only send didScroll event if scrollView is ready or document origin changed
BOOL didScroll = !NSEqualPoints(_scrollView.documentView.frame.origin, _lastDocumentOrigin);
if (_notifyDidScroll && didScroll) {
if (_notifyDidScroll) {
[self scrollViewDidScroll:_scrollView];
}
}
Expand Down Expand Up @@ -925,11 +923,24 @@ - (void)removeScrollListener:(NSObject<UIScrollViewDelegate> *)scrollListener

- (void)scrollViewDidScroll:(RCTCustomScrollView *)scrollView // [macOS]
{
#if TARGET_OS_OSX // [macOS
_lastDocumentOrigin = scrollView.documentView.frame.origin;
#endif // macOS]
NSTimeInterval now = CACurrentMediaTime();
[self updateClippedSubviews];

#if TARGET_OS_OSX // [macOS
/**
* To check for effective scroll position changes, the comparison with lastScrollPosition should happen
* after updateClippedSubviews. updateClippedSubviews will update the display of the vertical/horizontal
* scrollers which can change the clipview bounds.
* This change also ensures that no onScroll events are sent when the React setFrame call is running,
* which could submit onScroll events while the content view was not setup yet.
*/
BOOL didScroll = !NSEqualPoints(scrollView.contentView.bounds.origin, _lastScrollPosition);
if (!didScroll) {
return;
}
_lastScrollPosition = scrollView.contentView.bounds.origin;
#endif // macOS]

/**
* TODO: this logic looks wrong, and it may be because it is. Currently, if _scrollEventThrottle
* is set to zero (the default), the "didScroll" event is only sent once per scroll, instead of repeatedly
Expand Down