Skip to content

Commit

Permalink
fix(iOS): header subviews layout on tab change (#2385)
Browse files Browse the repository at this point in the history
## Description

This PR intents to fix header subviews incorrect layout when changing
tabs.

The previous solution did layout the subviews correctly in the test
cases, but triggered an undesirable `layoutIfNeeded` when going back
from tab to tab. In such case the navigation layout happened without
updating subview's layout metrics.

Moving the logic to subview resolves the issue as the re-layout is now
triggered only when subview's layout metrics are updated.

Related fixes from the past:
#2316,
#2248

## Changes

- combined `Test2231.tsx` with `Test432.tsx` to create comprehensive
test case
- moved re-layout logic to subview

## Screenshots / GIFs

### Before
![Screenshot 2024-10-04 at 10 10
15](https://github.com/user-attachments/assets/1a8a747e-fe1d-4b03-ba63-1891732d7d77)

### After
![Screenshot 2024-10-04 at 10 09
37](https://github.com/user-attachments/assets/68b3554f-d67d-47f4-946d-ace60e1bbf83)


## Test code and steps to reproduce

- Use `Test432.tsx` repro

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
  • Loading branch information
alduzy authored Oct 7, 2024
1 parent b099f29 commit 91d89c4
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 106 deletions.
56 changes: 0 additions & 56 deletions apps/src/tests/Test2231.tsx

This file was deleted.

137 changes: 91 additions & 46 deletions apps/src/tests/Test432.tsx
Original file line number Diff line number Diff line change
@@ -1,37 +1,36 @@
import { Pressable, View, Button, Text } from 'react-native';

import { NavigationContainer, useNavigation } from '@react-navigation/native';
import { View, Button, Text } from 'react-native';
import { NavigationContainer } from '@react-navigation/native';
import {
NativeStackScreenProps,
createNativeStackNavigator,
} from '@react-navigation/native-stack';
import React, { useCallback } from 'react';
import React, { useEffect, useLayoutEffect, useState } from 'react';
import { createBottomTabNavigator } from '@react-navigation/bottom-tabs';
import { Square } from '../shared';

type RootStackParamList = {
type StackParamList = {
Home: undefined;
Details: undefined;
Settings: undefined;
Info: undefined;
};
type RootStackScreenProps<T extends keyof RootStackParamList> =
NativeStackScreenProps<RootStackParamList, T>;

const HomeScreen = ({ navigation }: RootStackScreenProps<'Home'>) => {
const [x, setX] = React.useState(false);
React.useEffect(() => {
navigation.setOptions({
headerBackVisible: !x,
headerRight: x
? () => (
<View style={{ backgroundColor: 'green', width: 20, height: 20 }} />
)
: () => (
<View style={{ backgroundColor: 'green', width: 10, height: 10 }} />
),
});
}, [navigation, x]);
type StackScreenProps<T extends keyof StackParamList> = NativeStackScreenProps<
StackParamList,
T
>;

const HomeScreen = ({ navigation }: StackScreenProps<'Home'>) => {
return (
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
<Button title="Tap me for header update" onPress={() => setX(!x)} />
<Button
title={'Go to details'}
onPress={() => navigation.navigate('Details')}
/>
<Button
title={'Go to info'}
onPress={() => navigation.navigate('Info')}
/>
<Button
title={'Show settings'}
onPress={() => navigation.navigate('Settings')}
Expand All @@ -40,49 +39,95 @@ const HomeScreen = ({ navigation }: RootStackScreenProps<'Home'>) => {
);
};

const DetailsScreen = ({ navigation }: StackScreenProps<'Details'>) => {
const [x, setX] = useState(false);
useEffect(() => {
navigation.setOptions({
headerBackVisible: !x,
headerRight: () =>
x ? <Square size={20} color="green" /> : <Square size={10} />,
});
}, [navigation, x]);

return <Button title="Toggle subviews" onPress={() => setX(prev => !prev)} />;
};

const SettingsScreen = () => {
return (
<View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
<Text>Settings</Text>
return <Text>Settings</Text>;
};

const InfoScreen = ({ navigation }: StackScreenProps<'Info'>) => {
const [hasLeftItem, setHasLeftItem] = useState(false);

const square1 = (props: { tintColor?: string }) => (
<View style={{ gap: 8, flexDirection: 'row' }}>
{hasLeftItem && <Square {...props} color="green" size={20} />}
<Square {...props} color="green" size={20} />
</View>
);
};

const RootStack = createNativeStackNavigator<RootStackParamList>();
const RootNavigator = () => {
const navigation = useNavigation();
const headerRight = useCallback(
() => (
<Pressable
onPress={() => {
navigation.goBack();
}}>
<Text>Close</Text>
</Pressable>
),
[navigation],
const square2 = (props: { tintColor?: string }) => (
<Square {...props} color="red" size={20} />
);

useLayoutEffect(() => {
navigation.setOptions({
headerRight: square1,
headerTitle: undefined,
headerLeft: hasLeftItem ? square2 : undefined,
});
}, [navigation, hasLeftItem]);

return (
<RootStack.Navigator>
<RootStack.Screen name="Home" component={HomeScreen} />
<RootStack.Screen
<Button
title="Toggle subviews"
onPress={() => setHasLeftItem(prev => !prev)}
/>
);
};

const Stack = createNativeStackNavigator<StackParamList>();

const StackNavigator = () => {
return (
<Stack.Navigator>
<Stack.Screen
name="Home"
component={HomeScreen}
options={{
headerRight: () => <Square size={20} color="black" />,
}}
/>
<Stack.Screen name="Details" component={DetailsScreen} />
<Stack.Screen
name="Info"
component={InfoScreen}
options={{
headerTintColor: 'hotpink',
}}
/>
<Stack.Screen
name="Settings"
component={SettingsScreen}
options={{
presentation: 'modal',
animation: 'slide_from_bottom',
headerShown: true,
headerRight: headerRight,
headerRight: () => <Square size={30} />,
}}
/>
</RootStack.Navigator>
</Stack.Navigator>
);
};

const Tabs = createBottomTabNavigator();

export default function App() {
return (
<NavigationContainer>
<RootNavigator />
<Tabs.Navigator screenOptions={{ headerShown: false }}>
<Tabs.Screen name="First" component={StackNavigator} />
<Tabs.Screen name="Second" component={StackNavigator} />
</Tabs.Navigator>
</NavigationContainer>
);
}
1 change: 0 additions & 1 deletion apps/src/tests/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ export { default as Test2184 } from './Test2184';
export { default as Test2223 } from './Test2223';
export { default as Test2227 } from './Test2227';
export { default as Test2229 } from './Test2229';
export { default as Test2231 } from './Test2231';
export { default as Test2232 } from './Test2232';
export { default as Test2235 } from './Test2235';
export { default as Test2252 } from './Test2252';
Expand Down
3 changes: 0 additions & 3 deletions ios/RNSScreenStackHeaderConfig.mm
Original file line number Diff line number Diff line change
Expand Up @@ -689,9 +689,6 @@ + (void)updateViewController:(UIViewController *)vc
break;
}
}
// We're forcing a re-layout when the subviews change,
// see: https://github.com/software-mansion/react-native-screens/pull/2316
[navctr.view layoutIfNeeded];
}

// This assignment should be done after `navitem.titleView = ...` assignment (iOS 16.0 bug).
Expand Down
24 changes: 24 additions & 0 deletions ios/RNSScreenStackHeaderSubview.mm
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#import "RNSScreenStackHeaderSubview.h"
#import "RNSConvert.h"
#import "RNSScreenStackHeaderConfig.h"

#ifdef RCT_NEW_ARCH_ENABLED
#import <react/renderer/components/rnscreens/ComponentDescriptors.h>
Expand All @@ -22,6 +23,27 @@ @implementation RNSScreenStackHeaderSubview

#pragma mark - Common

- (nullable RNSScreenStackHeaderConfig *)getHeaderConfig
{
RNSScreenStackHeaderConfig *headerConfig = (RNSScreenStackHeaderConfig *_Nullable)self.reactSuperview;
#ifndef NDEBUG
if (headerConfig != nil && ![headerConfig isKindOfClass:[RNSScreenStackHeaderConfig class]]) {
RCTLogError(@"[RNScreens] Invalid view type, expecting RNSScreenStackHeaderConfig, got: %@", headerConfig);
return nil;
}
#endif
return headerConfig;
}

// We're forcing the navigation controller's view to re-layout
// see: https://github.com/software-mansion/react-native-screens/pull/2385
- (void)layoutNavigationBarIfNeeded
{
RNSScreenStackHeaderConfig *headerConfig = [self getHeaderConfig];
UINavigationController *navctr = headerConfig.screenView.reactViewController.navigationController;
[navctr.navigationBar layoutIfNeeded];
}

#ifdef RCT_NEW_ARCH_ENABLED

#pragma mark - Fabric specific
Expand Down Expand Up @@ -78,6 +100,7 @@ - (void)updateLayoutMetrics:(const react::LayoutMetrics &)layoutMetrics
self);
} else {
self.bounds = CGRect{CGPointZero, frame.size};
[self layoutNavigationBarIfNeeded];
}
}

Expand All @@ -102,6 +125,7 @@ - (void)reactSetFrame:(CGRect)frame
// Block any attempt to set coordinates on RNSScreenStackHeaderSubview. This
// makes UINavigationBar the only one to control the position of header content.
[super reactSetFrame:CGRectMake(0, 0, frame.size.width, frame.size.height)];
[self layoutNavigationBarIfNeeded];
}

#endif // RCT_NEW_ARCH_ENABLED
Expand Down

0 comments on commit 91d89c4

Please sign in to comment.