Skip to content

Commit 1f7c84c

Browse files
zetavgkkafar
authored andcommitted
fix(iOS): onNativeDismissCancelled called too early during modal dismissal (software-mansion#2129)
## Description Currently, on iOS, `onNativeDismissCancelled` is being called while the user begins to drag down a modal (which calls the callback function of the `usePreventRemove` hook, if `preventRemove` is `true`). This is not a common behavior of iOS apps and can sometimes be annoying: the callback will be triggered while the user is scrolling around a `ScrollView` in the modal and has hit the top, which is interrupting. | Ideal | Before | After | |-------|--------|-------| | <a href="https://github.com/software-mansion/react-native-screens/assets/3784687/167828c8-eb3a-4b4b-99a7-b9eb7ec55f06"><img alt="Ideal" width="240" src="https://github.com/software-mansion/react-native-screens/assets/3784687/fca2e909-a523-422a-80a5-3ac301539486" /></a> | <a href="https://github.com/software-mansion/react-native-screens/assets/3784687/6c166757-8c73-4e90-9236-9faf84514a14"><img alt="Before" width="240" src="https://github.com/software-mansion/react-native-screens/assets/3784687/812426eb-5e5d-4da5-855b-8ff6d1af14ef" /></a> | <a href="https://github.com/software-mansion/react-native-screens/assets/3784687/7993f36c-3455-4b23-8a28-f46956655f8a"><img alt="After" width="240" src="https://github.com/software-mansion/react-native-screens/assets/3784687/62ab528a-4675-44fe-a5ae-682c2d155817" /></a> | | The event fires when the user **releses** the drag. If the user did not intend to dismiss the modal (the user did not drag it down far enough or dragged it back to its original position before release), the event will not fire. | The event fires as soon as the modal is dragged down, regardless of whether the user intends to dismiss the modal or not. (In this case, the user just wants to scroll to the top.) | Same as "Ideal". | This PR adjusts this to the more ideal behavior. ## Changes Move the call of `notifyDismissCancelled` from [`presentationControllerShouldDismiss`](https://developer.apple.com/documentation/uikit/uiadaptivepresentationcontrollerdelegate/3229890-presentationcontrollershoulddism) to [`presentationControllerDidAttemptToDismiss`](https://developer.apple.com/documentation/uikit/uiadaptivepresentationcontrollerdelegate/3229888-presentationcontrollerdidattempt) - which "Notifies the delegate that a user-initiated attempt to dismiss a view was prevented", and has the same platform avalibality as `presentationControllerShouldDismiss`. ## Test code and steps to reproduce Use the new "Prevent Remove" example: <img width="300" src="https://github.com/software-mansion/react-native-screens/assets/3784687/622673dc-d520-4828-a2c3-83de7f2e3bef" /> ### Full test recordings **Modal** https://github.com/software-mansion/react-native-screens/assets/3784687/16777817-3949-413c-83ad-f0f7136af158 **Normal Screen** (this should not be affected since I believe that preventing going back from a normal screen is not handled here) https://github.com/software-mansion/react-native-screens/assets/3784687/41f25247-eb54-4ce8-9348-690d0a925e8f ## Checklist - [x] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation: <!-- For adding new props to native-stack --> - [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [ ] Ensured that CI passes --------- Co-authored-by: Kacper Kafara <[email protected]>
1 parent fdb967e commit 1f7c84c

File tree

3 files changed

+147
-14
lines changed

3 files changed

+147
-14
lines changed

apps/Example.tsx

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { ListItem, SettingsSwitch, ThemedText } from './src/shared';
2222
import SimpleNativeStack from './src/screens/SimpleNativeStack';
2323
import SwipeBackAnimation from './src/screens/SwipeBackAnimation';
2424
import StackPresentation from './src/screens/StackPresentation';
25+
import PreventRemove from './src/screens/PreventRemove';
2526
import HeaderOptions from './src/screens/HeaderOptions';
2627
import StatusBarExample from './src/screens/StatusBar';
2728
import Animations from './src/screens/Animations';
@@ -80,6 +81,11 @@ const SCREENS: Record<
8081
type: 'example',
8182
isTVOSReady: true,
8283
},
84+
PreventRemove: {
85+
title: 'Prevent Remove',
86+
component: PreventRemove,
87+
type: 'example',
88+
},
8389
HeaderOptions: {
8490
title: 'Header Options',
8591
component: HeaderOptions,
@@ -132,8 +138,8 @@ const playgrounds = screens.filter(name => SCREENS[name].type === 'playground');
132138
type RootStackParamList = {
133139
Main: undefined;
134140
} & {
135-
[P in keyof typeof SCREENS]: undefined;
136-
};
141+
[P in keyof typeof SCREENS]: undefined;
142+
};
137143

138144
const Stack = createNativeStackNavigator<RootStackParamList>();
139145

