Skip to content

Commit

Permalink
Fix onMomentumScrollBegin not dispatching from animations (facebook#4…
Browse files Browse the repository at this point in the history
…7578)

Summary:

facebook#47468 originally introduced this change but it contained a few bugs in the order the events would fire. It was possible that `onMomentumScrollBegin` gets queued after `onMomentumScrollEnd` because the event would dispatch after calling `setContentOffset`. As a result, the ScrollView JS gets stuck in an "animating" state and jest runners did not bother scrolling further.

This change ensures that events fire in the correct order and that `onMomentumScrollBegin` is fired from programmatically-driven scroll events

## Changelog:
[iOS][Fixed] - Fixed `onMomentumScrollBegin` event not firing on command-driven scroll events

Reviewed By: sammy-SC

Differential Revision: D65846878
  • Loading branch information
Abbondanzo authored and facebook-github-bot committed Dec 19, 2024
1 parent ea56c43 commit 5c70673
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,12 @@ - (void)scrollToOffset:(CGPoint)offset animated:(BOOL)animated

[self _forceDispatchNextScrollEvent];

// Notify of momentum scroll begin before setting content offset or events can fire out of order and scrollview gets
// stuck in "animating" state
if (animated && _eventEmitter) {
static_cast<const ScrollViewEventEmitter &>(*_eventEmitter).onMomentumScrollBegin([self _scrollViewMetrics]);
}

[_scrollView setContentOffset:offset animated:animated];

if (!animated) {
Expand Down
10 changes: 10 additions & 0 deletions packages/react-native/React/Views/ScrollView/RCTScrollView.m
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,11 @@ - (void)scrollToOffset:(CGPoint)offset animated:(BOOL)animated
y = fmin(y, CGRectGetMaxY(maxRect));
offset = CGPointMake(x, y);
}
// Notify of momentum scroll begin before setting content offset or events can fire out of order and scrollview gets
// stuck in "animating" state
if (animated) {
[self sendScrollEventWithName:@"onMomentumScrollBegin" scrollView:_scrollView userData:nil];
}
[_scrollView setContentOffset:offset animated:animated];
}
}
Expand All @@ -611,6 +616,11 @@ - (void)scrollToEnd:(BOOL)animated
if (!CGPointEqualToPoint(_scrollView.contentOffset, offset)) {
// Ensure at least one scroll event will fire
_allowNextScrollNoMatterWhat = YES;
// Notify of momentum scroll begin before setting content offset or events can fire out of order and scrollview gets
// stuck in "animating" state
if (animated) {
[self sendScrollEventWithName:@"onMomentumScrollBegin" scrollView:_scrollView userData:nil];
}
[_scrollView setContentOffset:offset animated:animated];
}
}
Expand Down
12 changes: 11 additions & 1 deletion packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import RNTesterText from '../../components/RNTesterText';
import ScrollViewPressableStickyHeaderExample from './ScrollViewPressableStickyHeaderExample';
import nullthrows from 'nullthrows';
import * as React from 'react';
import {useCallback, useState} from 'react';
import {useCallback, useRef, useState} from 'react';
import {
Platform,
RefreshControl,
Expand Down Expand Up @@ -855,11 +855,21 @@ const OnScrollOptions = () => {
};

const OnMomentumScroll = () => {
const ref = useRef<?React.ElementRef<typeof ScrollView>>(null);
const [scroll, setScroll] = useState('none');
return (
<View>
<RNTesterText>Scroll State: {scroll}</RNTesterText>
<Button
label="scrollTo top (animated)"
onPress={() => ref.current?.scrollTo({x: 0, y: 0, animated: true})}
/>
<Button
label="scrollTo top (not animated)"
onPress={() => ref.current?.scrollTo({x: 0, y: 0, animated: false})}
/>
<ScrollView
ref={ref}
style={[styles.scrollView, {height: 200}]}
onMomentumScrollBegin={() => setScroll('onMomentumScrollBegin')}
onMomentumScrollEnd={() => setScroll('onMomentumScrollEnd')}
Expand Down

0 comments on commit 5c70673

Please sign in to comment.