Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(iOS): wrong height of formSheet in nested stack with fitToContents #2670

Merged
merged 9 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions apps/src/tests/TestFormSheet.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { NavigationContainer, RouteProp } from "@react-navigation/native";
import { NativeStackNavigationProp, createNativeStackNavigator } from "@react-navigation/native-stack";
import React, { useLayoutEffect } from "react";
import { Button, Text, TextInput, View } from "react-native";
import { NavigationContainer, RouteProp } from '@react-navigation/native';
import { NativeStackNavigationProp, createNativeStackNavigator } from '@react-navigation/native-stack';
import React, { useLayoutEffect } from 'react';
import { Button, Text, TextInput, View } from 'react-native';

type RouteParamList = {
Home: undefined;
Expand Down Expand Up @@ -30,10 +30,19 @@ function FormSheet({ navigation }: RouteProps<'FormSheet'>) {
<Button title="Go back" onPress={() => navigation.goBack()} />
</View>
<View style={{ alignItems: 'center' }}>
<TextInput style={{ marginVertical: 12, paddingVertical: 8, backgroundColor: 'lavender', borderRadius: 24, width: '80%' }} placeholder="Trigger keyboard..."></TextInput>
<TextInput style={{ marginVertical: 12, paddingVertical: 8, backgroundColor: 'lavender', borderRadius: 24, width: '80%' }} placeholder="Trigger keyboard..." />
</View>
</View>
)
);
}

function FormSheetFooter() {
return (
<View style={{ height: 64, backgroundColor: 'red' }}>
<Text>Footer</Text>
<Button title="Just click me" onPress={() => console.log('Footer button clicked')} />
</View>
);
}

export default function App() {
Expand All @@ -43,20 +52,15 @@ export default function App() {
<Stack.Screen name="Home" component={Home} />
<Stack.Screen name="FormSheet" component={FormSheet} options={{
presentation: 'formSheet',
sheetAllowedDetents: [0.5, 1.0],
//sheetAllowedDetents: [0.4, 0.75, 1.0],
sheetAllowedDetents: 'fitToContents',
sheetCornerRadius: 8,
headerShown: false,
contentStyle: {
backgroundColor: 'lightblue',
},
unstable_sheetFooter: () => {
return (
<View style={{ height: 64, backgroundColor: 'red' }}>
<Text>Footer</Text>
<Button title="Just click me" onPress={() => console.log('Footer button clicked')} />
</View>
);
}
}}/>
//unstable_sheetFooter: FormSheetFooter,
}} />
</Stack.Navigator>
</NavigationContainer>
);
Expand Down
21 changes: 17 additions & 4 deletions ios/RNSScreen.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#import "RNSEnums.h"
#import "RNSScreenContainer.h"
#import "RNSScreenContentWrapper.h"

#if RCT_NEW_ARCH_ENABLED
#import <React/RCTViewComponentView.h>
Expand Down Expand Up @@ -54,6 +55,7 @@ namespace react = facebook::react;
#else
RCTView
#endif
<RNSScreenContentWrapperDelegate>

@property (nonatomic) BOOL fullScreenSwipeEnabled;
@property (nonatomic) BOOL fullScreenSwipeShadowEnabled;
Expand Down Expand Up @@ -126,9 +128,12 @@ namespace react = facebook::react;
- (void)updateBounds;
- (void)notifyDismissedWithCount:(int)dismissCount;
- (instancetype)initWithFrame:(CGRect)frame;
/// Tell `Screen` that it will be unmounted in next transaction.
/// The component needs this so that we can later decide whether to
/// replace it with snapshot or not.

/**
* Tell `Screen` that it will be unmounted in next transaction.
* The component needs this so that we can later decide whether to
* replace it with snapshot or not.
*/
- (void)willBeUnmountedInUpcomingTransaction;
#else
- (instancetype)initWithBridge:(RCTBridge *)bridge;
Expand All @@ -147,9 +152,17 @@ namespace react = facebook::react;
*/
- (void)invalidate;

