Skip to content

Commit fa08d3d

Browse files
Abbondanzofacebook-github-bot
authored andcommitted
Fix idle momentum end on iOS
Summary: Unmounting an idle `ScrollView` could incorrectly dispatch `onMomentumScrollEnd` because a non-tracking scroll view was treated as moving. Check `UIScrollView.isDecelerating` before the view leaves its window so the event is emitted only when momentum is active. Add native regression coverage plus a shared RNTester Maestro flow. The flow runs on Android and iOS and verifies an idle unmount emits no momentum event, one fling emits exactly one begin/end pair, and a later unmount does not increment the count. Changelog: [iOS][Fixed] - Prevent idle `ScrollView`s from firing `onMomentumScrollEnd` during unmount Differential Revision: D121477503
1 parent b353db7 commit fa08d3d

4 files changed

Lines changed: 107 additions & 14 deletions

File tree

‎packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -844,19 +844,19 @@ - (void)scrollViewDidEndScrollingAnimation:(UIScrollView *)scrollView
844844
[self _handleFinishedScrolling:scrollView];
845845
}
846846

847-
- (void)didMoveToWindow
847+
- (void)willMoveToWindow:(UIWindow *)newWindow
848848
{
849-
[super didMoveToWindow];
849+
[super willMoveToWindow:newWindow];
850850

851-
if (!self.window) {
851+
if (!newWindow) {
852852
// The view is being removed, ensure that the scroll end event is dispatched
853853
[self _handleScrollEndIfNeeded];
854854
}
855855
}
856856

857857
- (void)_handleScrollEndIfNeeded
858858
{
859-
if (_scrollView.isDecelerating || !_scrollView.isTracking) {
859+
if (_scrollView.isDecelerating) {
860860
if (!_eventEmitter) {
861861
return;
862862
}

‎packages/react-native/React/Tests/Mounting/RCTScrollViewComponentViewTests.mm‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,13 @@
77

88
#import <React/RCTScrollViewComponentView.h>
99
#import <XCTest/XCTest.h>
10+
#import <react/renderer/components/scrollview/ScrollViewEventEmitter.h>
1011
#import <react/renderer/components/scrollview/ScrollViewProps.h>
1112
#import <react/renderer/components/scrollview/ScrollViewShadowNode.h>
1213

14+
using facebook::react::EventDispatcher;
1315
using facebook::react::Props;
16+
using facebook::react::ScrollViewEventEmitter;
1417
using facebook::react::ScrollViewProps;
1518
using facebook::react::ScrollViewShadowNode;
1619

@@ -61,6 +64,21 @@ - (void)testAutomaticallyAdjustKeyboardInsetsAcrossRecycling
6164
XCTAssertEqual(view.scrollView.contentInset.bottom, 50);
6265
}
6366

67+
- (void)testUnmountingIdleScrollViewDoesNotEndMomentum
68+
{
69+
UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 100, 100)];
70+
RCTScrollViewComponentView *view = [[RCTScrollViewComponentView alloc] initWithFrame:window.bounds];
71+
[window addSubview:view];
72+
73+
auto eventEmitter = std::make_shared<ScrollViewEventEmitter>(nullptr, EventDispatcher::Weak{});
74+
[view updateEventEmitter:eventEmitter];
75+
[view setValue:@YES forKey:@"isUserTriggeredScrolling"];
76+
77+
[view removeFromSuperview];
78+
79+
XCTAssertTrue([[view valueForKey:@"isUserTriggeredScrolling"] boolValue]);
80+
}
81+
6482
@end
6583

6684
#endif
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
appId: ${APP_ID} # iOS: com.meta.RNTester.localDevelopment | Android: com.facebook.react.uiapp
2+
---
3+
- launchApp
4+
- stopApp
5+
- openLink: rntester://example/ScrollViewExample/onMomentumScroll
6+
- runFlow: ./helpers/confirm-open-link.yml
7+
- extendedWaitUntil:
8+
visible:
9+
id: 'momentum-scroll-view'
10+
timeout: 120000
11+
- assertVisible:
12+
id: 'momentum-scroll-begin-count'
13+
text: 'onMomentumScrollBegin called 0 times'
14+
- assertVisible:
15+
id: 'momentum-scroll-end-count'
16+
text: 'onMomentumScrollEnd called 0 times'
17+
# Removing an idle ScrollView must not synthesize a momentum-end event.
18+
- tapOn:
19+
id: 'toggle-momentum-scroll-view'
20+
- assertNotVisible:
21+
id: 'momentum-scroll-view'
22+
- waitForAnimationToEnd:
23+
timeout: 1000
24+
- assertVisible:
25+
id: 'momentum-scroll-end-count'
26+
text: 'onMomentumScrollEnd called 0 times'
27+
- tapOn:
28+
id: 'toggle-momentum-scroll-view'
29+
- assertVisible:
30+
id: 'momentum-scroll-view'
31+
# One momentum scroll must produce exactly one begin and one end event.
32+
- swipe:
33+
start: 50%, 55%
34+
end: 50%, 25%
35+
speed: fast
36+
- waitForAnimationToEnd:
37+
timeout: 5000
38+
- assertVisible:
39+
id: 'momentum-scroll-begin-count'
40+
text: 'onMomentumScrollBegin called 1 times'
41+
- assertVisible:
42+
id: 'momentum-scroll-end-count'
43+
text: 'onMomentumScrollEnd called 1 times'
44+
# Unmounting after momentum has ended must not increment the count again.
45+
- tapOn:
46+
id: 'toggle-momentum-scroll-view'
47+
- waitForAnimationToEnd:
48+
timeout: 1000
49+
- assertVisible:
50+
id: 'momentum-scroll-begin-count'
51+
text: 'onMomentumScrollBegin called 1 times'
52+
- assertVisible:
53+
id: 'momentum-scroll-end-count'
54+
text: 'onMomentumScrollEnd called 1 times'

‎packages/rn-tester/js/examples/ScrollView/ScrollViewExample.js‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -382,9 +382,10 @@ const examples: Array<RNTesterModuleExample> = [
382382
},
383383
},
384384
{
385+
name: 'onMomentumScroll',
385386
title: '<ScrollView> OnMomentumScroll\n',
386387
description:
387-
'An alert will be called when the momentum scroll starts or ends.',
388+
'Counts momentum scroll events and supports unmounting the ScrollView.',
388389
render(): React.Node {
389390
return <OnMomentumScroll />;
390391
},
@@ -920,17 +921,37 @@ const OnScrollOptions = () => {
920921
};
921922

922923
const OnMomentumScroll = () => {
923-
const [scroll, setScroll] = useState('none');
924+
const [scrollViewMounted, setScrollViewMounted] = useState(true);
925+
const [momentumScrollBeginCount, setMomentumScrollBeginCount] = useState(0);
926+
const [momentumScrollEndCount, setMomentumScrollEndCount] = useState(0);
927+
924928
return (
925929
<View>
926-
<RNTesterText>Scroll State: {scroll}</RNTesterText>
927-
<ScrollView
928-
style={[styles.scrollView, {height: 200}]}
929-
onMomentumScrollBegin={() => setScroll('onMomentumScrollBegin')}
930-
onMomentumScrollEnd={() => setScroll('onMomentumScrollEnd')}
931-
nestedScrollEnabled>
932-
{ITEMS.map(createItemRow)}
933-
</ScrollView>
930+
<RNTesterText testID="momentum-scroll-begin-count">
931+
onMomentumScrollBegin called {momentumScrollBeginCount} times
932+
</RNTesterText>
933+
<RNTesterText testID="momentum-scroll-end-count">
934+
onMomentumScrollEnd called {momentumScrollEndCount} times
935+
</RNTesterText>
936+
<Button
937+
label={scrollViewMounted ? 'Unmount ScrollView' : 'Mount ScrollView'}
938+
onPress={() => setScrollViewMounted(mounted => !mounted)}
939+
testID="toggle-momentum-scroll-view"
940+
/>
941+
{scrollViewMounted ? (
942+
<ScrollView
943+
style={styles.scrollView}
944+
onMomentumScrollBegin={() =>
945+
setMomentumScrollBeginCount(count => count + 1)
946+
}
947+
onMomentumScrollEnd={() =>
948+
setMomentumScrollEndCount(count => count + 1)
949+
}
950+
testID="momentum-scroll-view"
951+
nestedScrollEnabled>
952+
{ITEMS.map(createItemRow)}
953+
</ScrollView>
954+
) : null}
934955
</View>
935956
);
936957
};

0 commit comments

Comments
 (0)