Skip to content

Commit ad24734

Browse files
authored
fix(Fabric,iOS): header subviews do not support dynamic content changes (#2905)
## Description Regression most likely introduced in 4.5.0 by #2466. Fixes #2714 Fixes #2815 Supersedes #2845 This is a ugly hack to workaround issue with dynamic content change. When the size of this shadow node contents (children) change due to JS update, e.g. new icon is added, if the size is set for the yogaNode corresponding to this shadow node, the enforced size will be used and the size won't be updated by Yoga to reflect the contents size change -> host tree won't get layout metrics update -> we won't trigger native layout -> the views in header will be positioned incorrectly. > [!important] > This PR handles iOS only, however there is **similar** issue on Android. The issue can be reproduced on the same test example. Android will be handled in separate PR. ## Changes ## Test code and steps to reproduce In this approach I've settled with: 1. not calling set size on iOS for `RNSScreenStackHeaderSubviewShadowNode`, 2. updating header config padding & sending it as state to shadow tree. Added `Test2714` Most of the fragile header interactions must be tested: * [x] Header title truncation - `TestHeaderTitle` ~❌ This PR introduces regression here on iOS (Android not tested yet)~ ✅ Works * [x] Pressables in header - `Test2466` (iOS works, Android code is unmodified here) * [x] #2807 (this PR does not touch Android) * [x] #2811 (this PR does not touch Android) * [x] #2812 (this PR does not touch Android) * [x] Header behaviour on orientation changes - #2756 (this PR does not touch Android) * [x] New test `Test2714` handling header item resizing. ## Checklist - [x] Included code example that can be used to test this change - [ ] Ensured that CI passes
1 parent 3f1672e commit ad24734

9 files changed

Lines changed: 300 additions & 32 deletions

apps/src/tests/Test2714.tsx

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
import React from 'react';
2+
import { NavigationContainer, useNavigation } from '@react-navigation/native';
3+
import { Text, View, StyleSheet, Button } from 'react-native';
4+
import { createNativeStackNavigator } from '@react-navigation/native-stack';
5+
import PressableWithFeedback from '../shared/PressableWithFeedback';
6+
7+
const Stack = createNativeStackNavigator();
8+
9+
function RootStack() {
10+
return (
11+
<Stack.Navigator
12+
screenOptions={{
13+
headerShown: true,
14+
headerBackButtonDisplayMode: 'minimal',
15+
}}>
16+
<Stack.Screen name="Home" component={HomeScreen} />
17+
<Stack.Screen name="Profile" component={HomeScreen} />
18+
</Stack.Navigator>
19+
);
20+
}
21+
22+
const HomeScreen = ({ navigation }: any) => {
23+
const [secondButtonShown, setSecondButtonShown] = React.useState(true);
24+
const [thirdButtonShown, setThirdButtonShown] = React.useState(true);
25+
const [showAllButtons, setShowAllButtons] = React.useState(true);
26+
const [isLargeSize, setLargeSize] = React.useState(true);
27+
28+
const headerRight = React.useCallback(() => {
29+
return (
30+
<View style={[styles.buttonsView, !showAllButtons && { display: 'none' }]}>
31+
<PressableWithFeedback style={isLargeSize ? styles.largeButton : styles.button} onPress={() => console.log(1)}>
32+
<Text>1</Text>
33+
</PressableWithFeedback>
34+
{secondButtonShown && (
35+
<PressableWithFeedback style={styles.button} onPress={() => console.log(2)}>
36+
<Text>2</Text>
37+
</PressableWithFeedback>
38+
)}
39+
{thirdButtonShown && (
40+
<PressableWithFeedback style={styles.button} onPress={() => console.log(3)}>
41+
<Text>3</Text>
42+
</PressableWithFeedback>
43+
)}
44+
<PressableWithFeedback style={styles.button} onPress={() => console.log('D')}>
45+
<Text>[D]</Text>
46+
</PressableWithFeedback>
47+
</View>
48+
);
49+
}, [secondButtonShown, thirdButtonShown, showAllButtons, isLargeSize]);
50+
51+
React.useLayoutEffect(() => {
52+
navigation.setOptions({
53+
headerStyle: { backgroundColor: 'pink' },
54+
headerRight: headerRight,
55+
});
56+
}, [navigation, headerRight, showAllButtons]);
57+
58+
return (
59+
<View>
60+
<Text>Home Screen</Text>
61+
<Button
62+
title="Toggle size"
63+
onPress={() => setLargeSize(p => !p)}
64+
/>
65+
<Button
66+
title="Toggle 2nd button"
67+
onPress={() => setSecondButtonShown(p => !p)}
68+
/>
69+
<Button
70+
title="Toggle 3rd button"
71+
onPress={() => setThirdButtonShown(p => !p)}
72+
/>
73+
<Button
74+
title="Toggle All Right Buttons"
75+
onPress={() => setShowAllButtons(p => !p)}
76+
/>
77+
</View>
78+
);
79+
};
80+
81+
function SimpleHome() {
82+
const navigation = useNavigation();
83+
const [isExpanded, setExpanded] = React.useState(false);
84+
85+
const headerRight = React.useCallback(() => {
86+
return (
87+
<PressableWithFeedback>
88+
<View style={[{ width: 128, height: 42 }, isExpanded ? { width: 192 } : null]} />
89+
</PressableWithFeedback>
90+
);
91+
}, [isExpanded]);
92+
93+
React.useLayoutEffect(() => {
94+
navigation.setOptions({
95+
headerRight,
96+
});
97+
}, [headerRight, navigation]);
98+
99+
return (
100+
<View style={{ flex: 1, backgroundColor: 'lightsalmon' }}>
101+
<Button title="Toggle subview size" onPress={() => setExpanded(val => !val)} />
102+
<Button title="Go to HomeScreen" onPress={() => navigation.navigate('HomeScreen')} />
103+
</View>
104+
);
105+
}
106+
107+
function SimpleStack() {
108+
return (
109+
<Stack.Navigator>
110+
<Stack.Screen name="SimpleHome" component={SimpleHome} />
111+
<Stack.Screen name="HomeScreen" component={HomeScreen} />
112+
</Stack.Navigator>
113+
);
114+
}
115+
116+
// function App() {
117+
// return (
118+
// <NavigationContainer>
119+
// <RootStack />
120+
// </NavigationContainer>
121+
// );
122+
// }
123+
124+
function AppSimplified() {
125+
return (
126+
<NavigationContainer>
127+
<SimpleStack />
128+
</NavigationContainer>
129+
);
130+
}
131+
132+
// export default App;
133+
export default AppSimplified;
134+
135+
136+
const styles = StyleSheet.create({
137+
button: {
138+
width: 42,
139+
height: 42,
140+
marginHorizontal: 5,
141+
padding: 5,
142+
borderRadius: 20,
143+
justifyContent: 'center',
144+
alignItems: 'center',
145+
backgroundColor: 'blue',
146+
},
147+
largeButton: {
148+
width: 64,
149+
height: 42,
150+
marginHorizontal: 5,
151+
padding: 5,
152+
borderRadius: 20,
153+
justifyContent: 'center',
154+
alignItems: 'center',
155+
backgroundColor: 'blue',
156+
},
157+
buttonsView: {
158+
flexDirection: 'row',
159+
borderWidth: 1,
160+
},
161+
});

apps/src/tests/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ export { default as Test2611 } from './Test2611';
124124
export { default as Test2631 } from './Test2631';
125125
export { default as Test2668 } from './Test2668';
126126
export { default as Test2675 } from './Test2675';
127+
export { default as Test2714 } from './Test2714';
127128
export { default as Test2717 } from './Test2717';
128129
export { default as Test2767 } from './Test2767';
129130
export { default as Test2789 } from './Test2789';

common/cpp/react/renderer/components/rnscreens/RNSScreenStackHeaderConfigComponentDescriptor.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class RNSScreenStackHeaderConfigComponentDescriptor final
4747
#else
4848
if (stateData.frameSize.width != 0 && stateData.frameSize.height != 0) {
4949
layoutableShadowNode.setSize(stateData.frameSize);
50+
layoutableShadowNode.setPadding(stateData.edgeInsets);
5051
}
5152
#endif // ANDROID
5253

common/cpp/react/renderer/components/rnscreens/RNSScreenStackHeaderConfigState.h

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,31 @@ class JSI_EXPORT RNSScreenStackHeaderConfigState final {
1717

1818
RNSScreenStackHeaderConfigState() = default;
1919

20-
// Used in iOS code
21-
RNSScreenStackHeaderConfigState(Size frameSize_) : frameSize{frameSize_} {}
20+
#if !defined(ANDROID)
21+
RNSScreenStackHeaderConfigState(Size frameSize_, EdgeInsets edgeInsets_)
22+
: frameSize{frameSize_}, edgeInsets{edgeInsets_} {}
23+
24+
// Make it copyable
25+
RNSScreenStackHeaderConfigState(const RNSScreenStackHeaderConfigState &source)
26+
: frameSize{source.frameSize}, edgeInsets{source.edgeInsets} {}
27+
RNSScreenStackHeaderConfigState &operator=(
28+
const RNSScreenStackHeaderConfigState &source) {
29+
this->frameSize.width = source.frameSize.width;
30+
this->frameSize.height = source.frameSize.height;
31+
this->edgeInsets = source.edgeInsets;
32+
return *this;
33+
}
34+
35+
bool operator==(const RNSScreenStackHeaderConfigState &other) {
36+
return this->frameSize == other.frameSize &&
37+
this->edgeInsets == other.edgeInsets;
38+
}
39+
40+
bool operator!=(const RNSScreenStackHeaderConfigState &other) {
41+
return this->frameSize != other.frameSize ||
42+
this->edgeInsets != other.edgeInsets;
43+
}
44+
#endif
2245

2346
#ifdef ANDROID
2447
RNSScreenStackHeaderConfigState(
@@ -45,7 +68,11 @@ class JSI_EXPORT RNSScreenStackHeaderConfigState final {
4568
#endif // !NDEBUG
4669
#endif // ANDROID
4770

48-
const Size frameSize{};
71+
Size frameSize{};
72+
73+
#if !defined(ANDROID)
74+
EdgeInsets edgeInsets{}; // zero initialized
75+
#endif
4976

5077
#ifdef ANDROID
5178
Float paddingStart{0.f};

common/cpp/react/renderer/components/rnscreens/RNSScreenStackHeaderSubviewComponentDescriptor.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class RNSScreenStackHeaderSubviewComponentDescriptor final
2020
using ConcreteComponentDescriptor::ConcreteComponentDescriptor;
2121

2222
void adopt(ShadowNode &shadowNode) const override {
23+
#ifdef ANDROID
2324
react_native_assert(
2425
dynamic_cast<RNSScreenStackHeaderSubviewShadowNode *>(&shadowNode));
2526
auto &subviewShadowNode =
@@ -38,6 +39,7 @@ class RNSScreenStackHeaderSubviewComponentDescriptor final
3839
if (!isSizeEmpty(stateData.frameSize)) {
3940
layoutableShadowNode.setSize(stateData.frameSize);
4041
}
42+
#endif
4143

4244
ConcreteComponentDescriptor::adopt(shadowNode);
4345
}

ios/RNSScreenStackHeaderConfig.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ NS_ASSUME_NONNULL_END
103103
* Allows to send information with size to the corresponding node in shadow tree.
104104
* This method updates state of header config shadow node only.
105105
*/
106-
- (void)updateHeaderConfigState:(CGSize)size;
106+
- (void)updateShadowStateWithSize:(CGSize)size edgeInsets:(NSDirectionalEdgeInsets)edgeInsets;
107107

108108
/**
109109
* Updates state of header config shadow node and all subview shadow nodes in context of given UINavigationBar.

ios/RNSScreenStackHeaderConfig.mm

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#import <react/renderer/components/rnscreens/RCTComponentViewHelpers.h>
1212
#import <rnscreens/RNSScreenStackHeaderConfigComponentDescriptor.h>
1313
#import "RCTImageComponentView+RNSScreenStackHeaderConfig.h"
14+
#import "UINavigationBar+RNSUtility.h"
1415
#ifndef NDEBUG
1516
#import <react/utils/ManagedObjectWrapper.h>
1617
#endif // !NDEBUG
@@ -71,7 +72,8 @@ @implementation RNSScreenStackHeaderConfig {
7172
NSMutableArray<RNSScreenStackHeaderSubview *> *_reactSubviews;
7273
#ifdef RCT_NEW_ARCH_ENABLED
7374
BOOL _initialPropsSet;
74-
CGSize _lastSize;
75+
76+
react::RNSScreenStackHeaderConfigState _lastSendState;
7577
react::RNSScreenStackHeaderConfigShadowNode::ConcreteState::Shared _state;
7678

7779
/// Whether a react subview has been added / removed in current transaction. This flag is reset after each react
@@ -102,6 +104,7 @@ - (instancetype)initWithFrame:(CGRect)frame
102104
_show = YES;
103105
_translucent = NO;
104106
_addedReactSubviewsInCurrentTransaction = false;
107+
_lastSendState = react::RNSScreenStackHeaderConfigState(react::Size{}, react::EdgeInsets{});
105108
[self initProps];
106109
}
107110
return self;
@@ -214,12 +217,17 @@ - (void)layoutNavigationControllerView
214217
}
215218

216219
#ifdef RCT_NEW_ARCH_ENABLED
217-
- (void)updateHeaderConfigState:(CGSize)size
220+
- (void)updateShadowStateWithSize:(CGSize)size edgeInsets:(NSDirectionalEdgeInsets)edgeInsets
218221
{
219-
if (!CGSizeEqualToSize(size, _lastSize)) {
220-
auto newState = react::RNSScreenStackHeaderConfigState(RCTSizeFromCGSize(size));
222+
// I believe Yoga handles RTL internally & .left will be treated as .right in RTL etc.
223+
react::EdgeInsets convertedEdgeInsets{
224+
.left = edgeInsets.leading, .top = edgeInsets.top, .right = edgeInsets.trailing, .bottom = edgeInsets.bottom};
225+
react::Size convertedSize = RCTSizeFromCGSize(size);
226+
auto newState = react::RNSScreenStackHeaderConfigState(convertedSize, convertedEdgeInsets);
227+
228+
if (newState != _lastSendState) {
229+
_lastSendState = newState;
221230
_state->updateState(std::move(newState));
222-
_lastSize = size;
223231
}
224232
}
225233

@@ -229,12 +237,33 @@ - (void)updateHeaderStateInShadowTreeInContextOfNavigationBar:(nullable UINaviga
229237
return;
230238
}
231239

232-
[self updateHeaderConfigState:navigationBar.frame.size];
240+
[self updateShadowStateWithSize:navigationBar.frame.size
241+
edgeInsets:[self computeEdgeInsetsOfNavigationBar:navigationBar]];
233242
for (RNSScreenStackHeaderSubview *subview in self.reactSubviews) {
234-
CGRect frameInNavBarCoordinates = [subview convertRect:subview.frame toView:navigationBar];
235-
[subview updateHeaderSubviewFrameInShadowTree:frameInNavBarCoordinates];
243+
[subview updateShadowStateInContextOfAncestorView:navigationBar];
236244
}
237245
}
246+
247+
- (NSDirectionalEdgeInsets)computeEdgeInsetsOfNavigationBar:(nonnull UINavigationBar *)navigationBar
248+
{
249+
NSDirectionalEdgeInsets navBarMargins = [navigationBar directionalLayoutMargins];
250+
NSDirectionalEdgeInsets navBarContentMargins = [navigationBar.rnscreens_findContentView directionalLayoutMargins];
251+
252+
BOOL isDisplayingBackButton = [self shouldBackButtonBeVisibleInNavigationBar:navigationBar];
253+
254+
// 44.0 is just "closed eyes default". It is so on device I've tested with, nothing more.
255+
UIView *barButtonView = isDisplayingBackButton ? navigationBar.rnscreens_findBackButtonWrapperView : nil;
256+
CGFloat platformBackButtonWidth = barButtonView != nil ? barButtonView.frame.size.width : 44.0f;
257+
258+
const auto edgeInsets = NSDirectionalEdgeInsets{
259+
.leading =
260+
navBarMargins.leading + navBarContentMargins.leading + (isDisplayingBackButton ? platformBackButtonWidth : 0),
261+
.trailing = navBarMargins.trailing + navBarContentMargins.trailing,
262+
};
263+
264+
return edgeInsets;
265+
}
266+
238267
#else
239268
- (void)updateHeaderConfigState:(NSDirectionalEdgeInsets)insets
240269
{
@@ -897,7 +926,7 @@ - (void)prepareForRecycle
897926
_initialPropsSet = NO;
898927

899928
#ifdef RCT_NEW_ARCH_ENABLED
900-
_lastSize = CGSizeZero;
929+
_lastSendState = react::RNSScreenStackHeaderConfigState(react::Size{}, react::EdgeInsets{});
901930
#else
902931
_lastHeaderInsets = NSDirectionalEdgeInsets{};
903932
#endif

ios/RNSScreenStackHeaderSubview.h

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,30 @@ NS_ASSUME_NONNULL_BEGIN
2121
@property (nonatomic, weak) UIView *reactSuperview;
2222

2323
#ifdef RCT_NEW_ARCH_ENABLED
24-
- (void)updateHeaderSubviewFrameInShadowTree:(CGRect)frame;
24+
/**
25+
* Updates state of the header subview shadow node in shadow tree.
26+
* This method updates state of header subview shadow node only.
27+
*/
28+
- (void)updateShadowStateWithFrame:(CGRect)frame;
29+
30+
/**
31+
* Updates state of the header subview shadow node in shadow tree in context of given ancestor view.
32+
* This method updates state of header subview shadow node only.
33+
*
34+
* @param ancestorView - ancestor view in relation to which, the frame send in state update is computed; if this is
35+
* `nil` the method does nothing.
36+
*/
37+
- (void)updateShadowStateInContextOfAncestorView:(nullable UIView *)ancestorView;
38+
39+
/**
40+
* Updates state of the header subview shadow node in shadow tree in context of given ancestor view.
41+
* This method updates state of header subview shadow node only.
42+
*
43+
* @param ancestorView ancestor view in relation to which, the frame send in state update is computed; if this is
44+
* `nil` the method does nothing.
45+
* @param frame source frame, which will be transformed in relation to `ancestorView`.
46+
*/
47+
- (void)updateShadowStateInContextOfAncestorView:(nullable UIView *)ancestorView withFrame:(CGRect)frame;
2548
#endif
2649

2750
@end

0 commit comments

Comments
 (0)