Skip to content

Commit 25e2f16

Browse files
jamesdev-kingkkafar
jamesdev-king
and
kkafar
committed
fix(iOS): header snapshots not working (#2393)
## Description The `updateViewControllerIfNeeded` call introduced by software-mansion/react-native-screens#2230 forces the view controller to rebuild the subviews with the recent config. When the screen is being unmounted it replaces the subviews with nil values as the `reactSubviews` are removed from the config one by one. Snapshots made earlier get discarded in the process. This PR adds a condition that prevents updating the view controller when the screen is being changed + stops unnecessary snapshots when the screen is not changed. ## Changes - updated `Test556.tsx` repro - added `isGoingToBeRemoved` property to `RNSScreenView` - making snapshots / updating the view controller conditionally <!-- ## Screenshots / GIFs --> ## Test code and steps to reproduce - Use `Test556.tsx` repro ## Checklist - [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> Co-authored-by: Kacper Kafara <kacperkafara@gmail.com>
1 parent b663c4a commit 25e2f16

File tree

5 files changed

+99
-34
lines changed

5 files changed

+99
-34
lines changed

‎apps/src/tests/Test556.tsx‎

Lines changed: 75 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
import * as React from 'react';
22
import { Button, StyleSheet, Text, View } from 'react-native';
33
import { NavigationContainer, ParamListBase } from '@react-navigation/native';
4-
import { NativeStackNavigationProp, createNativeStackNavigator } from '@react-navigation/native-stack';
4+
import {
5+
NativeStackNavigationProp,
6+
createNativeStackNavigator,
7+
} from '@react-navigation/native-stack';
8+
import { Square } from '../shared';
59

610
const Stack = createNativeStackNavigator();
711

812
type ScreenBaseProps = {
913
navigation: NativeStackNavigationProp<ParamListBase>;
10-
}
14+
};
1115

1216
export default function App() {
1317
return (
@@ -16,30 +20,48 @@ export default function App() {
1620
screenOptions={{
1721
animation: 'fade',
1822
}}>
19-
<Stack.Screen name="First" component={First} options={{
20-
headerTitle: () => (
21-
<View style={[styles.container, { backgroundColor: 'goldenrod' }]}>
22-
<Text>Hello there!</Text>
23-
</View>
24-
),
25-
headerRight: () => (
26-
<View style={[styles.container, { backgroundColor: 'lightblue' }]}>
27-
<Text>Right-1</Text>
28-
</View>
29-
),
30-
}} />
31-
<Stack.Screen name="Second" component={Second} options={{
32-
headerTitle: () => (
33-
<View style={[styles.container, { backgroundColor: 'mediumseagreen' }]}>
34-
<Text>General Kenobi</Text>
35-
</View>
36-
),
37-
headerRight: () => (
38-
<View style={[styles.container, { backgroundColor: 'mediumvioletred' }]}>
39-
<Text>Right-2</Text>
40-
</View>
41-
),
42-
}} />
23+
<Stack.Screen
24+
name="First"
25+
component={First}
26+
options={{
27+
headerTitle: () => (
28+
<View
29+
style={[styles.container, { backgroundColor: 'goldenrod' }]}>
30+
<Text>Hello there!</Text>
31+
</View>
32+
),
33+
headerRight: () => (
34+
<View
35+
style={[styles.container, { backgroundColor: 'lightblue' }]}>
36+
<Text>Right-1</Text>
37+
</View>
38+
),
39+
}}
40+
/>
41+
<Stack.Screen
42+
name="Second"
43+
component={Second}
44+
options={{
45+
headerTitle: () => (
46+
<View
47+
style={[
48+
styles.container,
49+
{ backgroundColor: 'mediumseagreen' },
50+
]}>
51+
<Text>General Kenobi</Text>
52+
</View>
53+
),
54+
headerRight: () => (
55+
<View
56+
style={[
57+
styles.container,
58+
{ backgroundColor: 'mediumvioletred' },
59+
]}>
60+
<Text>Right-2</Text>
61+
</View>
62+
),
63+
}}
64+
/>
4365
</Stack.Navigator>
4466
</NavigationContainer>
4567
);
@@ -55,11 +77,34 @@ function First({ navigation }: ScreenBaseProps) {
5577
}
5678

5779
function Second({ navigation }: ScreenBaseProps) {
80+
const [backButtonVisible, setBackButtonVisible] = React.useState(true);
81+
5882
return (
59-
<Button
60-
title="Tap me for first screen"
61-
onPress={() => navigation.popTo('First')}
62-
/>
83+
<>
84+
<Button
85+
title="Toggle left subview"
86+
onPress={() => {
87+
setBackButtonVisible(prev => !prev);
88+
navigation.setOptions({
89+
headerLeft: backButtonVisible
90+
? () => (
91+
<View
92+
style={[
93+
styles.container,
94+
{ backgroundColor: 'mediumblue' },
95+
]}>
96+
<Text>Left</Text>
97+
</View>
98+
)
99+
: undefined,
100+
});
101+
}}
102+
/>
103+
<Button
104+
title="Tap me for first screen"
105+
onPress={() => navigation.popTo('First')}
106+
/>
107+
</>
63108
);
64109
}
65110

‎ios/RNSScreen.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ namespace react = facebook::react;
100100
@property (nonatomic) react::LayoutMetrics newLayoutMetrics;
101101
@property (weak, nonatomic) RNSScreenStackHeaderConfig *config;
102102
@property (nonatomic, readonly) BOOL hasHeaderConfig;
103+
@property (nonatomic, readonly, getter=isMarkedForUnmountInCurrentTransaction)
104+
BOOL markedForUnmountInCurrentTransaction;
103105
#else
104106
@property (nonatomic, copy) RCTDirectEventBlock onAppear;
105107
@property (nonatomic, copy) RCTDirectEventBlock onDisappear;
@@ -124,6 +126,10 @@ namespace react = facebook::react;
124126
- (void)updateBounds;
125127
- (void)notifyDismissedWithCount:(int)dismissCount;
126128
- (instancetype)initWithFrame:(CGRect)frame;
129+
/// Tell `Screen` that it will be unmounted in next transaction.
130+
/// The component needs this so that we can later decide whether to
131+
/// replace it with snapshot or not.
132+
- (void)willBeUnmountedInUpcomingTransaction;
127133
#else
128134
- (instancetype)initWithBridge:(RCTBridge *)bridge;
129135
#endif // RCT_NEW_ARCH_ENABLED

‎ios/RNSScreen.mm‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ - (void)initCommonProps
123123
#endif // !TARGET_OS_TV
124124
_sheetsScrollView = nil;
125125
_didSetSheetAllowedDetentsOnController = NO;
126+
#ifdef RCT_NEW_ARCH_ENABLED
127+
_markedForUnmountInCurrentTransaction = NO;
128+
#endif // RCT_NEW_ARCH_ENABLED
126129
}
127130

128131
- (UIViewController *)reactViewController
@@ -1082,6 +1085,11 @@ - (BOOL)hasHeaderConfig
10821085
return _config != nil;
10831086
}
10841087