@@ -208,9 +214,8 @@ const ExampleApp = (): React.JSX.Element => {
208214
<Stack.Screen
209215
name="Main"
210216
options={{
211-
title: `${
212-
Platform.isTV ? '📺' : '📱'
213-
} React Native Screens Examples`,
217+
title: `${Platform.isTV ? '📺' : '📱'
218+
} React Native Screens Examples`,
214219
}}
215220
component={MainScreen}
216221
/>

apps/src/screens/PreventRemove.tsx

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import React from 'react';
2+
import { View, StyleSheet, TextInput, Platform, Alert, ScrollView, Text } from 'react-native';
3+
import { usePreventRemove } from '@react-navigation/native';
4+
import {
5+
createNativeStackNavigator,
6+
NativeStackNavigationProp,
7+
} from '@react-navigation/native-stack';
8+
import { Button } from '../shared';
9+
10+
type StackParamList = {
11+
Main: undefined;
12+
PreventRemoveScreen: undefined;
13+
PreventRemoveModal: undefined;
14+
};
15+
16+
interface MainScreenProps {
17+
navigation: NativeStackNavigationProp<StackParamList, 'Main'>;
18+
}
19+
20+
const MainScreen = ({ navigation }: MainScreenProps): JSX.Element => (
21+
<View style={{ ...styles.container, backgroundColor: 'bisque' }}>
22+
<Button
23+
title="Go to screen"
24+
onPress={() => navigation.navigate('PreventRemoveScreen')}
25+
/>
26+
<Button
27+
title="Open modal"
28+
onPress={() => navigation.navigate('PreventRemoveModal')}
29+
/>
30+
<Button onPress={() => navigation.pop()} title="🔙 Back to Examples" />
31+
</View>
32+
);
33+
34+
const PreventRemoveScreen = ({
35+
navigation,
36+
}: {
37+
navigation: NativeStackNavigationProp<StackParamList>;
38+
}): JSX.Element => {
39+
const [text, setText] = React.useState('');
40+
41+
const hasUnsavedChanges = Boolean(text);
42+
43+
usePreventRemove(hasUnsavedChanges, ({ data }) => {
44+
Alert.alert(
45+
'Discard changes?',
46+
'You have unsaved changes. Discard them and leave the screen?',
47+
[
48+
{ text: "Don't leave", style: 'cancel', onPress: () => { } },
49+
{
50+
text: 'Discard',
51+
style: 'destructive',
52+
onPress: () => navigation.dispatch(data.action),
53+
},
54+
],
55+
);
56+
});
57+
58+
return (
59+
<View style={{ ...styles.container, backgroundColor: 'thistle' }}>
60+
<ScrollView
61+
contentContainerStyle={styles.contentContainer}
62+
keyboardDismissMode="interactive"
63+
>
64+
<TextInput
65+
autoFocus
66+
value={text}
67+
placeholder="Type something to prevent remove…"
68+
placeholderTextColor="#999"
69+
onChangeText={setText}
70+
/>
71+
<Button
72+
title="Discard and go back"
73+
onPress={() => navigation.goBack()}
74+
/>
75+
{Array.from({ length: 10 }).map((_, i) => (
76+
<Text key={`lorem-ipsum-${i}`} style={styles.loremIpsum}>
77+
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do
78+
eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim
79+
ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut
80+
aliquip ex ea commodo consequat. Duis aute irure dolor in
81+
reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla
82+
pariatur. Excepteur sint occaecat cupidatat non proident, sunt in
83+
culpa qui officia deserunt mollit anim id est laborum.
84+
</Text>
85+
))}
86+
</ScrollView>
87+
</View>
88+
);
89+
};
90+
91+
const Stack = createNativeStackNavigator<StackParamList>();
92+
93+
const App = (): JSX.Element => (
94+
<Stack.Navigator>
95+
<Stack.Screen
96+
name="Main"
97+
component={MainScreen}
98+
options={{ title: 'Prevent Remove' }}
99+
/>
100+
<Stack.Screen name="PreventRemoveScreen" component={PreventRemoveScreen} />
101+
<Stack.Screen
102+
name="PreventRemoveModal"
103+
component={PreventRemoveScreen}
104+
options={{ presentation: 'modal' }}
105+
/>
106+
</Stack.Navigator>
107+
);
108+
109+
const styles = StyleSheet.create({
110+
container: {
111+
flex: 1,
112+
},
113+
contentContainer: {
114+
paddingTop: 10,
115+
paddingHorizontal: 10,
116+
},
117+
loremIpsum: {
118+
marginVertical: 8,
119+
fontSize: 16,
120+
color: 'gray',
121+
},
122+
});
123+
124+
export default App;

ios/RNSScreen.mm

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -514,14 +514,6 @@ - (void)notifyTransitionProgress:(double)progress closing:(BOOL)closing goingFor
514514
#endif
515515
}
516516

517-
#if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && defined(__IPHONE_13_0) && \
518-
__IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_13_0
519-
- (void)presentationControllerDidAttemptToDismiss:(UIPresentationController *)presentationController
520-
{
521-
[self notifyGestureCancel];
522-
}
523-
#endif
524-
525517
- (void)presentationControllerWillDismiss:(UIPresentationController *)presentationController
526518
{
527519
// We need to call both "cancel" and "reset" here because RN's gesture recognizer
@@ -546,12 +538,24 @@ - (void)presentationControllerWillDismiss:(UIPresentationController *)presentati
546538
- (BOOL)presentationControllerShouldDismiss:(UIPresentationController *)presentationController
547539
{
548540
if (_preventNativeDismiss) {
549-
[self notifyDismissCancelledWithDismissCount:1];
550541
return NO;
551542
}
552543
return _gestureEnabled;
553544
}
554545

546+
#if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && defined(__IPHONE_13_0) && \
547+
__IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_13_0
548+
- (void)presentationControllerDidAttemptToDismiss:(UIPresentationController *)presentationController
549+
{
550+
// NOTE(kkafar): We should consider depracating the use of gesture cancel here & align
551+
// with usePreventRemove API of react-navigation v7.
552+
[self notifyGestureCancel];
553+
if (_preventNativeDismiss) {
554+
[self notifyDismissCancelledWithDismissCount:1];
555+
}
556+
}
557+
#endif
558+
555559
- (void)presentationControllerDidDismiss:(UIPresentationController *)presentationController
556560
{
557561
if ([_reactSuperview respondsToSelector:@selector(presentationControllerDidDismiss:)]) {

0 commit comments

Comments
 (0)