Skip to content

Commit

Permalink
fix: issues with presenting owned modals from foreign ones (#2113)
Browse files Browse the repository at this point in the history
## Description

Basically this is another edition of the issue #1829 (handled by #1912).
The issue comes down to the fact, that our `ScreenStack` is not aware of
all modal view controllers being in presentation,
but this time it is not aware of third-party modal view controllers
(I've named them "foreign" modals in opposite to "owned" modals).

This PR is not a comprehensive solution but rather just a patch aiming
at fixing one particular interaction reported in #2048.

I've left verbose code comments explaining the issue and suggesting
solution in the source code, including:

```
  // TODO: Find general way to manage owned and foreign modal view controllers and refactor this code. Consider building
  // model first (data structue, attempting to be aware of all modals in presentation and some text-like algorithm for
  // computing required operations).
```

Closes #2048
Closes #2085

## Changes

Trigger dissmisal of foreign modal if it is presented above `changeRoot`
modal (last modal that is to stay on stack after the updates).

## Test code and steps to reproduce

`Test2048` in `TestsExample` & `FabricTestExample`.

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
  • Loading branch information
kkafar authored Apr 27, 2024
1 parent f8a45e1 commit c21de50
Show file tree
Hide file tree
Showing 5 changed files with 198 additions and 4 deletions.
1 change: 1 addition & 0 deletions FabricTestExample/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ import TestScreenAnimation from './src/TestScreenAnimation';
import Test1981 from './src/Test1981';
import Test2008 from './src/Test2008';
import Test2028 from './src/Test2028';
import Test2048 from './src/Test2048';
import Test2069 from './src/Test2069';

enableFreeze(true);
Expand Down
85 changes: 85 additions & 0 deletions FabricTestExample/src/Test2048.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import React from 'react';
import { View, Modal, Button, TouchableWithoutFeedback } from 'react-native';
import { useState } from 'react';

import { NavigationContainer, useNavigation } from '@react-navigation/native';
import { createNativeStackNavigator } from '@react-navigation/native-stack';

type AppStackPages = {
Home: undefined;
Modal: undefined;
};

function HomeScreen() {
const navigation = useNavigation();
const [visible, setVisible] = useState(false);

return (
<View style={{ flex: 1, justifyContent: 'center', alignItems: 'center' }}>
<Button
title="Toggle bottom modal"
onPress={() => setVisible(prev => !prev)}
/>
<Modal animationType="slide" visible={visible} transparent>
<TouchableWithoutFeedback onPress={() => setVisible(false)}>
<View style={{ flex: 1 }} />
</TouchableWithoutFeedback>
<View
style={{
borderTopLeftRadius: 10,
borderTopRightRadius: 10,
borderWidth: 2,
borderColor: 'red',
padding: 10,
minHeight: '40%',
alignItems: 'center',
justifyContent: 'center',
}}>
<Button
title="Open navigation modal"
onPress={() => {
// Issue: autohiding the Modal that serves as a bottom sheet unmounts
// the anchor component for the screen that is in { presentation: "modal" } mode
// Previously the anchoring component for a { presentation: "modal" }-based screen was different and it worked
// The culprit is: https://github.com/software-mansion/react-native-screens/pull/1912 released in https://github.com/software-mansion/react-native-screens/releases/tag/3.29.0
// adding setTimeout does not bring any good, because
// - we either don't see navigation action
// - we unmount both the bottom sheet modal and the screen itself

setVisible(false);

navigation.navigate('Modal');
}}
/>
</View>
</Modal>
</View>
);
}

function ModalScreen() {
return <View style={{ flex: 1, backgroundColor: 'rgb(50,150,50)' }} />;
}

const AppStack = createNativeStackNavigator<AppStackPages>();

function Navigation() {
return (
<AppStack.Navigator>
<AppStack.Screen name="Home" component={HomeScreen} />
<AppStack.Screen
name="Modal"
component={ModalScreen}
options={{ presentation: 'modal' }}
/>
</AppStack.Navigator>
);
}

export default function App() {
return (
<NavigationContainer>
<Navigation />
</NavigationContainer>
);
}
1 change: 1 addition & 0 deletions TestsExample/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ import Test1844 from './src/Test1844';
import Test1864 from './src/Test1864';
import Test1981 from './src/Test1981';
import Test2008 from './src/Test2008';
import Test2048 from './src/Test2048';
import Test2069 from './src/Test2069';

enableFreeze(true);
Expand Down
85 changes: 85 additions & 0 deletions TestsExample/src/Test2048.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import React from 'react';
import { View, Modal, Button, TouchableWithoutFeedback } from 'react-native';
import { useState } from 'react';

import { NavigationContainer, useNavigation } from '@react-navigation/native';
import { createNativeStackNavigator } from '@react-navigation/native-stack';

type AppStackPages = {
Home: undefined;
Modal: undefined;
};

function HomeScreen() {
const navigation = useNavigation();
const [visible, setVisible] = useState(false);

return (
<View style={{ flex: 1, justifyContent: 'center', alignItems: 'center' }}>
<Button
title="Toggle bottom modal"
onPress={() => setVisible(prev => !prev)}
/>
<Modal animationType="slide" visible={visible} transparent>
<TouchableWithoutFeedback onPress={() => setVisible(false)}>
<View style={{ flex: 1 }} />
</TouchableWithoutFeedback>
<View
style={{
borderTopLeftRadius: 10,
borderTopRightRadius: 10,
borderWidth: 2,
borderColor: 'red',
padding: 10,
minHeight: '40%',
alignItems: 'center',
justifyContent: 'center',
}}>
<Button
title="Open navigation modal"
onPress={() => {
// Issue: autohiding the Modal that serves as a bottom sheet unmounts
// the anchor component for the screen that is in { presentation: "modal" } mode
// Previously the anchoring component for a { presentation: "modal" }-based screen was different and it worked
// The culprit is: https://github.com/software-mansion/react-native-screens/pull/1912 released in https://github.com/software-mansion/react-native-screens/releases/tag/3.29.0
// adding setTimeout does not bring any good, because
// - we either don't see navigation action
// - we unmount both the bottom sheet modal and the screen itself

setVisible(false);

navigation.navigate('Modal');
}}
/>
</View>
</Modal>
</View>
);
}

function ModalScreen() {
return <View style={{ flex: 1, backgroundColor: 'rgb(50,150,50)' }} />;
}

const AppStack = createNativeStackNavigator<AppStackPages>();

function Navigation() {
return (
<AppStack.Navigator>
<AppStack.Screen name="Home" component={HomeScreen} />
<AppStack.Screen
name="Modal"
component={ModalScreen}
options={{ presentation: 'modal' }}
/>
</AppStack.Navigator>
);
}

export default function App() {
return (
<NavigationContainer>
<Navigation />
</NavigationContainer>
);
}
30 changes: 26 additions & 4 deletions ios/RNSScreenStack.mm
Original file line number Diff line number Diff line change
Expand Up @@ -382,9 +382,12 @@ - (void)setModalViewControllers:(NSArray<UIViewController *> *)controllers
[newControllers removeObjectsInArray:_presentedModals];

// We need to find bottom-most view controller that should stay on the stack
// for the duration of transition. There are couple of scenarios:
// (1) No modals are presented or all modals were presented by this RNSNavigationController,
// (2) There are modals presented by other RNSNavigationControllers (nested/outer)
// for the duration of transition.

// There are couple of scenarios:
// (1) no modals are presented or all modals were presented by this RNSNavigationController,
// (2) there are modals presented by other RNSNavigationControllers (nested/outer),
// (3) there are modals presented by other controllers (e.g. React Native's Modal view).

// Last controller that is common for both _presentedModals & controllers
__block UIViewController *changeRootController = _controller;
Expand Down Expand Up @@ -479,16 +482,35 @@ - (void)setModalViewControllers:(NSArray<UIViewController *> *)controllers
}
};

