Skip to content

Commit 51a1488

Browse files
authored
fix(iOS): wrong height of formSheet in nested stack with fitToContents (#2670)
## Description Closes #2665 In case when we present `formSheet` with header, the content is nested in another stack -> content wrapper is *not* mounted directly under presented screen --> we need to forward information of content dimensions from nested content wrapper to the screen with `formSheet` presentation. Due to order of updates on Fabric (bottom up assembling of the host tree) first moment when we can look for *ancestor* screen with `formSheet` presentation is when we move to window... And that is what I did. Now `RNSScreenContentWrapper` looks for appropriate screen to attach to in `willMoveToWindow`. There was another problem: any stack along the way impacts desired height of the sheet -> therefore while looking for appropriate screen the wrapper sums up heights of encountered navigation bars and gives that information to the screen. > [!important] using `formSheet` with header enabled could lead to flicker (we won't be able to solve it for now, until we are able to trigger react commit & layout synchronously). Maybe we should describe this in types? ## Changes ☝🏻 ## Test code and steps to reproduce `TestFormSheet` + set `headerShown: true` on `formSheet` screen. ## Checklist - [ ] Included code example that can be used to test this change - [ ] Ensured that CI passes
1 parent 32c3a19 commit 51a1488

5 files changed

Lines changed: 139 additions & 46 deletions

File tree

apps/src/tests/TestFormSheet.tsx

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { NavigationContainer, RouteProp } from "@react-navigation/native";
2-
import { NativeStackNavigationProp, createNativeStackNavigator } from "@react-navigation/native-stack";
3-
import React, { useLayoutEffect } from "react";
4-
import { Button, Text, TextInput, View } from "react-native";
1+
import { NavigationContainer, RouteProp } from '@react-navigation/native';
2+
import { NativeStackNavigationProp, createNativeStackNavigator } from '@react-navigation/native-stack';
3+
import React, { useLayoutEffect } from 'react';
4+
import { Button, Text, TextInput, View } from 'react-native';
55

66
type RouteParamList = {
77
Home: undefined;
@@ -30,10 +30,19 @@ function FormSheet({ navigation }: RouteProps<'FormSheet'>) {
3030
<Button title="Go back" onPress={() => navigation.goBack()} />
3131
</View>
3232
<View style={{ alignItems: 'center' }}>
33-
<TextInput style={{ marginVertical: 12, paddingVertical: 8, backgroundColor: 'lavender', borderRadius: 24, width: '80%' }} placeholder="Trigger keyboard..."></TextInput>
33+
<TextInput style={{ marginVertical: 12, paddingVertical: 8, backgroundColor: 'lavender', borderRadius: 24, width: '80%' }} placeholder="Trigger keyboard..." />
3434
</View>
3535
</View>
36-
)
36+
);
37+
}
38+
39+
function FormSheetFooter() {
40+
return (
41+
<View style={{ height: 64, backgroundColor: 'red' }}>
42+
<Text>Footer</Text>
43+
<Button title="Just click me" onPress={() => console.log('Footer button clicked')} />
44+
</View>
45+
);
3746
}
3847

3948
export default function App() {
@@ -43,20 +52,15 @@ export default function App() {
4352
<Stack.Screen name="Home" component={Home} />
4453
<Stack.Screen name="FormSheet" component={FormSheet} options={{
4554
presentation: 'formSheet',
46-
sheetAllowedDetents: [0.5, 1.0],
55+
//sheetAllowedDetents: [0.4, 0.75, 1.0],
56+
sheetAllowedDetents: 'fitToContents',
4757
sheetCornerRadius: 8,
58+
headerShown: false,
4859
contentStyle: {
4960
backgroundColor: 'lightblue',
5061
},
51-
unstable_sheetFooter: () => {
52-
return (
53-
<View style={{ height: 64, backgroundColor: 'red' }}>
54-
<Text>Footer</Text>
55-
<Button title="Just click me" onPress={() => console.log('Footer button clicked')} />
56-
</View>
57-
);
58-
}
59-
}}/>
62+
//unstable_sheetFooter: FormSheetFooter,
63+
}} />
6064
</Stack.Navigator>
6165
</NavigationContainer>
6266
);