/// Looks for header configuration in instance's `reactSubviews` and returns it. If not present returns `nil`.
/**
* Looks for header configuration in instance's `reactSubviews` and returns it. If not present returns `nil`.
*/
- (RNSScreenStackHeaderConfig *_Nullable)findHeaderConfig;

/**
* Returns `YES` if the wrapper has been registered and it should not attempt to register on screen views higher in the
* tree.
*/
- (BOOL)registerContentWrapper:(nonnull RNSScreenContentWrapper *)contentWrapper contentHeightErrata:(float)errata;

@end

@interface UIView (RNSScreen)
Expand Down
45 changes: 32 additions & 13 deletions ios/RNSScreen.mm
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,13 @@
constexpr NSInteger SHEET_FIT_TO_CONTENTS = -1;
constexpr NSInteger SHEET_LARGEST_UNDIMMED_DETENT_NONE = -1;

struct ContentWrapperBox {
__weak RNSScreenContentWrapper *contentWrapper{nil};
float contentHeightErrata{0.f};
};

@interface RNSScreenView () <
UIAdaptivePresentationControllerDelegate,
RNSScreenContentWrapperDelegate,
#if !TARGET_OS_TV
UISheetPresentationControllerDelegate,
#endif
Expand All @@ -62,12 +66,12 @@ @interface RNSScreenView () <
@implementation RNSScreenView {
__weak ReactScrollViewBase *_sheetsScrollView;
BOOL _didSetSheetAllowedDetentsOnController;
ContentWrapperBox _contentWrapperBox;
#ifdef RCT_NEW_ARCH_ENABLED
RCTSurfaceTouchHandler *_touchHandler;
react::RNSScreenShadowNode::ConcreteState::Shared _state;
// on fabric, they are not available by default so we need them exposed here too
NSMutableArray<UIView *> *_reactSubviews;
__weak RNSScreenContentWrapper *_contentWrapper;
#else
__weak RCTBridge *_bridge;
RCTTouchHandler *_touchHandler;
Expand All @@ -89,7 +93,7 @@ - (instancetype)initWithFrame:(CGRect)frame
static const auto defaultProps = std::make_shared<const react::RNSScreenProps>();
_props = defaultProps;
_reactSubviews = [NSMutableArray new];
_contentWrapper = nil;
_contentWrapperBox = {};
[self initCommonProps];
}
return self;
Expand Down Expand Up @@ -384,8 +388,19 @@ - (UIView *)reactSuperview
}
RNS_IGNORE_SUPER_CALL_END

- (BOOL)registerContentWrapper:(RNSScreenContentWrapper *)contentWrapper contentHeightErrata:(float)errata;
{
if (self.stackPresentation != RNSScreenStackPresentationFormSheet) {
return NO;
}
_contentWrapperBox = {.contentWrapper = contentWrapper, .contentHeightErrata = errata};
contentWrapper.delegate = self;
[contentWrapper triggerDelegateUpdate];
return YES;
}