1088+
- (void)willBeUnmountedInUpcomingTransaction
1089+
{
1090+
_markedForUnmountInCurrentTransaction = YES;
1091+
}
1092+
10851093
+ (react::ComponentDescriptorProvider)componentDescriptorProvider
10861094
{
10871095
return react::concreteComponentDescriptorProvider<react::RNSScreenComponentDescriptor>();

‎ios/RNSScreenStack.mm‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,6 +1208,7 @@ - (void)mountingTransactionWillMount:(const facebook::react::MountingTransaction
12081208
if (mutation.type == react::ShadowViewMutation::Delete) {
12091209
RNSScreenView *_Nullable toBeRemovedChild = [self childScreenForTag:mutation.oldChildShadowView.tag];
12101210
if (toBeRemovedChild != nil) {
1211+
[toBeRemovedChild willBeUnmountedInUpcomingTransaction];
12111212
_toBeDeletedScreens.push_back(toBeRemovedChild);
12121213
}
12131214
}

‎ios/RNSScreenStackHeaderConfig.mm‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -823,12 +823,17 @@ - (void)mountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompone
823823

824824
- (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
825825
{
826-
// For explanation of why we can make a snapshot here despite the fact that our children are already
827-
// unmounted see https://github.com/software-mansion/react-native-screens/pull/2261
828-
[self replaceNavigationBarViewsWithSnapshotOfSubview:(RNSScreenStackHeaderSubview *)childComponentView];
826+
BOOL isGoingToBeRemoved = _screenView.isMarkedForUnmountInCurrentTransaction;
827+
if (isGoingToBeRemoved) {
828+
// For explanation of why we can make a snapshot here despite the fact that our children are already
829+
// unmounted see https://github.com/software-mansion/react-native-screens/pull/2261
830+
[self replaceNavigationBarViewsWithSnapshotOfSubview:(RNSScreenStackHeaderSubview *)childComponentView];
831+
}
829832
[_reactSubviews removeObject:(RNSScreenStackHeaderSubview *)childComponentView];
830833
[childComponentView removeFromSuperview];
831-
[self updateViewControllerIfNeeded];
834+
if (!isGoingToBeRemoved) {
835+
[self updateViewControllerIfNeeded];
836+
}
832837
}
833838

834839
- (void)replaceNavigationBarViewsWithSnapshotOfSubview:(RNSScreenStackHeaderSubview *)childComponentView

0 commit comments

Comments
 (0)