Skip to content

Commit

Permalink
fix: buggy behaviour of search bar / large title on Fabric with nativ…
Browse files Browse the repository at this point in the history
…e stack v6 (#11502)

## 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.
  • Loading branch information
kkafar committed Aug 1, 2023
1 parent 9e37c5d commit cbdcbfb
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 33 deletions.
19 changes: 12 additions & 7 deletions packages/native-stack/src/views/DebugContainer.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,30 @@ type ContainerProps = ViewProps & {
children: React.ReactNode;
};

let Container = View as unknown as React.ComponentType<ContainerProps>;
/**
* This view must *not* be flattened.
* See https://github.com/software-mansion/react-native-screens/pull/1825
* for detailed explanation.
*/
let DebugContainer = (props: ContainerProps) => {
return <View {...props} collapsable={false} />;
};

if (process.env.NODE_ENV !== 'production') {
const DebugContainer = (props: ContainerProps) => {
DebugContainer = (props: ContainerProps) => {
const { stackPresentation, ...rest } = props;

if (Platform.OS === 'ios' && stackPresentation !== 'push') {
// This is necessary for LogBox
return (
<AppContainer>
<View {...rest} />
<View {...rest} collapsable={false} />
</AppContainer>
);
}

return <View {...rest} />;
return <View {...rest} collapsable={false} />;
};

Container = DebugContainer;
}

export default Container;
export default DebugContainer;
56 changes: 30 additions & 26 deletions packages/native-stack/src/views/NativeStackView.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,14 @@ const MaybeNestedStack = ({
return (
<ScreenStack style={styles.container}>
<Screen enabled style={StyleSheet.absoluteFill}>
{content}
<HeaderConfig
{...options}
route={route}
headerHeight={headerHeight}
headerTopInsetEnabled={headerTopInsetEnabled}
canGoBack
/>
{content}
</Screen>
</ScreenStack>
);
Expand Down Expand Up @@ -291,31 +291,6 @@ const SceneView = ({
headerShown !== false ? headerHeight : parentHeaderHeight ?? 0
}
>
{/**
* `HeaderConfig` needs to be the direct child of `Screen` without any intermediate `View`
* We don't render it conditionally to make it possible to dynamically render a custom `header`
* Otherwise dynamically rendering a custom `header` leaves the native header visible
*
* https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md#screenstackheaderconfig
*/}
<HeaderConfig
{...options}
route={route}
headerBackButtonMenuEnabled={
isRemovePrevented !== undefined
? !isRemovePrevented
: headerBackButtonMenuEnabled
}
headerShown={header !== undefined ? false : headerShown}
headerHeight={headerHeight}
headerBackTitle={
options.headerBackTitle !== undefined
? options.headerBackTitle
: undefined
}
headerTopInsetEnabled={headerTopInsetEnabled}
canGoBack={headerBack !== undefined}
/>
<View
accessibilityElementsHidden={!focused}
importantForAccessibility={
Expand Down Expand Up @@ -350,6 +325,35 @@ const SceneView = ({
</View>
) : null}
</View>
{/**
* `HeaderConfig` needs to be the direct child of `Screen` without any intermediate `View`
* We don't render it conditionally to make it possible to dynamically render a custom `header`
* Otherwise dynamically rendering a custom `header` leaves the native header visible
*
* https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md#screenstackheaderconfig
*
* 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}
headerBackButtonMenuEnabled={
isRemovePrevented !== undefined
? !isRemovePrevented
: headerBackButtonMenuEnabled
}
headerShown={header !== undefined ? false : headerShown}
headerHeight={headerHeight}
headerBackTitle={
options.headerBackTitle !== undefined
? options.headerBackTitle
: undefined
}
headerTopInsetEnabled={headerTopInsetEnabled}
canGoBack={headerBack !== undefined}
/>
</HeaderHeightContext.Provider>
</HeaderShownContext.Provider>
</NavigationRouteContext.Provider>
Expand Down

0 comments on commit cbdcbfb

Please sign in to comment.