Skip to content

Commit

Permalink
fix(iOS): buggy search bar / large title behaviour on Fabric (#1825)
Browse files Browse the repository at this point in the history
## Description

`UIKit` requires `ScrollView` to be at index 0 in given view's subview
array to enable it's interaction with navigation bar. On Fabric our
`MaybeNestedStack` view got flattened, however it was not removed from
hierarchy -- instead the view was attached as first child of the
`RNSScreenView` disabling system interaction between navigation bar and
a scroll view.

## Changes

1. Added `collapsable={false}` to `MaybeNestedStack` view, preventing it
from being flattened by React Native.
2. Moved `HeaderConfig` component "down" -- so it is attached as second
child of a `Screen`. It was necessary, because currently on Fabric we
are adding `RNSScreenStackHeaderConfig` into subviews of the `Screen`
and it also interrupted the interaction.
3. Refactored code that relied on `RNSScreenStackHeaderConfig` being
first child of a `Screen` (if present at all).

Initially I've played around with fixing (modyfing) the view hierarchy
on the native side just by removing / adding some views when scroll view
was being attached under the `RNSScreenView`, however such approach led
to cascade of bugs (fixing first, caused another one and so on). We can
assume that in general discrepancy between native & shadow tree leads to
unpleasant bugs.

Possible alternative solution would be to not add
`RNSScreenStackHeaderConfig` as a subview of `RNSScreenView` on Fabric.
I did not research it, but it seems that it would require more changes.

**Note**

We need to consult with `@react-navigation` whether the
`@react-navigation/native-stack` has to be adjusted.

### Before


https://github.com/software-mansion/react-native-screens/assets/50801299/3b89dee8-a307-44ba-a9f4-02e4dfdd95ac


### After


https://github.com/software-mansion/react-native-screens/assets/50801299/205f24d4-590b-402e-8b0a-848fd4482d59


## Test code and steps to reproduce

`Test1097` in `FabricTestExample` can be used to test these changes.
Just use the scroll view and observe that the navigation bar gets
stretched / dwindled while scrolling down / up respectively.

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
  • Loading branch information
kkafar authored Jul 19, 2023
1 parent f047f95 commit ca6c9dc
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 14 deletions.
1 change: 0 additions & 1 deletion FabricTestExample/src/Test1097.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */
import * as React from 'react';
import { Button, NativeSyntheticEvent, ScrollView } from 'react-native';
import {
Expand Down
9 changes: 8 additions & 1 deletion ios/RNSScreen.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ NS_ASSUME_NONNULL_BEGIN

@end

@class RNSScreenStackHeaderConfig;

@interface RNSScreenView :
#ifdef RCT_NEW_ARCH_ENABLED
RCTViewComponentView
Expand Down Expand Up @@ -88,7 +90,8 @@ NS_ASSUME_NONNULL_BEGIN
// we recreate the behavior of `reactSetFrame` on new architecture
@property (nonatomic) facebook::react::LayoutMetrics oldLayoutMetrics;
@property (nonatomic) facebook::react::LayoutMetrics newLayoutMetrics;
@property (weak, nonatomic) UIView *config;
@property (weak, nonatomic) RNSScreenStackHeaderConfig *config;
@property (nonatomic, readonly) BOOL hasHeaderConfig;
#else
@property (nonatomic, copy) RCTDirectEventBlock onAppear;
@property (nonatomic, copy) RCTDirectEventBlock onDisappear;
Expand All @@ -109,12 +112,16 @@ NS_ASSUME_NONNULL_BEGIN
- (void)notifyDisappear;
- (void)updateBounds;
- (void)notifyDismissedWithCount:(int)dismissCount;
- (instancetype)initWithFrame:(CGRect)frame;
#endif

- (void)notifyTransitionProgress:(double)progress closing:(BOOL)closing goingForward:(BOOL)goingForward;
- (void)notifyDismissCancelledWithDismissCount:(int)dismissCount;
- (BOOL)isModal;

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

@end

@interface UIView (RNSScreen)
Expand Down
28 changes: 20 additions & 8 deletions ios/RNSScreen.mm
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ - (instancetype)initWithFrame:(CGRect)frame
_reactSubviews = [NSMutableArray new];
[self initCommonProps];
}

return self;
}
#endif // RCT_NEW_ARCH_ENABLED
Expand Down Expand Up @@ -531,6 +530,16 @@ - (BOOL)isModal
return self.stackPresentation != RNSScreenStackPresentationPush;
}