/// This is RNSScreenContentWrapperDelegate method, where we do get notified when React did update frame of our child.
- (void)reactDidSetFrame:(CGRect)reactFrame forContentWrapper:(RNSScreenContentWrapper *)contentWrapepr
- (void)contentWrapper:(RNSScreenContentWrapper *)contentWrapper receivedReactFrame:(CGRect)reactFrame
{
if (self.stackPresentation != RNSScreenStackPresentationFormSheet || _didSetSheetAllowedDetentsOnController == YES) {
return;
Expand All @@ -403,7 +418,8 @@ - (void)reactDidSetFrame:(CGRect)reactFrame forContentWrapper:(RNSScreenContentW
}

if (_sheetAllowedDetents.count > 0 && _sheetAllowedDetents[0].intValue == SHEET_FIT_TO_CONTENTS) {
auto detents = [self detentsFromMaxHeights:@[ [NSNumber numberWithFloat:reactFrame.size.height] ]];
auto detents = [self detentsFromMaxHeights:@[ [NSNumber numberWithFloat:reactFrame.size.height +
_contentWrapperBox.contentHeightErrata] ]];
[self setAllowedDetentsForSheet:sheetController to:detents animate:YES];
}
}
Expand All @@ -416,6 +432,7 @@ - (void)addSubview:(UIView *)view
if ([view isKindOfClass:RNSScreenContentWrapper.class] &&
self.stackPresentation == RNSScreenStackPresentationFormSheet) {
auto contentWrapper = (RNSScreenContentWrapper *)view;
_contentWrapperBox.contentWrapper = contentWrapper;
contentWrapper.delegate = self;
}

Expand Down Expand Up @@ -902,6 +919,7 @@ - (void)updateFormSheetPresentationStyle
if (_stackPresentation != RNSScreenStackPresentationFormSheet) {
return;
}

#if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && defined(__IPHONE_15_0) && \
__IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_15_0
int firstDimmedDetentIndex = _sheetAllowedDetents.count;
Expand All @@ -926,8 +944,10 @@ - (void)updateFormSheetPresentationStyle
// Paper: we do not set anything here, we will set once React computed layout of our React's children, namely
// RNSScreenContentWrapper, which in case of formSheet presentation style does have exactly the same frame as
// actual content. The update will be triggered once our child is mounted and laid out by React.
// Fabric: in this very moment our children are already mounted & laid out. In the very end of this method,
// after all other configuration is applied we trigger content wrapper to send us update on its frame.
// Fabric: no nested stack: in this very moment our children are already mounted & laid out. In the very end
// of this method, after all other configuration is applied we trigger content wrapper to send us update on
// its frame. Fabric: nested stack: we wait until nested content wrapper registers itself with this view and
// then update the dimensions.
} else {
[self setAllowedDetentsForSheet:sheet
to:[self detentsFromMaxHeightFractions:_sheetAllowedDetents]
Expand Down Expand Up @@ -1030,7 +1050,7 @@ - (void)updateFormSheetPresentationStyle
#ifdef RCT_NEW_ARCH_ENABLED
// We trigger update from content wrapper, because on Fabric we update props after the children are mounted & laid
// out.
[self->_contentWrapper triggerDelegateUpdate];
[self->_contentWrapperBox.contentWrapper triggerDelegateUpdate];
#endif // RCT_NEW_ARCH_ENABLED
#endif // Check for iOS >= 15
}
Expand Down Expand Up @@ -1122,9 +1142,8 @@ - (void)mountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompone
if ([childComponentView isKindOfClass:RNSScreenContentWrapper.class]) {
auto contentWrapper = (RNSScreenContentWrapper *)childComponentView;
contentWrapper.delegate = self;
_contentWrapper = contentWrapper;
}
if ([childComponentView isKindOfClass:[RNSScreenStackHeaderConfig class]]) {
_contentWrapperBox.contentWrapper = contentWrapper;
} else if ([childComponentView isKindOfClass:RNSScreenStackHeaderConfig.class]) {
_config = (RNSScreenStackHeaderConfig *)childComponentView;
_config.screenView = self;
}
Expand All @@ -1138,8 +1157,8 @@ - (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompo
_config = nil;
}
if ([childComponentView isKindOfClass:[RNSScreenContentWrapper class]]) {
_contentWrapper.delegate = nil;
_contentWrapper = nil;
_contentWrapperBox.contentWrapper.delegate = nil;
_contentWrapperBox.contentWrapper = nil;
}
[_reactSubviews removeObject:childComponentView];
[super unmountChildComponentView:childComponentView index:index];
Expand Down
4 changes: 2 additions & 2 deletions ios/RNSScreenContentWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ NS_ASSUME_NONNULL_BEGIN
@protocol RNSScreenContentWrapperDelegate <NSObject>

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

@end

Expand Down
79 changes: 68 additions & 11 deletions ios/RNSScreenContentWrapper.mm
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
#import "RNSScreenContentWrapper.h"
#import "RNSScreen.h"
#import "RNSScreenStack.h"

#ifdef RCT_NEW_ARCH_ENABLED
#import <React/RCTConversions.h>
#import <React/RCTLog.h>
#import <react/renderer/components/rnscreens/ComponentDescriptors.h>
#import <react/renderer/components/rnscreens/EventEmitters.h>
#import <react/renderer/components/rnscreens/Props.h>
Expand All @@ -12,34 +15,82 @@

@implementation RNSScreenContentWrapper

#ifndef RCT_NEW_ARCH_ENABLED

- (void)reactSetFrame:(CGRect)frame
{
[super reactSetFrame:frame];
if (self.delegate != nil) {
[self.delegate reactDidSetFrame:frame forContentWrapper:self];
[self.delegate contentWrapper:self receivedReactFrame:frame];
}
}

#endif // !RCT_NEW_ARCH_ENABLED

- (void)notifyDelegateWithFrame:(CGRect)frame
{
[self.delegate contentWrapper:self receivedReactFrame:frame];
}

- (void)triggerDelegateUpdate
{
[self.delegate reactDidSetFrame:self.frame forContentWrapper:self];
[self notifyDelegateWithFrame:self.frame];
}

#ifdef RCT_NEW_ARCH_ENABLED
- (void)willMoveToWindow:(UIWindow *)newWindow
{
if (newWindow == nil) {
return;
}
[self attachToAncestorScreenView];
}

- (void)updateLayoutMetrics:(const facebook::react::LayoutMetrics &)layoutMetrics
oldLayoutMetrics:(const facebook::react::LayoutMetrics &)oldLayoutMetrics
/**
* Searches for first `RNSScreen` instance that uses `formSheet` presentation and returns it together with accumulated
* heights of navigation bars discovered along tree path up.
*
* TODO: Such travelsal method could be defined as its own algorithm in separate helper methods set.
*/
- (void)attachToAncestorScreenViewStartingFrom:(nonnull RNSScreen *)screenCtrl
{
[super updateLayoutMetrics:layoutMetrics oldLayoutMetrics:oldLayoutMetrics];
if (self.delegate != nil) {
[self.delegate reactDidSetFrame:RCTCGRectFromRect(layoutMetrics.frame) forContentWrapper:self];
UIViewController *controller = screenCtrl;
float headerHeightErrata = 0.f;

do {
if ([controller isKindOfClass:RNSScreen.class]) {
RNSScreen *currentScreen = static_cast<RNSScreen *>(controller);
if ([currentScreen.screenView registerContentWrapper:self contentHeightErrata:headerHeightErrata]) {
break;
}
} else if ([controller isKindOfClass:RNSNavigationController.class]) {
UINavigationBar *navigationBar = static_cast<RNSNavigationController *>(controller).navigationBar;
headerHeightErrata += navigationBar.frame.size.height * !navigationBar.isHidden;
}

controller = controller.parentViewController;
} while (controller != nil);
}

- (void)attachToAncestorScreenView
{
if (![self.reactSuperview isKindOfClass:RNSScreenView.class]) {
RCTLogError(@"Expected reactSuperview to be a RNSScreenView. Found %@", self.reactSuperview);
return;
}

RNSScreen *screen = (RNSScreen *)[self.reactSuperview reactViewController];
[self attachToAncestorScreenViewStartingFrom:screen];
}

// Needed because of this: https://github.com/facebook/react-native/pull/37274
+ (void)load
#ifdef RCT_NEW_ARCH_ENABLED

#pragma mark - RCTComponentViewProtocol

- (void)updateLayoutMetrics:(const facebook::react::LayoutMetrics &)layoutMetrics
oldLayoutMetrics:(const facebook::react::LayoutMetrics &)oldLayoutMetrics
{
[super load];
[super updateLayoutMetrics:layoutMetrics oldLayoutMetrics:oldLayoutMetrics];
[self notifyDelegateWithFrame:RCTCGRectFromRect(layoutMetrics.frame)];
}

+ (react::ComponentDescriptorProvider)componentDescriptorProvider
Expand All @@ -51,6 +102,12 @@ + (void)load
{
return RNSScreenContentWrapper.class;
}

// Needed because of this: https://github.com/facebook/react-native/pull/37274
+ (void)load
{
[super load];
}
#endif // RCT_NEW_ARCH_ENABLED

@end
Expand Down
Loading