Skip to content

Commit

Permalink
fix(iOS, Paper): prevent double modal dismissal (#2568)
Browse files Browse the repository at this point in the history
## Description

Fixes #2525

View hierarchy for modal with header looks kinda like this:

`UITransitionView` -> `Screen` -> `Stack` (`UINavigationController`) ->
`Screen` -> content

calling `[_controller dismissViewControllerAnimated:YES completion:nil]`
on the `Stack` ☝️, according to
[documentation](https://developer.apple.com/documentation/uikit/uiviewcontroller/dismiss(animated:completion:)?language=objc)
should dismiss the view controller itself and any view controllers
presented from that view controller. However in practive it dismisses
not only itself and "above" modal, but also a single modal "below". I'm
not sure
why it is the case. Removing this line of code introduces a bug: during
development, when multiple modals are opened and you reload the
react-native, the modals could be left in stuck state. I'll proceed, as
this bug
bug in development is of much lesser severity. 

I've decided to move the call "one up" the view controller hierarchy, to
avoid calling dismiss directly on UINavigationController as it looks
like it causes the problem. This fixes the problem & keeps the
development behaviour working as intended.

## Changes

Described above ☝️

## Test code and steps to reproduce

I've enhanced `TestModalNavigation` test case to cover this issue. 

## Checklist

- [x] Included code example that can be used to test this change
- [ ] Ensured that CI passes
  • Loading branch information
kkafar authored Dec 11, 2024
1 parent 8b9b9f2 commit 2152383
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 4 deletions.
25 changes: 22 additions & 3 deletions apps/src/tests/TestModalNavigation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ type StackParamList = {
Home: undefined;
NestedStack: undefined;
MainStackScreen: undefined;
MainStackScreen2: undefined;
Screen1: undefined;
Screen2: undefined;
Screen3: undefined;
};


function HomeScreen({
navigation,
}: NativeStackScreenProps<StackParamList, 'Home'>) {
Expand All @@ -34,6 +36,22 @@ function MainStackScreen({ navigation }: NativeStackScreenProps<StackParamList,
return (
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
<Text>Main stack screen</Text>
<Button
title="Go to second modal"
onPress={() => navigation.navigate('MainStackScreen2')}
/>
<Button
title="goBack"
onPress={() => navigation.goBack()}
/>
</View>
);
}

function MainStackScreen2({ navigation }: NativeStackScreenProps<StackParamList, 'MainStackScreen2'>) {
return (
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
<Text>Main stack screen 2</Text>
<Button
title="goBack"
onPress={() => navigation.goBack()}
Expand All @@ -54,7 +72,7 @@ function Screen1({
/>
</View>
);
};
}

function Screen2({
navigation,
Expand All @@ -72,7 +90,7 @@ function Screen2({
/>
</View>
);
};
}

function Screen3({
navigation,
Expand All @@ -90,7 +108,7 @@ function Screen3({
/>
</View>
);
};
}

const Stack = createNativeStackNavigator<StackParamList>();
const NestedStack = createNativeStackNavigator<StackParamList>();
Expand All @@ -101,6 +119,7 @@ function RootStack() {
<Stack.Screen name="Home" component={HomeScreen} />
<Stack.Screen name="NestedStack" component={NestedStackScreen} options={{ headerShown: true, presentation: 'modal' }} />
<Stack.Screen name="MainStackScreen" component={MainStackScreen} options={{ headerShown: true, presentation: 'modal' }} />
<Stack.Screen name="MainStackScreen2" component={MainStackScreen2} options={{ headerShown: true, presentation: 'modal' }} />
</Stack.Navigator>
);
}
Expand Down
2 changes: 1 addition & 1 deletion ios/RNSScreenStack.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1281,7 +1281,7 @@ - (void)invalidate
// with modal presentation or foreign modal presented from inside a Screen.
- (void)dismissAllRelatedModals
{
[_controller dismissViewControllerAnimated:YES completion:nil];
[_controller.presentedViewController dismissViewControllerAnimated:YES completion:nil];

// This loop seems to be excessive. Above message send to `_controller` should
// be enough, because system dismisses the controllers recursively,
Expand Down

0 comments on commit 2152383

Please sign in to comment.