// changeRootController is the last controller that *is owned by this stack*, and should stay unchanged after this
// batch of transitions. Therefore changeRootController.presentedViewController is the first constroller to be
// dismissed (implying also all controllers above). Notice here, that firstModalToBeDismissed could have been
// RNSScreen modal presented from *this* stack, another stack, or any other view controller with modal presentation
// provided by third-party libraries (e.g. React Native's Modal view). In case of presence of other (not managed by
// us) modal controllers, weird interactions might arise. The code below, besides handling our presentation /
// dismissal logic also attempts to handle possible wide gamut of cases of interactions with third-party modal
// controllers, however it's not perfect.
// TODO: Find general way to manage owned and foreign modal view controllers and refactor this code. Consider building
// model first (data structue, attempting to be aware of all modals in presentation and some text-like algorithm for
// computing required operations).

UIViewController *firstModalToBeDismissed = changeRootController.presentedViewController;

if (firstModalToBeDismissed != nil) {
BOOL shouldAnimate = changeRootIndex == controllers.count &&
[firstModalToBeDismissed isKindOfClass:[RNSScreen class]] &&
((RNSScreen *)firstModalToBeDismissed).screenView.stackAnimation != RNSScreenStackAnimationNone;

if ([_presentedModals containsObject:firstModalToBeDismissed]) {
if ([_presentedModals containsObject:firstModalToBeDismissed] ||
![firstModalToBeDismissed isKindOfClass:RNSScreen.class]) {
// We dismiss every VC that was presented by changeRootController VC or its descendant.
// After the series of dismissals is completed we run completion block in which
// we present modals on top of changeRootController (which may be the this stack VC)
//
// There also might the second case, where the firstModalToBeDismissed is foreign.
// See: https://github.com/software-mansion/react-native-screens/issues/2048
// For now, to mitigate the issue, we also decide to trigger its dismissal before
// starting the presentation chain down below in finish() callback.
[changeRootController dismissViewControllerAnimated:shouldAnimate completion:finish];
return;
}
Expand Down

0 comments on commit c21de50

Please sign in to comment.