- (RNSScreenStackHeaderConfig *_Nullable)findHeaderConfig
{
for (UIView *view in self.reactSubviews) {
if ([view isKindOfClass:RNSScreenStackHeaderConfig.class]) {
return (RNSScreenStackHeaderConfig *)view;
}
}
return nil;
}

#if !TARGET_OS_TV
/**
* Updates settings for sheet presentation controller.
Expand Down Expand Up @@ -588,19 +597,24 @@ - (void)updatePresentationStyle
#pragma mark - Fabric specific
#ifdef RCT_NEW_ARCH_ENABLED

- (BOOL)hasHeaderConfig
{
return _config != nil;
}

+ (facebook::react::ComponentDescriptorProvider)componentDescriptorProvider
{
return facebook::react::concreteComponentDescriptorProvider<facebook::react::RNSScreenComponentDescriptor>();
}

- (void)mountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
{
[super mountChildComponentView:childComponentView index:index];
if ([childComponentView isKindOfClass:[RNSScreenStackHeaderConfig class]]) {
_config = childComponentView;
((RNSScreenStackHeaderConfig *)childComponentView).screenView = self;
_config = (RNSScreenStackHeaderConfig *)childComponentView;
_config.screenView = self;
}
[_reactSubviews insertObject:childComponentView atIndex:index];
[super mountChildComponentView:childComponentView index:index];
}

- (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
Expand Down Expand Up @@ -1194,12 +1208,10 @@ - (void)hideHeaderIfNecessary

// we need to check whether reactSubviews array is empty, because on Fabric child nodes are unmounted first ->
// reactSubviews array may be empty
if (currentIndex > 0 && [self.screenView.reactSubviews count] > 0 &&
[self.screenView.reactSubviews[0] isKindOfClass:[RNSScreenStackHeaderConfig class]]) {
RNSScreenStackHeaderConfig *config = [self.screenView findHeaderConfig];
if (currentIndex > 0 && config != nil) {
UINavigationItem *prevNavigationItem =
[self.navigationController.viewControllers objectAtIndex:currentIndex - 1].navigationItem;
RNSScreenStackHeaderConfig *config = ((RNSScreenStackHeaderConfig *)self.screenView.reactSubviews[0]);

BOOL wasSearchBarActive = prevNavigationItem.searchController.active;

#ifdef RCT_NEW_ARCH_ENABLED
Expand Down
4 changes: 1 addition & 3 deletions ios/RNSScreenStack.mm
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
#ifdef RCT_NEW_ARCH_ENABLED
#import <React/RCTFabricComponentsPlugins.h>
#import <React/RCTMountingTransactionObserving.h>
#import <React/RCTSurfaceTouchHandler.h>
#import <React/UIView+React.h>
#import <react/renderer/components/rnscreens/ComponentDescriptors.h>
#import <react/renderer/components/rnscreens/EventEmitters.h>
#import <react/renderer/components/rnscreens/Props.h>
#import <react/renderer/components/rnscreens/RCTComponentViewHelpers.h>

#import <React/RCTFabricComponentsPlugins.h>

#else
#import <React/RCTBridge.h>
#import <React/RCTRootContentView.h>
Expand Down
9 changes: 8 additions & 1 deletion src/native-stack/views/NativeStackView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ const MaybeNestedStack = ({
]}
// @ts-ignore Wrong props passed to View
stackPresentation={stackPresentation}
// This view must *not* be flattened.
// See https://github.com/software-mansion/react-native-screens/pull/1825
// for detailed explanation.
collapsable={false}
>
{children}
</Container>
Expand Down Expand Up @@ -331,14 +335,17 @@ const RouteView = ({
isHeaderInPush !== false ? headerHeight : parentHeaderHeight ?? 0
}
>
<HeaderConfig {...options} route={route} headerShown={isHeaderInPush} />
<MaybeNestedStack
options={options}
route={route}
stackPresentation={stackPresentation}
>
{renderScene()}
</MaybeNestedStack>
{/* HeaderConfig must not be first child of a Screen.
See https://github.com/software-mansion/react-native-screens/pull/1825
for detailed explanation */}
<HeaderConfig {...options} route={route} headerShown={isHeaderInPush} />
</HeaderHeightContext.Provider>
</Screen>
);
Expand Down

0 comments on commit ca6c9dc

Please sign in to comment.