Skip to content

Commit 0564d43

Browse files
authored
fix(iOS): lack of animation on modal replacement (#2958)
## Description The fix boils down to removing check whether `changeRootIndex == controllers.count`. It was introduced here: #502. It can be represented differently: `changeRootIndex == controllers.lastIndex + 1` & if I get this correctly it checks whether the modal change in question takes place at the top of view hierarchy, with purpose of not animating non visible changes. It does not take account the replace operation though. In case of replace, the `changeRootIndex == controllers.lastIndex`. Another aspect is that I believe this part of the check is not necessary. If some modal view controller is being dismissed, **all** view controllers above it will also endup dismissed. Therefore, there is no scenario where this check would be useful. Hence, this commit ends up removing the check. ## Before & After | before | after | | --- | --- | | <video src="https://github.com/user-attachments/assets/d3a718e2-aef3-49bc-abdd-fa172c2097a3" /> | <video src="https://github.com/user-attachments/assets/e3b7c05c-fd9e-48b7-96e7-c59e75cd4c3f" /> | ## Test code and steps to reproduce I've enhanced `TestModalNavigation` with additional button that allows for modal replacement. I've also tested `TestModalNavigation` to make sure other flows keep working. I've also tested #502 - setting `animation: 'none'` keeps working for dismissed modals. ## Checklist - [x] Included code example that can be used to test this change - [ ] Ensured that CI passes
1 parent b6c325b commit 0564d43

File tree

2 files changed

+42
-20
lines changed

2 files changed

+42
-20
lines changed

apps/src/tests/TestModalNavigation.tsx

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import * as React from 'react';
22
import { View, Text, Button } from 'react-native';
33
import { NavigationContainer } from '@react-navigation/native';
4-
import { NativeStackScreenProps, createNativeStackNavigator } from '@react-navigation/native-stack';
4+
import {
5+
type NativeStackScreenProps,
6+
createNativeStackNavigator,
7+
} from '@react-navigation/native-stack';
58

69
type StackParamList = {
710
Home: undefined;
@@ -13,7 +16,6 @@ type StackParamList = {
1316
Screen3: undefined;
1417
};
1518

16-
1719
function HomeScreen({
1820
navigation,
1921
}: NativeStackScreenProps<StackParamList, 'Home'>) {
@@ -32,7 +34,9 @@ function HomeScreen({
3234
);
3335
}
3436

35-
function MainStackScreen({ navigation }: NativeStackScreenProps<StackParamList, 'MainStackScreen'>) {
37+
function MainStackScreen({
38+
navigation,
39+
}: NativeStackScreenProps<StackParamList, 'MainStackScreen'>) {
3640
return (
3741
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
3842
<Text>Main stack screen</Text>
@@ -41,21 +45,21 @@ function MainStackScreen({ navigation }: NativeStackScreenProps<StackParamList,
4145
onPress={() => navigation.navigate('MainStackScreen2')}
4246
/>
4347
<Button
44-
title="goBack"
45-
onPress={() => navigation.goBack()}
48+
title="Replace with the second modal"
49+
onPress={() => navigation.replace('MainStackScreen2')}
4650
/>
51+
<Button title="goBack" onPress={() => navigation.goBack()} />
4752
</View>
4853
);
4954
}
5055

51-
function MainStackScreen2({ navigation }: NativeStackScreenProps<StackParamList, 'MainStackScreen2'>) {
56+
function MainStackScreen2({
57+
navigation,
58+
}: NativeStackScreenProps<StackParamList, 'MainStackScreen2'>) {
5259
return (
5360
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
5461
<Text>Main stack screen 2</Text>
55-
<Button
56-
title="goBack"
57-
onPress={() => navigation.goBack()}
58-
/>
62+
<Button title="goBack" onPress={() => navigation.goBack()} />
5963
</View>
6064
);
6165
}
@@ -117,9 +121,21 @@ function RootStack() {
117121
return (
118122
<Stack.Navigator>
119123
<Stack.Screen name="Home" component={HomeScreen} />
120-
<Stack.Screen name="NestedStack" component={NestedStackScreen} options={{ headerShown: true, presentation: 'modal' }} />
121-
<Stack.Screen name="MainStackScreen" component={MainStackScreen} options={{ headerShown: true, presentation: 'modal' }} />
122-
<Stack.Screen name="MainStackScreen2" component={MainStackScreen2} options={{ headerShown: true, presentation: 'modal' }} />
124+
<Stack.Screen
125+
name="NestedStack"
126+
component={NestedStackScreen}
127+
options={{ headerShown: true, presentation: 'modal' }}
128+
/>
129+
<Stack.Screen
130+
name="MainStackScreen"
131+
component={MainStackScreen}
132+
options={{ headerShown: true, presentation: 'modal' }}
133+
/>
134+
<Stack.Screen
135+
name="MainStackScreen2"
136+
component={MainStackScreen2}
137+
options={{ headerShown: true, presentation: 'modal' }}
138+
/>
123139
</Stack.Navigator>
124140
);
125141
}

ios/RNSScreenStack.mm

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -429,8 +429,10 @@ - (void)setModalViewControllers:(NSArray<UIViewController *> *)controllers
429429
// (2) there are modals presented by other RNSNavigationControllers (nested/outer),
430430
// (3) there are modals presented by other controllers (e.g. React Native's Modal view).
431431

432-
// Last controller that is common for both _presentedModals & controllers
432+
// Last controller that is common for both _presentedModals & controllers or this RNSNavigationController in case
433+
// there is no common part.
433434
__block UIViewController *changeRootController = _controller;
435+
434436
// Last common controller index + 1
435437
NSUInteger changeRootIndex = 0;
436438
for (NSUInteger i = 0; i < MIN(_presentedModals.count, controllers.count); i++) {
@@ -533,12 +535,11 @@ - (void)setModalViewControllers:(NSArray<UIViewController *> *)controllers
533535
UIViewController *firstModalToBeDismissed = changeRootController.presentedViewController;
534536

535537
if (firstModalToBeDismissed != nil) {
536-
BOOL shouldAnimate = changeRootIndex == controllers.count &&
537-
[firstModalToBeDismissed isKindOfClass:[RNSScreen class]] &&
538-
((RNSScreen *)firstModalToBeDismissed).screenView.stackAnimation != RNSScreenStackAnimationNone;
538+
const BOOL firstModalToBeDismissedIsOwned = [firstModalToBeDismissed isKindOfClass:RNSScreen.class];
539+
const BOOL firstModalToBeDismissedIsOwnedByThisStack =
540+
firstModalToBeDismissedIsOwned && [_presentedModals containsObject:firstModalToBeDismissed];
539541

540-
if ([_presentedModals containsObject:firstModalToBeDismissed] ||
541-
![firstModalToBeDismissed isKindOfClass:RNSScreen.class]) {
542+
if (firstModalToBeDismissedIsOwnedByThisStack || !firstModalToBeDismissedIsOwned) {
542543
// We dismiss every VC that was presented by changeRootController VC or its descendant.
543544
// After the series of dismissals is completed we run completion block in which
544545
// we present modals on top of changeRootController (which may be the this stack VC)
@@ -548,7 +549,12 @@ - (void)setModalViewControllers:(NSArray<UIViewController *> *)controllers
548549
// For now, to mitigate the issue, we also decide to trigger its dismissal before
549550
// starting the presentation chain down below in finish() callback.
550551
if (!firstModalToBeDismissed.isBeingDismissed) {
551-
[changeRootController dismissViewControllerAnimated:shouldAnimate completion:finish];
552+
// If the modal is owned we let it control whether the dismissal is animated or not. For foreign controllers
553+
// we just assume animation.
554+
const BOOL firstModalToBeDismissedPrefersAnimation = firstModalToBeDismissedIsOwned
555+
? static_cast<RNSScreen *>(firstModalToBeDismissed).screenView.stackAnimation != RNSScreenStackAnimationNone
556+
: YES;
557+
[changeRootController dismissViewControllerAnimated:firstModalToBeDismissedPrefersAnimation completion:finish];
552558
} else {
553559
// We need to wait for its dismissal and then run our presentation code.
554560
// This happens, e.g. when we have foreign modal presented on top of owned one & we dismiss foreign one and

0 commit comments

Comments
 (0)