Skip to content

Commit

Permalink
fix(iOS, Fabric): wrong headerCenter layout after native dismissal (#…
Browse files Browse the repository at this point in the history
…2263)

## Description

This PR fixes #2232 by disabling view recycling in
`RNSScreenStackHeaderSubview` view component, as no better solution was
found.

The following sequence leads to the bug:

1. User clicks back button
2. The header subviews are properly removed and put in recycle pool in
the same order they were mounted (recycle pool is a stack).
3. User pushes again just-popped-screen.
4. React Native reuses view from recycle pool, but `headerCenter` is to
be mounted first, so RN reuses the view that was previously
`headerRight` (because it is on top of the recycle pool stack).
Similarly the second view in recycle pool, which was previously
`headerCenter` is now repurposed as `headerRight`.
5. RN sets correct frames for both subviews.
6. System sets wrong frame for the new `headerCenter` due to layout
triggered by `setNavigationBarHidden` in `updateViewController` of
`RNSScreenStackHeaderConfig`. The `setNavigationBarHidden` is called,
because there is difference in value of the prop between the screens
(see issue for repro). The only fact that matters is the one, that
native layout is triggered & it computes the `width` incorrectly <- it
looks like UIKit does hold some cache for layout computations & when
seeing the recycled view it just bonks it with it's previous frame, for
some reason not observing that deeper in view hierarchy contents have
changed.

I've tried to invalidate layout of the subview in various places that
could make sense, e.g. `prepareForRecycle`, just before computing native
layout in `updateViewController`, etc. for no result.

To avoid putting more debugging hours into this particular issue I went
with disabling view recycling for header subviews, originally suggested
by @janicduplessis in the issue description.


### Open questions

We would want to answer those at some time in the future:

1. [ ] What's the difference between JS-initiated dismissal and native
one, in this very case? Where does the difference in behaviour come
from? (JS-initiated works properly)
2. [ ] In what other way can we solve this w/o disabling view recycling?

Fixes #2232 

## Changes

Disable view recycling in `RNSScreenStackHeaderSubview` view component.

## Test code and steps to reproduce

`Test2232`

## Checklist

- [x] Included code example that can be used to test this change
- [x] Ensured that CI passes
  • Loading branch information
kkafar authored Jul 29, 2024
1 parent 68dabd9 commit c5a36b3
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 0 deletions.
55 changes: 55 additions & 0 deletions apps/src/tests/Test2232.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { createNativeStackNavigator } from '@react-navigation/native-stack';
import React from 'react';
import { ScrollView, View } from 'react-native';

import { Button, Square } from '../shared';
import { NavigationContainer } from '@react-navigation/native';

const MainScreen = ({ navigation }: any) => (
<ScrollView style={{ marginTop: 100 }}>
<Button
onPress={() => navigation.navigate('Settings')}
title="Go to next screen"
/>
</ScrollView>
);

const SettingsScreen = ({ navigation }: any) => (
<Button title="Go back" onPress={() => navigation.goBack()} />
);

const Stack = createNativeStackNavigator();

const App = () => (
<NavigationContainer>
<Stack.Navigator>
<Stack.Screen
name="Main"
options={{
headerShown: false,
}}
component={MainScreen}
/>
<Stack.Screen
name="Settings"
component={SettingsScreen}
options={{
headerTintColor: 'hotpink',
headerBackTitleVisible: false,
headerTitle: () => (
<View style={{ gap: 16, flexDirection: 'row' }}>
<Square color="green" size={20} />
<Square color="green" size={20} />
<Square color="green" size={20} />
<Square color="green" size={20} />
</View>
),
headerRight: () => <Square color="red" size={20} />,
}}
/>
</Stack.Navigator>
</NavigationContainer>
);

export default App;

1 change: 1 addition & 0 deletions apps/src/tests/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ 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 TestScreenAnimation } from './TestScreenAnimation';
export { default as TestHeader } from './TestHeader';
5 changes: 5 additions & 0 deletions ios/RNSScreenStackHeaderSubview.mm
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ - (void)updateLayoutMetrics:(const react::LayoutMetrics &)layoutMetrics
}
}

+ (BOOL)shouldBeRecycled
{
return NO;
}

#else
#pragma mark - Paper specific

Expand Down

0 comments on commit c5a36b3

Please sign in to comment.