-
-
Notifications
You must be signed in to change notification settings - Fork 530
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(iOS, Paper): fix header layout when updating non focued screen (#…
…2552) ## Description > [!note] > This issue seems to concern only old architecture. See below for description of Fabric situation 👇🏻 This PR aims to fix a bug described below 👇🏻 and at the same time balancing on the thin edge of not introducing regressions regarding: #2316, #2248, #2385. ### Bug description See `Test2552`. The flow is as follows: 1. we have tab navigator with nested stack navigators on each tab (A & B), 2. In `useLayoutEffect` we schedule a timer which callback changing the subview elements, 3. before the timer fires we change the tab from A to B, 4. wait few seconds fot timer to fire, 5. go back to A, 6. notice that the subviews are laid out incorrectly (video below 👇🏻) https://github.com/user-attachments/assets/2bf621a7-efd4-44cf-95e1-45a46e425f07 Basically what happens is we're sending `layoutIfNeeded` to navigation bar before subviews are mounted under navigation bar view hierarchy. This causes "isLayoutDirty" flags to be cleaned up and subsequent `layoutIfNeeded` messages have no effect. ## Changes We now wait with triggering layout for the subview to be attached to window. > [!note] > TODO: possibly we should call the layout from `didMoveToWindow` but I haven't found the case it does not work without the call, so I'm not adding it for now. > [!note] > Calling layout on whole navigation bar for every subview update seems wrong, however the change is subview change can have effect on other neighbouring views (e.g. long title which need to be truncated) & it seems that we need to do this. Maybe we could get away will requesting it only from UINavigationBarContentView skipping few levels, but this is left for consideration in the future. > [!important] > I do not understand why we need to send `layoutIfNeeded` and `setNeedsLayout` is not enough, but not sending the former results in regressions in Test432. Leaving it for future considerations. ### Fabric The strategy with setting screen options inside timer nested in useLayoutEffect seems to not work at all on new architecture. My impression is that the timer gets cancelled every time the screen loses focus (tab is changed). I do not know whether this is a bug on our side, or maybe it should work this way or it is Fabric bug. This should be debugged in future. ## Test code and steps to reproduce Test2552 - Follows the steps described above ☝🏻 Test432 - Follow the steps from issues described in mentioned issues :point_up: ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation: <!-- For adding new props to native-stack --> - [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [ ] Ensured that CI passes
- Loading branch information
Showing
4 changed files
with
215 additions
and
9 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,182 @@ | ||
| import { View, Button, StyleSheet } from 'react-native'; | ||
| import { NavigationContainer } from '@react-navigation/native'; | ||
| import { | ||
| NativeStackScreenProps, | ||
| createNativeStackNavigator, | ||
| } from '@react-navigation/native-stack'; | ||
| import React, { useEffect, useLayoutEffect, useState } from 'react'; | ||
| import { createBottomTabNavigator } from '@react-navigation/bottom-tabs'; | ||
| import { Square } from '../shared'; | ||
|
|
||
| type FirstStackParamList = { | ||
| Home: undefined; | ||
| Details: undefined; | ||
| }; | ||
|
|
||
| type SecondStackParamList = { | ||
| Settings: undefined; | ||
| Info: undefined; | ||
| }; | ||
|
|
||
| const HomeScreen = ({ | ||
| navigation, | ||
| }: NativeStackScreenProps<FirstStackParamList>) => { | ||
|
|
||
| useLayoutEffect(() => { | ||
| // Set initial title | ||
| navigation.setOptions({ | ||
| title: 'Initial Title', | ||
| }); | ||
|
|
||
| // Set headerLeft and headerRight after 2 seconds | ||
| const timer1 = setTimeout(() => { | ||
| navigation.setOptions({ | ||
| headerLeft: () => <Square size={16} color="goldenrod" />, | ||
| headerRight: () => <Square size={20} color="green" />, | ||
| }); | ||
| }, 3000); | ||
|
|
||
| // Clean up timers | ||
| return () => { | ||
| clearTimeout(timer1); | ||
| }; | ||
| }, [navigation]); | ||
| return ( | ||
| <View style={styles.container}> | ||
| <Button | ||
| title={'Go to details'} | ||
| onPress={() => navigation.navigate('Details')} | ||
| /> | ||
| </View> | ||
| ); | ||
| }; | ||
|
|
||
| const DetailsScreen = ({ | ||
| navigation, | ||
| }: NativeStackScreenProps<FirstStackParamList>) => { | ||
| 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 InfoScreen = ({ | ||
| navigation, | ||
| }: NativeStackScreenProps<SecondStackParamList>) => { | ||
| 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 square2 = (props: { tintColor?: string }) => ( | ||
| <Square {...props} color="red" size={20} /> | ||
| ); | ||
|
|
||
| useLayoutEffect(() => { | ||
| navigation.setOptions({ | ||
| headerRight: square1, | ||
| headerTitle: undefined, | ||
| headerLeft: hasLeftItem ? square2 : undefined, | ||
| }); | ||
| }, [navigation, hasLeftItem]); | ||
|
|
||
| return ( | ||
| <View style={styles.container}> | ||
| <Button | ||
| title="Toggle subviews" | ||
| onPress={() => setHasLeftItem(prev => !prev)} | ||
| /> | ||
| </View> | ||
| ); | ||
| }; | ||
|
|
||
| const SettingsScreen = ({ | ||
| navigation, | ||
| }: NativeStackScreenProps<SecondStackParamList>) => { | ||
| return ( | ||
| <View style={styles.container}> | ||
| <Button | ||
| title={'Go to Info'} | ||
| onPress={() => navigation.navigate('Info')} | ||
| /> | ||
| </View> | ||
| ); | ||
| }; | ||
|
|
||
| const FirstStack = createNativeStackNavigator<FirstStackParamList>(); | ||
|
|
||
| const FirstStackNavigator = () => { | ||
| return ( | ||
| <FirstStack.Navigator> | ||
| <FirstStack.Screen | ||
| name="Home" | ||
| component={HomeScreen} | ||
| /> | ||
| <FirstStack.Screen name="Details" component={DetailsScreen} /> | ||
| </FirstStack.Navigator> | ||
| ); | ||
| }; | ||
|
|
||
| const SecondStack = createNativeStackNavigator<SecondStackParamList>(); | ||
|
|
||
| const SecondStackNavigator = () => { | ||
| return ( | ||
| <SecondStack.Navigator> | ||
| <SecondStack.Screen | ||
| name="Settings" | ||
| component={SettingsScreen} | ||
| options={{ | ||
| headerLeft: () => <Square size={16} />, | ||
| }} | ||
| /> | ||
| <SecondStack.Screen name="Info" component={InfoScreen} /> | ||
| </SecondStack.Navigator> | ||
| ); | ||
| }; | ||
|
|
||
| const Tabs = createBottomTabNavigator(); | ||
|
|
||
|
|
||
|
|
||
| export function BottomTabNavigator() { | ||
| return ( | ||
| <Tabs.Navigator screenOptions={{ headerShown: false }}> | ||
| <Tabs.Screen name="First" component={FirstStackNavigator} /> | ||
| <Tabs.Screen name="Second" component={SecondStackNavigator} /> | ||
| </Tabs.Navigator> | ||
| ); | ||
| } | ||
|
|
||
| const RootStack = createNativeStackNavigator(); | ||
|
|
||
| export default function App() { | ||
| return ( | ||
| <NavigationContainer> | ||
| <RootStack.Navigator> | ||
| <RootStack.Screen | ||
| name="Root" | ||
| component={BottomTabNavigator} | ||
| options={{ headerShown: false }} | ||
| /> | ||
| </RootStack.Navigator> | ||
| </NavigationContainer> | ||
| ); | ||
| } | ||
|
|
||
| const styles = StyleSheet.create({ | ||
| container: { | ||
| flex: 1, | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| }, | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters