From c5cd87a1c9d228029e64e0bfb7cf3ea5b4ed2b88 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alex=20Du=C5=BCy?=
 <91994767+alduzy@users.noreply.github.com>
Date: Tue, 9 Jul 2024 09:55:52 +0200
Subject: [PATCH] fix(iOS): white flash on tab change when using native stack
 (#2188)

This PR gets rid of undesired white flashes during
`maybeAddToParentAndUpdateContainer`. The white flash was present on
paper architecture when `unmountOnBlur` option was set to true on parent
bottomStackNavigator (see repro).

The affected logic was previously introduced or changed by following
PRs:
- https://github.com/software-mansion/react-native-screens/pull/600
- https://github.com/software-mansion/react-native-screens/pull/613
- https://github.com/software-mansion/react-native-screens/pull/643

The removed `_hasLayout` was initially added by
https://github.com/software-mansion/react-native-screens/pull/600 in
order to resolve an issue:
https://github.com/software-mansion/react-native-screens/issues/432.
However the logic was later changed by
https://github.com/software-mansion/react-native-screens/pull/613 in
order to fix another issue and the added `_hasLayout` may not fix
anything eventually, as stated by [this
comment](https://github.com/software-mansion/react-native-screens/issues/432#issuecomment-687868469).

Fixes #1645.

- removed `_hasLayout` variable
- added repros

https://github.com/software-mansion/react-native-screens/assets/91994767/226a32d7-728b-48dd-b21a-6a1e4195add2

https://github.com/software-mansion/react-native-screens/assets/91994767/32febcf1-d159-4a9d-ae3a-373042732a6d

- added `Test1645.js` repro to test examples
- added `Test432.tsx` repro to test examples

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

---------

Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>
---
 apps/src/tests/Test1645.js | 98 ++++++++++++++++++++++++++++++++++++++
 apps/src/tests/Test432.tsx | 72 ++++++++++++++++++++++++++++
 apps/src/tests/index.ts    |  2 +
 ios/RNSScreenStack.mm      | 18 ++-----
 4 files changed, 175 insertions(+), 15 deletions(-)
 create mode 100644 apps/src/tests/Test1645.js
 create mode 100644 apps/src/tests/Test432.tsx

diff --git a/apps/src/tests/Test1645.js b/apps/src/tests/Test1645.js
new file mode 100644
index 0000000000..ec4a508944
--- /dev/null
+++ b/apps/src/tests/Test1645.js
@@ -0,0 +1,98 @@
+/* eslint-disable react-native/no-inline-styles */
+
+import React, {useEffect, useState} from 'react';
+import {Text, View} from 'react-native';
+
+import {createBottomTabNavigator} from '@react-navigation/bottom-tabs';
+import {createNativeStackNavigator} from '@react-navigation/native-stack';
+import {createStackNavigator} from '@react-navigation/stack';
+import {NavigationContainer} from '@react-navigation/native';
+
+const TestBottomTabBar = createBottomTabNavigator();
+const TestNativeStack1 = createNativeStackNavigator();
+const TestNativeStack2 = createNativeStackNavigator();
+
+const TestScreen1 = () => {
+  const [t, setT] = useState(110);
+
+  useEffect(() => {
+    const interval = setInterval(() => {
+      setT(lastT => lastT + 1);
+    }, 100);
+
+    return () => clearInterval(interval);
+  }, []);
+  return (
+    <View
+      style={{
+        flex: 1,
+        backgroundColor: '#000',
+      }}>
+      {Array.from({length: 100}).map((e, idx) => (
+        <Text style={{color: '#fff'}} key={idx}>
+          T{idx}: {t}
+        </Text>
+      ))}
+    </View>
+  );
+};
+
+const TestScreen2 = () => {
+  const [t, setT] = useState(0);
+
+  useEffect(() => {
+    const interval = setInterval(() => {
+      setT(lastT => lastT + 1);
+    }, 100);
+
+    return () => clearInterval(interval);
+  }, []);
+
+  return (
+    <View
+      style={{
+        flex: 1,
+        backgroundColor: '#000',
+      }}>
+      {Array.from({length: 100}).map((e, idx) => (
+        <Text style={{color: 'red'}} key={idx}>
+          T{idx}: {t}
+        </Text>
+      ))}
+    </View>
+  );
+};
+const TestScreenTab1 = () => {
+  return (
+    <TestNativeStack1.Navigator initialRouteName="screen1a">
+      <TestNativeStack1.Screen name="screen1a" component={TestScreen1} />
+      <TestNativeStack1.Screen name="screen1b" component={TestScreen2} />
+    </TestNativeStack1.Navigator>
+  );
+};
+
+const TestScreenTab2 = () => {
+  return (
+    <TestNativeStack2.Navigator initialRouteName="screen2a">
+      <TestNativeStack2.Screen name="screen2a" component={TestScreen2} />
+      <TestNativeStack2.Screen name="screen2b" component={TestScreen1} />
+    </TestNativeStack2.Navigator>
+  );
+};
+
+const App = () => {
+  return (
+    <NavigationContainer>
+      <TestBottomTabBar.Navigator
+        initialRouteName="tab1"
+        screenOptions={{
+          unmountOnBlur: true,
+        }}>
+        <TestBottomTabBar.Screen name="tab1" component={TestScreenTab1} />
+        <TestBottomTabBar.Screen name="tab2" component={TestScreenTab2} />
+      </TestBottomTabBar.Navigator>
+    </NavigationContainer>
+  );
+};
+
+export default App;
diff --git a/apps/src/tests/Test432.tsx b/apps/src/tests/Test432.tsx
new file mode 100644
index 0000000000..5a0843dbc0
--- /dev/null
+++ b/apps/src/tests/Test432.tsx
@@ -0,0 +1,72 @@
+import { Pressable, View, Button, Text } from 'react-native';
+
+import { NavigationContainer, useNavigation } from '@react-navigation/native';
+import {
+  NativeStackScreenProps,
+  createNativeStackNavigator,
+} from '@react-navigation/native-stack';
+import React, { useCallback } from 'react';
+
+type RootStackParamList = {
+  Home: undefined;
+  Settings: undefined;
+};
+type RootStackScreenProps<T extends keyof RootStackParamList> =
+  NativeStackScreenProps<RootStackParamList, T>;
+const HomeScreen = ({ navigation }: RootStackScreenProps<'Home'>) => {
+  const showSettings = useCallback(() => {
+    navigation.navigate('Settings');
+  }, [navigation]);
+  return (
+    <View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
+      <Button onPress={showSettings} title={'Show settings'} />
+    </View>
+  );
+};
+
+const SettingsScreen = ({ navigation }: RootStackScreenProps<'Settings'>) => {
+  return (
+    <View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
+      <Text>Settings</Text>
+    </View>
+  );
+};
+
+const RootStack = createNativeStackNavigator<RootStackParamList>();
+const RootNavigator = () => {
+  const navigation = useNavigation();
+  const headerRight = useCallback(
+    () => (
+      <Pressable
+        onPress={() => {
+          navigation.goBack();
+        }}>
+        <Text>Close</Text>
+      </Pressable>
+    ),
+    [navigation],
+  );
+  return (
+    <RootStack.Navigator screenOptions={{ headerShown: false }}>
+      <RootStack.Screen name="Home" component={HomeScreen} />
+      <RootStack.Screen
+        name="Settings"
+        component={SettingsScreen}
+        options={{
+          presentation: 'modal',
+          animation: 'slide_from_bottom',
+          headerShown: true,
+          headerRight: headerRight,
+        }}
+      />
+    </RootStack.Navigator>
+  );
+};
+
+export default function App() {
+  return (
+    <NavigationContainer>
+      <RootNavigator />
+    </NavigationContainer>
+  );
+}
diff --git a/apps/src/tests/index.ts b/apps/src/tests/index.ts
index eac3106dc2..ff4c9e2baa 100644
--- a/apps/src/tests/index.ts
+++ b/apps/src/tests/index.ts
@@ -5,6 +5,7 @@ export { default as Test111 } from './Test111';
 export { default as Test263 } from './Test263';
 export { default as Test349 } from './Test349';
 export { default as Test364 } from './Test364';
+export { default as Test432 } from './Test432';
 export { default as Test528 } from './Test528';
 export { default as Test550 } from './Test550';
 export { default as Test556 } from './Test556';
@@ -81,6 +82,7 @@ export { default as Test1473 } from './Test1473';
 export { default as Test1476 } from './Test1476';
 export { default as Test1509 } from './Test1509';
 export { default as Test1539 } from './Test1539';
+export { default as Test1645 } from './Test1645';
 export { default as Test1646 } from './Test1646';
 export { default as Test1649 } from './Test1649';
 export { default as Test1671 } from './Test1671';
diff --git a/ios/RNSScreenStack.mm b/ios/RNSScreenStack.mm
index 7c5b6968ba..934d2f77fc 100644
--- a/ios/RNSScreenStack.mm
+++ b/ios/RNSScreenStack.mm
@@ -118,7 +118,6 @@ @implementation RNSScreenStackView {
   BOOL _invalidated;
   BOOL _isFullWidthSwiping;
   UIPercentDrivenInteractiveTransition *_interactionController;
-  BOOL _hasLayout;
   __weak RNSScreenStackManager *_manager;
   BOOL _updateScheduled;
 #ifdef RCT_NEW_ARCH_ENABLED
@@ -142,7 +141,6 @@ - (instancetype)initWithFrame:(CGRect)frame
 - (instancetype)initWithManager:(RNSScreenStackManager *)manager
 {
   if (self = [super init]) {
-    _hasLayout = NO;
     _invalidated = NO;
     _manager = manager;
     [self initCommonProps];
@@ -272,18 +270,9 @@ - (void)didMoveToWindow
 - (void)maybeAddToParentAndUpdateContainer
 {
   BOOL wasScreenMounted = _controller.parentViewController != nil;
-#ifdef RCT_NEW_ARCH_ENABLED
-  BOOL isScreenReadyForShowing = self.window;
-#else
-  BOOL isScreenReadyForShowing = self.window && _hasLayout;
-#endif
-  if (!isScreenReadyForShowing && !wasScreenMounted) {
-    // We wait with adding to parent controller until the stack is mounted and has its initial
-    // layout done.
-    // If we add it before layout, some of the items (specifically items from the navigation bar),
-    // won't be able to position properly. Also the position and size of such items, even if it
-    // happens to change, won't be properly updated (this is perhaps some internal issue of UIKit).
-    // If we add it when window is not attached, some of the view transitions will be bloced (i.e.
+  if (!self.window && !wasScreenMounted) {
+    // We wait with adding to parent controller until the stack is mounted.
+    // If we add it when window is not attached, some of the view transitions will be blocked (i.e.
     // modal transitions) and the internal view controler's state will get out of sync with what's
     // on screen without us knowing.
     return;
@@ -1076,7 +1065,6 @@ - (void)didUpdateReactSubviews
   // set yet, however the layout call is already enqueued on ui thread. Enqueuing update call on the
   // ui queue will guarantee that the update will run after layout.
   dispatch_async(dispatch_get_main_queue(), ^{
-    self->_hasLayout = YES;
     [self maybeAddToParentAndUpdateContainer];
   });
 }