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): change implementation of calculating status bar, refactor methods used on header height change #1917

Merged
merged 17 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from 8 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
49 changes: 10 additions & 39 deletions ios/RNSScreen.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,7 @@ - (BOOL)hasNestedStack
return NO;
}

- (CGFloat)getCalculatedHeaderHeightIsModal:(BOOL)isModal
- (CGFloat)getNavigationBarHeightIsModal:(BOOL)isModal
{
CGFloat navbarHeight = self.navigationController.navigationBar.frame.size.height;

Expand All @@ -1058,50 +1058,21 @@ - (CGFloat)getCalculatedHeaderHeightIsModal:(BOOL)isModal
return navbarHeight;
}

- (CGSize)getCalculatedStatusBarHeightIsModal:(BOOL)isModal
- (CGFloat)getNavigationBarInsetIsModal:(BOOL)isModal
{
#if !TARGET_OS_TV
BOOL isDraggableModal = isModal && ![self.screenView isFullscreenModal];
BOOL isDraggableModalWithChildViewCtr =
isDraggableModal && self.childViewControllers.count > 0 && self.childViewControllers[0] != nil;

// When modal is floating (we can grab its header), we don't want to calculate status bar in it.
// Thus, we return '0' as a height of status bar.
if (isDraggableModalWithChildViewCtr || self.screenView.isTransparentModal) {
return CGSizeMake(0, 0);
}

CGSize fallbackStatusBarSize = [[UIApplication sharedApplication] statusBarFrame].size;

#if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && defined(__IPHONE_13_0) && \
__IPHONE_OS_VERSION_MAX_ALLOWED >= __IPHONE_13_0
if (@available(iOS 13.0, *)) {
CGSize primaryStatusBarSize = self.view.window.windowScene.statusBarManager.statusBarFrame.size;
if (primaryStatusBarSize.height == 0 || primaryStatusBarSize.width == 0)
return fallbackStatusBarSize;

return primaryStatusBarSize;
} else {
return fallbackStatusBarSize;
}
#endif /* Check for iOS 13.0 */
#if TARGET_OS_TV
// On TVOS there's no inset of navigation bar.
return 0;
#endif // TARGET_OS_TV

#else
// On TVOS, status bar doesn't exist
return CGSizeMake(0, 0);
#endif // !TARGET_OS_TV
return self.navigationController.navigationBar.frame.origin.y;
}

- (CGFloat)calculateHeaderHeightIsModal:(BOOL)isModal
{
CGFloat navbarHeight = [self getCalculatedHeaderHeightIsModal:isModal];
CGSize statusBarSize = [self getCalculatedStatusBarHeightIsModal:isModal];

// Unfortunately, UIKit doesn't care about switching width and height options on screen rotation.
// We should check if user has rotated its screen, so we're choosing the minimum value between the
// width and height.
CGFloat statusBarHeight = MIN(statusBarSize.width, statusBarSize.height);
return navbarHeight + statusBarHeight;
CGFloat navbarHeight = [self getNavigationBarHeightIsModal:isModal];
CGFloat navbarInset = [self getNavigationBarInsetIsModal:isModal];
return navbarHeight + navbarInset;
tboba marked this conversation as resolved.
Show resolved Hide resolved
}

- (void)calculateAndNotifyHeaderHeightChangeIsModal:(BOOL)isModal
Copy link
Member Author

@tboba tboba Oct 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment from me - leaving the name calculateHeaderHeightIsModal is made on purpose here. I did not change it to calculateNavigationBarHeightIsModal because for me header is the navigationBar + an inset of it - that's why I'm leaving it here.

Expand Down
8 changes: 8 additions & 0 deletions ios/RNSScreenStackHeaderConfig.mm
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ - (void)updateViewControllerIfNeeded
// if nav is nil, it means we can be in a fullScreen modal, so there is no nextVC, but we still want to update
if (vc != nil && (nextVC == vc || isInFullScreenModal || isPresentingVC)) {
[RNSScreenStackHeaderConfig updateViewController:self.screenView.controller withConfig:self animated:YES];
// As the header might have change in `updateViewController` we need to ensure that header height
// returned by the `onHeaderHeightChange` event is correct.
[self.screenView.controller calculateAndNotifyHeaderHeightChangeIsModal:NO];
}
}

Expand Down Expand Up @@ -353,6 +356,11 @@ + (void)willShowViewController:(UIViewController *)vc
withConfig:(RNSScreenStackHeaderConfig *)config
{
[self updateViewController:vc withConfig:config animated:animated];
// As the header might have change in `updateViewController` we need to ensure that header height
// returned by the `onHeaderHeightChange` event is correct.
if ([vc isKindOfClass:[RNSScreen class]]) {
[(RNSScreen *)vc calculateAndNotifyHeaderHeightChangeIsModal:NO];
}
Comment on lines +359 to +363
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I'm not sure, so we can leave it, but I have a feeling that it is also always RNSScreen.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep it for now - I'm afraid if there will be some cases that vc won't be actually RNSScreen (RNSScreenStack?) and that may cause some crashes on iOS.

}

#if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && defined(__IPHONE_13_0) && \
Expand Down