Skip to content

Commit

Permalink
fix(iOS): incorrect safe area on transparent modals using landscape o…
Browse files Browse the repository at this point in the history
…rientation (software-mansion#2008)

## Description
Normally, while opening a modal, each screen should preserve its safe
area, depending on the screen orientation. Unfortunately, when user
dismisses transparent modals, safe area of the origin screen breaks,
resulting in the vertical safe area on landscape orientation.
Because of that, I investigated the logic behind the final decision of
screen orientations and it looks like the problem was lying on
`supportedScreenOrientations` method in RNSScreen.mm file.

1. First, the modal was being asked for its supported device
orientation. Once it reached the `supportedScreenOrientations` method,
it was asking for config of childVC. Because the childVC was `nil` and
the orientation wasn't set in screen options, it was returning `nil`,
resulting in returning all orientations without `upside down`.
2. After that, there was a time for looking onto child VCs of a screen
behind the modal. Since it was presenting a modal, it was first checking
for its last child - since it didn't provide any modal and it haven't
got any children, the last child was also `nil`, resulting in returning
the modal as an orientation of the screen.
3. Returning a modal is (probably) a bad idea here, since it does not
have any screen orientation specified, resulting in returning `nil` as a
screen orientation. This was probably resulting a bug with wrong safe
area.

This PR changes this bad behavior by not returning `lastViewController`
(which is a modal that is being presented from a screen) and going
further for looking a config in child view controllers.
However, this behavior may lead to the further bugs we haven't
discovered yet.

## Changes

- Changed `return` statement in `findChildVCForConfigAndTrait` method.

## Screenshots / GIFs

### Before


https://github.com/software-mansion/react-native-screens/assets/23281839/6ff888d9-a7df-466c-ac45-d3db1cbe928c

### After


https://github.com/software-mansion/react-native-screens/assets/23281839/dfc03a6c-ca7a-49a0-9c3e-7f019f7d405c

## Test code and steps to reproduce

You can check Test2008 in `TestsExample` and `FabricTestExample` in
order to tests how transparent modals behave.

## Checklist

- [X] Included code example that can be used to test this change
- [X] Ensured that CI passes

---------

Co-authored-by: Kacper Kafara <[email protected]>
  • Loading branch information
2 people authored and ja1ns committed Oct 9, 2024
1 parent 32a8945 commit 8060a48
Show file tree
Hide file tree
Showing 5 changed files with 387 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 @@ -92,6 +92,7 @@ import Test1844 from './src/Test1844';
import Test1864 from './src/Test1864';
import TestScreenAnimation from './src/TestScreenAnimation';
import Test1981 from './src/Test1981';
import Test2008 from './src/Test2008';

enableFreeze(true);

Expand Down
184 changes: 184 additions & 0 deletions FabricTestExample/src/Test2008.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
import React from 'react';
import { Text, View, SafeAreaView, StyleSheet, Pressable } from 'react-native';
import {
createNativeStackNavigator,
NativeStackNavigationProp,
} from '@react-navigation/native-stack';
import {
NavigationContainer,
useNavigation,
ParamListBase,
type NavigationProp,
} from '@react-navigation/native';
import { SafeAreaProvider } from 'react-native-safe-area-context';

const styles = StyleSheet.create({
inner: {
flex: 1,
backgroundColor: 'white',
},
safeArea: {
flex: 1,
backgroundColor: 'red',
},
innerModal: {
flex: 1,
backgroundColor: 'rgba(0,0,0,0.4)',
justifyContent: 'center',
alignItems: 'center',
},
pressable: {
padding: 20,
backgroundColor: '#ccc',
marginVertical: 5,
},

textIntro: {
padding: 10,
},

buttons: {
flexDirection: 'row',
padding: 10,
},

text: {
textAlign: 'center',
},
});

type RootStackScreens = {
Home: undefined;
Modal: undefined;
TransparentModal: undefined;
ContainedTransparentModal: undefined;
};

const RootStack = createNativeStackNavigator<RootStackScreens>();

function Home() {
const navigation = useNavigation<NavigationProp<RootStackScreens>>();
return (
<SafeAreaView style={styles.safeArea}>
<View style={styles.inner}>
<Text style={styles.textIntro}>
Red represents the safe area padding as provided by React Native Safe
Area Context (although I've noticed that the issue also affects the
build in react native SafeArea component).
</Text>
<Text style={styles.textIntro}>
This only applies to iOS. Ensure you have rotation lock off, and
rotate the view into landscape orientation. Note how the red safe
areas appear. Then tap `Spawn Transparent Modal`, dismiss the modal,
and then rotate the screen again to see how the safe areas are now
stuck as the portrait values. You must force quite the app to undo the
bug.
</Text>
<Pressable
style={styles.pressable}
onPress={() => navigation.navigate('Modal')}>
<Text style={styles.text}>"modal"</Text>
</Pressable>
<Pressable
style={styles.pressable}
onPress={() => navigation.navigate('ContainedTransparentModal')}>
<Text style={styles.text}>"containedTransparentModal"</Text>
</Pressable>
<Pressable
style={styles.pressable}
onPress={() => navigation.navigate('TransparentModal')}>
<Text style={styles.text}>"transparentModal"</Text>
</Pressable>
</View>
</SafeAreaView>
);
}

function Modal({
navigation,
}: {
navigation: NativeStackNavigationProp<ParamListBase>;
}) {
return (
<View style={styles.innerModal}>
<Text>Modal</Text>
<Pressable style={styles.pressable} onPress={() => navigation.goBack()}>
<Text>Go Back!</Text>
</Pressable>
<Pressable
style={styles.pressable}
onPress={() => navigation.push('Modal')}>
<Text>Open another modal!</Text>
</Pressable>
</View>
);
}

function TransparentModal({
navigation,
}: {
navigation: NativeStackNavigationProp<ParamListBase>;
}) {
return (
<View style={styles.innerModal}>
<Text>Transparent Modal</Text>
<Pressable style={styles.pressable} onPress={() => navigation.goBack()}>
<Text>Go Back!</Text>
</Pressable>
<Pressable
style={styles.pressable}
onPress={() => navigation.push('TransparentModal')}>
<Text>Open another modal!</Text>
</Pressable>
</View>
);
}

function ContainedTransparentModal({
navigation,
}: {
navigation: NativeStackNavigationProp<ParamListBase>;
}) {
return (
<View style={styles.innerModal}>
<Text>Contained Transparent Modal</Text>
<Pressable style={styles.pressable} onPress={() => navigation.goBack()}>
<Text>Go Back!</Text>
</Pressable>
<Pressable
style={styles.pressable}
onPress={() => navigation.push('ContainedTransparentModal')}>
<Text>Open another modal!</Text>
</Pressable>
</View>
);
}

export default function App() {
return (
<SafeAreaProvider>
<NavigationContainer>
<RootStack.Navigator
initialRouteName="Home"
screenOptions={{ headerShown: false }}>
<RootStack.Screen name="Home" component={Home} />
<RootStack.Screen
name="TransparentModal"
component={TransparentModal}
options={{ presentation: 'transparentModal' }}
/>
<RootStack.Screen
name="ContainedTransparentModal"
component={ContainedTransparentModal}
options={{ presentation: 'containedTransparentModal' }}
/>
<RootStack.Screen
name="Modal"
component={Modal}
options={{ presentation: 'modal' }}
/>
</RootStack.Navigator>
</NavigationContainer>
</SafeAreaProvider>
);
}
1 change: 1 addition & 0 deletions TestsExample/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ import Test1829 from './src/Test1829';
import Test1844 from './src/Test1844';
import Test1864 from './src/Test1864';
import Test1981 from './src/Test1981';
import Test2008 from './src/Test2008';

enableFreeze(true);

Expand Down
184 changes: 184 additions & 0 deletions TestsExample/src/Test2008.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
import React from 'react';
import { Text, View, SafeAreaView, StyleSheet, Pressable } from 'react-native';
import {
createNativeStackNavigator,
NativeStackNavigationProp,
} from '@react-navigation/native-stack';
import {
NavigationContainer,
useNavigation,
ParamListBase,
type NavigationProp,
} from '@react-navigation/native';
import { SafeAreaProvider } from 'react-native-safe-area-context';

const styles = StyleSheet.create({
inner: {
flex: 1,
backgroundColor: 'white',
},
safeArea: {
flex: 1,
backgroundColor: 'red',
},
innerModal: {
flex: 1,
backgroundColor: 'rgba(0,0,0,0.4)',
justifyContent: 'center',
alignItems: 'center',
},
pressable: {
padding: 20,
backgroundColor: '#ccc',
marginVertical: 5,
},

textIntro: {
padding: 10,
},

buttons: {
flexDirection: 'row',
padding: 10,
},

text: {
textAlign: 'center',
},
});

type RootStackScreens = {
Home: undefined;
Modal: undefined;
TransparentModal: undefined;
ContainedTransparentModal: undefined;
};

const RootStack = createNativeStackNavigator<RootStackScreens>();

function Home() {
const navigation = useNavigation<NavigationProp<RootStackScreens>>();
return (
<SafeAreaView style={styles.safeArea}>
<View style={styles.inner}>
<Text style={styles.textIntro}>
Red represents the safe area padding as provided by React Native Safe
Area Context (although I've noticed that the issue also affects the
build in react native SafeArea component).
</Text>
<Text style={styles.textIntro}>
This only applies to iOS. Ensure you have rotation lock off, and
rotate the view into landscape orientation. Note how the red safe
areas appear. Then tap `Spawn Transparent Modal`, dismiss the modal,
and then rotate the screen again to see how the safe areas are now
stuck as the portrait values. You must force quite the app to undo the
bug.
</Text>
<Pressable
style={styles.pressable}
onPress={() => navigation.navigate('Modal')}>
<Text style={styles.text}>"modal"</Text>
</Pressable>
<Pressable
style={styles.pressable}
onPress={() => navigation.navigate('ContainedTransparentModal')}>
<Text style={styles.text}>"containedTransparentModal"</Text>
</Pressable>
<Pressable
style={styles.pressable}
onPress={() => navigation.navigate('TransparentModal')}>
<Text style={styles.text}>"transparentModal"</Text>
</Pressable>
</View>
</SafeAreaView>
);
}

function Modal({
navigation,
}: {
navigation: NativeStackNavigationProp<ParamListBase>;
}) {
return (
<View style={styles.innerModal}>
<Text>Modal</Text>
<Pressable style={styles.pressable} onPress={() => navigation.goBack()}>
<Text>Go Back!</Text>
</Pressable>
<Pressable
style={styles.pressable}
onPress={() => navigation.push('Modal')}>
<Text>Open another modal!</Text>
</Pressable>
</View>
);
}

function TransparentModal({
navigation,
}: {
navigation: NativeStackNavigationProp<ParamListBase>;
}) {
return (
<View style={styles.innerModal}>
<Text>Transparent Modal</Text>
<Pressable style={styles.pressable} onPress={() => navigation.goBack()}>
<Text>Go Back!</Text>
</Pressable>
<Pressable
style={styles.pressable}
onPress={() => navigation.push('TransparentModal')}>
<Text>Open another modal!</Text>
</Pressable>
</View>
);
}

function ContainedTransparentModal({
navigation,
}: {
navigation: NativeStackNavigationProp<ParamListBase>;
}) {
return (
<View style={styles.innerModal}>
<Text>Contained Transparent Modal</Text>
<Pressable style={styles.pressable} onPress={() => navigation.goBack()}>
<Text>Go Back!</Text>
</Pressable>
<Pressable
style={styles.pressable}
onPress={() => navigation.push('ContainedTransparentModal')}>
<Text>Open another modal!</Text>
</Pressable>
</View>
);
}

export default function App() {
return (
<SafeAreaProvider>
<NavigationContainer>
<RootStack.Navigator
initialRouteName="Home"
screenOptions={{ headerShown: false }}>
<RootStack.Screen name="Home" component={Home} />
<RootStack.Screen
name="TransparentModal"
component={TransparentModal}
options={{ presentation: 'transparentModal' }}
/>
<RootStack.Screen
name="ContainedTransparentModal"
component={ContainedTransparentModal}
options={{ presentation: 'containedTransparentModal' }}
/>
<RootStack.Screen
name="Modal"
component={Modal}
options={{ presentation: 'modal' }}
/>
</RootStack.Navigator>
</NavigationContainer>
</SafeAreaProvider>
);
}
Loading

0 comments on commit 8060a48

Please sign in to comment.