ios/RNSScreen.h

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#import "RNSEnums.h"
55
#import "RNSScreenContainer.h"
6+
#import "RNSScreenContentWrapper.h"
67

78
#if RCT_NEW_ARCH_ENABLED
89
#import <React/RCTViewComponentView.h>
@@ -54,6 +55,7 @@ namespace react = facebook::react;
5455
#else
5556
RCTView
5657
#endif
58+
<RNSScreenContentWrapperDelegate>
5759

5860
@property (nonatomic) BOOL fullScreenSwipeEnabled;
5961
@property (nonatomic) BOOL fullScreenSwipeShadowEnabled;
@@ -126,9 +128,12 @@ namespace react = facebook::react;
126128
- (void)updateBounds;
127129
- (void)notifyDismissedWithCount:(int)dismissCount;
128130
- (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.
131+
132+
/**
133+
* Tell `Screen` that it will be unmounted in next transaction.
134+
* The component needs this so that we can later decide whether to
135+
* replace it with snapshot or not.
136+
*/
132137
- (void)willBeUnmountedInUpcomingTransaction;
133138
#else
134139
- (instancetype)initWithBridge:(RCTBridge *)bridge;
@@ -147,9 +152,17 @@ namespace react = facebook::react;
147152
*/
148153
- (void)invalidate;
149154

150-
/// Looks for header configuration in instance's `reactSubviews` and returns it. If not present returns `nil`.
155+
/**
156+
* Looks for header configuration in instance's `reactSubviews` and returns it. If not present returns `nil`.
157+
*/
151158
- (RNSScreenStackHeaderConfig *_Nullable)findHeaderConfig;
152159

160+
/**
161+
* Returns `YES` if the wrapper has been registered and it should not attempt to register on screen views higher in the
162+
* tree.
163+
*/
164+
- (BOOL)registerContentWrapper:(nonnull RNSScreenContentWrapper *)contentWrapper contentHeightErrata:(float)errata;
165+
153166
@end
154167

155168
@interface UIView (RNSScreen)

ios/RNSScreen.mm

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,13 @@
4444
constexpr NSInteger SHEET_FIT_TO_CONTENTS = -1;
4545
constexpr NSInteger SHEET_LARGEST_UNDIMMED_DETENT_NONE = -1;
4646

47+
struct ContentWrapperBox {
48+
__weak RNSScreenContentWrapper *contentWrapper{nil};
49+
float contentHeightErrata{0.f};
50+
};
51+
4752
@interface RNSScreenView () <
4853
UIAdaptivePresentationControllerDelegate,
49-
RNSScreenContentWrapperDelegate,
5054
#if !TARGET_OS_TV
5155
UISheetPresentationControllerDelegate,
5256
#endif
@@ -62,12 +66,12 @@ @interface RNSScreenView () <
6266
@implementation RNSScreenView {
6367
__weak ReactScrollViewBase *_sheetsScrollView;
6468
BOOL _didSetSheetAllowedDetentsOnController;
69+
ContentWrapperBox _contentWrapperBox;
6570
#ifdef RCT_NEW_ARCH_ENABLED
6671
RCTSurfaceTouchHandler *_touchHandler;
6772
react::RNSScreenShadowNode::ConcreteState::Shared _state;
6873
// on fabric, they are not available by default so we need them exposed here too
6974
NSMutableArray<UIView *> *_reactSubviews;
70-
__weak RNSScreenContentWrapper *_contentWrapper;
7175
#else
7276
__weak RCTBridge *_bridge;
7377
RCTTouchHandler *_touchHandler;
@@ -89,7 +93,7 @@ - (instancetype)initWithFrame:(CGRect)frame
8993
static const auto defaultProps = std::make_shared<const react::RNSScreenProps>();
9094
_props = defaultProps;
9195
_reactSubviews = [NSMutableArray new];
92-
_contentWrapper = nil;
96+
_contentWrapperBox = {};
9397
[self initCommonProps];
9498
}
9599
return self;
@@ -384,8 +388,19 @@ - (UIView *)reactSuperview
384388
}
385389
RNS_IGNORE_SUPER_CALL_END
386390

