fix: incorrect dispatch of header props for screen beneath a modal (#…

…2229) ## Description When we push multiple screens with different configuration of `headerShown` prop (e.g. 4 screens with visible header and one screen with hidden header), open a modal and trigger rerenders on screens (e.g. by changing the theme) there's a high chance that value of header hide prop will be incorrect on the screen under the modal. Big kudos to @WoLewicki for helping with debugging! ## Changes `isPresentingVC` flag wasn't checking if current view controller was on the top, which was causing header updates for all screens in the stack (moreover it was done in non-deterministic order as context updates can cause non-deterministic rerenders of particular screens), so check `&& vc == nav.topViewController` was added to `isPresentingVC` flag in `ios/RNSScreenStackHeaderConfig.mm` file. After that change, we still update the screen which is directly under the modal, so fix merged in #1228 is still valid. ## Screenshots / GIFs ### Before https://github.com/software-mansion/react-native-screens/assets/44415389/cf9b07fc-4e95-4362-95c7-ab387a94148e ### After https://github.com/software-mansion/react-native-screens/assets/44415389/94a5dd72-498b-4959-a2de-1e6008d63895 ## Test code and steps to reproduce 1. `Test2229` was added, so just render it in `TestsExample` app 2. Push a few screens with header 3. Push screen without header 4. Open modal 5. Background and open the app or change theme 6. Close the modal 7. Before the fix header should be visible on the screen although it should be a screen without a header ## Checklist - [X] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation:  - [ ] 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
software-mansion · Jul 5, 2024 · ab065f2 · ab065f2
1 parent e5a6220
commit ab065f2
Show file tree

Hide file tree

Showing 3 changed files with 139 additions and 2 deletions.
diff --git a/apps/test-examples/App.js b/apps/test-examples/App.js
@@ -104,6 +104,7 @@ import Test2069 from './src/Test2069';
 import Test2118 from './src/Test2118';
 import Test2184 from './src/Test2184';
 import Test2223 from './src/Test2223';
+import Test2229 from './src/Test2229';
 import TestScreenAnimation from './src/TestScreenAnimation';
 import TestHeader from './src/TestHeader';
 

diff --git a/apps/test-examples/src/Test2229.tsx b/apps/test-examples/src/Test2229.tsx
@@ -0,0 +1,136 @@
+import React, { useCallback, useEffect, useState } from 'react';
+import {
+  ScrollView,
+  StyleSheet,
+  useColorScheme,
+  View,
+  AppState,
+  AppStateStatus,
+  Button,
+} from 'react-native';
+import {
+  DarkTheme,
+  DefaultTheme,
+  NavigationContainer,
+  ParamListBase,
+} from '@react-navigation/native';
+import {
+  NativeStackNavigationProp,
+  createNativeStackNavigator,
+} from '@react-navigation/native-stack';
+import { enableFreeze } from 'react-native-screens';
+
+const Stack = createNativeStackNavigator();
+
+let lastState: AppStateStatus;
+
+export default function App() {
+  const [counter, setCounter] = useState(0);
+  const handleChange = useCallback((nextState: AppStateStatus) => {
+    if (nextState === 'active' && lastState === 'background') {
+      setCounter(c => c + 1);
+    }
+    lastState = nextState;
+  }, []);
+
+  useEffect(() => {
+    enableFreeze(false);
+    const listener = AppState.addEventListener('change', handleChange);
+    handleChange(AppState.currentState);
+    return () => {
+      listener.remove();
+    };
+  }, [handleChange]);
+
+  const theme = useColorScheme();
+
+  return (
+    <NavigationContainer theme={theme === 'light' ? DefaultTheme : DarkTheme}>
+      <Stack.Navigator>
+        <Stack.Screen
+          name="Push"
+          component={PushScreen}
+          options={{
+            presentation: 'card',
+            title: `header ${counter}`,
+          }}
+        />
+        <Stack.Screen
+          name="PushWithoutHeader"
+          component={PushScreen}
+          options={{
+            presentation: 'card',
+            headerShown: false,
+            title: 'no header',
+          }}
+        />
+        <Stack.Screen
+          name="Modal"
+          component={ModalScreen}
+          options={{
+            presentation: 'modal',
+            orientation: 'portrait_up',
+            title: 'modal',
+            headerShown: false,
+          }}
+        />
+      </Stack.Navigator>
+    </NavigationContainer>
+  );
+}
+
+interface ModalScreenProps {
+  navigation: NativeStackNavigationProp<ParamListBase>;
+}
+
+function ModalScreen({ navigation }: ModalScreenProps) {
+  return (
+    <View style={styles.container}>
+      <Button
+        testID="stack-presentation-modal-screen-go-back-button"
+        title="Go back"
+        onPress={navigation.goBack}
+      />
+    </View>
+  );
+}
+
+interface PushScreenProps {
+  navigation: NativeStackNavigationProp<ParamListBase>;
+}
+
+function PushScreen({ navigation }: PushScreenProps) {
+  return (
+    <ScrollView contentContainerStyle={styles.container}>
+      <Button
+        testID="stack-presentation-screen-with-header"
+        title="Push"
+        onPress={() => navigation.push('Push')}
+      />
+      <Button
+        testID="stack-presentation-form-screen-push-modal"
+        title="Open modal"
+        onPress={() => navigation.push('Modal')}
+      />
+      <Button
+        testID="stack-presentation-screen-without-header"
+        title="Push without header"
+        onPress={() => navigation.push('PushWithoutHeader')}
+      />
+      {navigation.canGoBack() && (
+        <Button
+          testID="stack-presentation-screen-go-back-button"
+          title="Go back"
+          onPress={navigation.goBack}
+        />
+      )}
+    </ScrollView>
+  );
+}
+
+const styles = StyleSheet.create({
+  container: {
+    flex: 1,
+    paddingTop: 10,
+  },
+});
diff --git a/ios/RNSScreenStackHeaderConfig.mm b/ios/RNSScreenStackHeaderConfig.mm
@@ -141,8 +141,8 @@ - (void)updateViewControllerIfNeeded
     nextVC = nav.topViewController;
   }
 
-  // we want updates sent to the VC below modal too since it is also visible
-  BOOL isPresentingVC = nextVC != nil && vc.presentedViewController == nextVC;
+  // we want updates sent to the VC directly below modal too since it is also visible
+  BOOL isPresentingVC = nextVC != nil && vc.presentedViewController == nextVC && vc == nav.topViewController;
 
   BOOL isInFullScreenModal = nav == nil && _screenView.stackPresentation == RNSScreenStackPresentationFullScreenModal;
   // if nav is nil, it means we can be in a fullScreen modal, so there is no nextVC, but we still want to update