391+
- (BOOL)registerContentWrapper:(RNSScreenContentWrapper *)contentWrapper contentHeightErrata:(float)errata;
392+
{
393+
if (self.stackPresentation != RNSScreenStackPresentationFormSheet) {
394+
return NO;
395+
}
396+
_contentWrapperBox = {.contentWrapper = contentWrapper, .contentHeightErrata = errata};
397+
contentWrapper.delegate = self;
398+
[contentWrapper triggerDelegateUpdate];
399+
return YES;
400+
}
401+
387402
/// This is RNSScreenContentWrapperDelegate method, where we do get notified when React did update frame of our child.
388-
- (void)reactDidSetFrame:(CGRect)reactFrame forContentWrapper:(RNSScreenContentWrapper *)contentWrapepr
403+
- (void)contentWrapper:(RNSScreenContentWrapper *)contentWrapper receivedReactFrame:(CGRect)reactFrame
389404
{
390405
if (self.stackPresentation != RNSScreenStackPresentationFormSheet || _didSetSheetAllowedDetentsOnController == YES) {
391406
return;
@@ -403,7 +418,8 @@ - (void)reactDidSetFrame:(CGRect)reactFrame forContentWrapper:(RNSScreenContentW
403418
}
404419

405420
if (_sheetAllowedDetents.count > 0 && _sheetAllowedDetents[0].intValue == SHEET_FIT_TO_CONTENTS) {
406-
auto detents = [self detentsFromMaxHeights:@[ [NSNumber numberWithFloat:reactFrame.size.height] ]];
421+
auto detents = [self detentsFromMaxHeights:@[ [NSNumber numberWithFloat:reactFrame.size.height +
422+
_contentWrapperBox.contentHeightErrata] ]];
407423
[self setAllowedDetentsForSheet:sheetController to:detents animate:YES];
408424
}
409425
}
@@ -416,6 +432,7 @@ - (void)addSubview:(UIView *)view
416432
if ([view isKindOfClass:RNSScreenContentWrapper.class] &&
417433
self.stackPresentation == RNSScreenStackPresentationFormSheet) {
418434
auto contentWrapper = (RNSScreenContentWrapper *)view;
435+
_contentWrapperBox.contentWrapper = contentWrapper;
419436
contentWrapper.delegate = self;
420437
}
421438

@@ -902,6 +919,7 @@ - (void)updateFormSheetPresentationStyle
902919
if (_stackPresentation != RNSScreenStackPresentationFormSheet) {
903920
return;
904921
}
922+
905923
#if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && defined(__IPHONE_15_0) && \
906924
__IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_15_0
907925
int firstDimmedDetentIndex = _sheetAllowedDetents.count;
@@ -926,8 +944,10 @@ - (void)updateFormSheetPresentationStyle
926944
// Paper: we do not set anything here, we will set once React computed layout of our React's children, namely
927945
// RNSScreenContentWrapper, which in case of formSheet presentation style does have exactly the same frame as
928946
// actual content. The update will be triggered once our child is mounted and laid out by React.
929-
// Fabric: in this very moment our children are already mounted & laid out. In the very end of this method,
930-
// after all other configuration is applied we trigger content wrapper to send us update on its frame.
947+
// Fabric: no nested stack: in this very moment our children are already mounted & laid out. In the very end
948+
// of this method, after all other configuration is applied we trigger content wrapper to send us update on
949+
// its frame. Fabric: nested stack: we wait until nested content wrapper registers itself with this view and
950+
// then update the dimensions.
931951
} else {
932952
[self setAllowedDetentsForSheet:sheet
933953
to:[self detentsFromMaxHeightFractions:_sheetAllowedDetents]
@@ -1030,7 +1050,7 @@ - (void)updateFormSheetPresentationStyle
10301050
#ifdef RCT_NEW_ARCH_ENABLED
10311051
// We trigger update from content wrapper, because on Fabric we update props after the children are mounted & laid
10321052
// out.
1033-
[self->_contentWrapper triggerDelegateUpdate];
1053+
[self->_contentWrapperBox.contentWrapper triggerDelegateUpdate];
10341054
#endif // RCT_NEW_ARCH_ENABLED
10351055
#endif // Check for iOS >= 15
10361056
}
@@ -1122,9 +1142,8 @@ - (void)mountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompone
11221142
if ([childComponentView isKindOfClass:RNSScreenContentWrapper.class]) {
11231143
auto contentWrapper = (RNSScreenContentWrapper *)childComponentView;
11241144
contentWrapper.delegate = self;
1125-
_contentWrapper = contentWrapper;
1126-
}
1127-
if ([childComponentView isKindOfClass:[RNSScreenStackHeaderConfig class]]) {
1145+
_contentWrapperBox.contentWrapper = contentWrapper;
1146+
} else if ([childComponentView isKindOfClass:RNSScreenStackHeaderConfig.class]) {
11281147
_config = (RNSScreenStackHeaderConfig *)childComponentView;
11291148
_config.screenView = self;
11301149
}
@@ -1138,8 +1157,8 @@ - (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompo
11381157
_config = nil;
11391158
}
11401159
if ([childComponentView isKindOfClass:[RNSScreenContentWrapper class]]) {
1141-
_contentWrapper.delegate = nil;
1142-
_contentWrapper = nil;
1160+
_contentWrapperBox.contentWrapper.delegate = nil;
1161+
_contentWrapperBox.contentWrapper = nil;
11431162
}
11441163
[_reactSubviews removeObject:childComponentView];
11451164
[super unmountChildComponentView:childComponentView index:index];

ios/RNSScreenContentWrapper.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ NS_ASSUME_NONNULL_BEGIN
1515
@protocol RNSScreenContentWrapperDelegate <NSObject>
1616

1717
/**
18-
* This method is called by the content wrapper on a delegate when React Native updates the layout.
18+
* Called by the content wrapper on a delegate when React Native updates the layout.
1919
*/
20-
- (void)reactDidSetFrame:(CGRect)reactFrame forContentWrapper:(RNSScreenContentWrapper *)contentWrapepr;
20+
- (void)contentWrapper:(RNSScreenContentWrapper *)contentWrapper receivedReactFrame:(CGRect)reactFrame;
2121

2222
@end
2323

ios/RNSScreenContentWrapper.mm

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
#import "RNSScreenContentWrapper.h"
2+
#import "RNSScreen.h"
3+
#import "RNSScreenStack.h"
24

35
#ifdef RCT_NEW_ARCH_ENABLED
46
#import <React/RCTConversions.h>
7+
#import <React/RCTLog.h>
58
#import <react/renderer/components/rnscreens/ComponentDescriptors.h>
69
#import <react/renderer/components/rnscreens/EventEmitters.h>
710
#import <react/renderer/components/rnscreens/Props.h>
@@ -12,34 +15,82 @@
1215

1316
@implementation RNSScreenContentWrapper
1417

18+
#ifndef RCT_NEW_ARCH_ENABLED
19+
1520
- (void)reactSetFrame:(CGRect)frame
1621
{
1722
[super reactSetFrame:frame];
1823
if (self.delegate != nil) {
19-
[self.delegate reactDidSetFrame:frame forContentWrapper:self];
24+
[self.delegate contentWrapper:self receivedReactFrame:frame];
2025
}
2126
}
2227

28+
#endif // !RCT_NEW_ARCH_ENABLED
29+
30+
- (void)notifyDelegateWithFrame:(CGRect)frame
31+
{
32+
[self.delegate contentWrapper:self receivedReactFrame:frame];
33+
}
34+
2335
- (void)triggerDelegateUpdate
2436
{
25-
[self.delegate reactDidSetFrame:self.frame forContentWrapper:self];
37+
[self notifyDelegateWithFrame:self.frame];
2638
}
2739

28-
#ifdef RCT_NEW_ARCH_ENABLED
40+
- (void)willMoveToWindow:(UIWindow *)newWindow
41+
{
42+
if (newWindow == nil) {
43+
return;
44+
}
45+
[self attachToAncestorScreenView];
46+
}
2947

30-
- (void)updateLayoutMetrics:(const facebook::react::LayoutMetrics &)layoutMetrics
31-
oldLayoutMetrics:(const facebook::react::LayoutMetrics &)oldLayoutMetrics
48+
/**
49+
* Searches for first `RNSScreen` instance that uses `formSheet` presentation and returns it together with accumulated
50+
* heights of navigation bars discovered along tree path up.
51+
*
52+
* TODO: Such travelsal method could be defined as its own algorithm in separate helper methods set.
53+
*/
54+
- (void)attachToAncestorScreenViewStartingFrom:(nonnull RNSScreen *)screenCtrl
3255
{
33-
[super updateLayoutMetrics:layoutMetrics oldLayoutMetrics:oldLayoutMetrics];
34-
if (self.delegate != nil) {
35-
[self.delegate reactDidSetFrame:RCTCGRectFromRect(layoutMetrics.frame) forContentWrapper:self];
56+
UIViewController *controller = screenCtrl;
57+
float headerHeightErrata = 0.f;
58+
59+
do {
60+
if ([controller isKindOfClass:RNSScreen.class]) {
61+
RNSScreen *currentScreen = static_cast<RNSScreen *>(controller);
62+
if ([currentScreen.screenView registerContentWrapper:self contentHeightErrata:headerHeightErrata]) {
63+
break;
64+
}
65+
} else if ([controller isKindOfClass:RNSNavigationController.class]) {
66+
UINavigationBar *navigationBar = static_cast<RNSNavigationController *>(controller).navigationBar;
67+
headerHeightErrata += navigationBar.frame.size.height * !navigationBar.isHidden;
68+
}
69+
70+
controller = controller.parentViewController;
71+
} while (controller != nil);
72+
}
73+
74+
- (void)attachToAncestorScreenView
75+
{
76+
if (![self.reactSuperview isKindOfClass:RNSScreenView.class]) {
77+
RCTLogError(@"Expected reactSuperview to be a RNSScreenView. Found %@", self.reactSuperview);
78+
return;
3679
}
80+
81+
RNSScreen *screen = (RNSScreen *)[self.reactSuperview reactViewController];
82+
[self attachToAncestorScreenViewStartingFrom:screen];
3783
}
3884

39-
// Needed because of this: https://github.com/facebook/react-native/pull/37274
40-
+ (void)load
85+
#ifdef RCT_NEW_ARCH_ENABLED
86+
87+
#pragma mark - RCTComponentViewProtocol
88+
89+
- (void)updateLayoutMetrics:(const facebook::react::LayoutMetrics &)layoutMetrics
90+
oldLayoutMetrics:(const facebook::react::LayoutMetrics &)oldLayoutMetrics
4191
{
42-
[super load];
92+
[super updateLayoutMetrics:layoutMetrics oldLayoutMetrics:oldLayoutMetrics];
93+
[self notifyDelegateWithFrame:RCTCGRectFromRect(layoutMetrics.frame)];
4394
}
4495

4596
+ (react::ComponentDescriptorProvider)componentDescriptorProvider
@@ -51,6 +102,12 @@ + (void)load
51102
{
52103
return RNSScreenContentWrapper.class;
53104
}
105+
106+
// Needed because of this: https://github.com/facebook/react-native/pull/37274
107+
+ (void)load
108+
{
109+
[super load];
110+
}
54111
#endif // RCT_NEW_ARCH_ENABLED
55112

56113
@end

0 commit comments

Comments
 (0)