Skip to content

fix(Android): don't remount components during navigation on the new architecture#11806

Merged
satya164 merged 3 commits intoreact-navigation:mainfrom
j-piasecki:patch-1
Feb 7, 2024
Merged

fix(Android): don't remount components during navigation on the new architecture#11806
satya164 merged 3 commits intoreact-navigation:mainfrom
j-piasecki:patch-1

Conversation

@j-piasecki
Copy link
Copy Markdown
Contributor

Motivation

This PR prevents the native view hierarchy built by Stack from re-mounting to prevent blurring focus - when a view is removed from the hierarchy it's blurred on Android, but it's not focused back when added again. Due to one of the views in the Card component being flattened, children of Card were removed from the hierarchy and attached under a different parent.

Before the change
Screen.Recording.2024-02-01.at.11.45.27.mov

Native hierarchy on the first screen:
Screenshot 2024-02-01 at 11 46 27

Native hierarchy on the second screen:
Screenshot 2024-02-01 at 11 46 46

Notice the different nesting levels between the two - one of the views got collapsed during navigation.

After the change
Screen.Recording.2024-02-01.at.11.45.45.mov

Native hierarchy on the first screen:
Screenshot 2024-02-01 at 11 47 29

Native hierarchy on the second screen:
Screenshot 2024-02-01 at 11 47 36

Notice the same nesting levels between the two.

Test plan

I've tested the change on the following code
import 'react-native-gesture-handler';
import {NavigationContainer, useFocusEffect} from '@react-navigation/native';
import React, {useRef} from 'react';
import {createStackNavigator} from '@react-navigation/stack';
import {Button, TextInput, View} from 'react-native';

const Stack = createStackNavigator();

function Thing({navigation}: any) {
  const ref = useRef(null);

  useFocusEffect(() => {
    setTimeout(() => {
      ref.current?.focus();
    }, 300);
  });

  return (
    <View style={{flex: 1, backgroundColor: 'wheat'}}>
      <TextInput
        ref={ref}
        style={{backgroundColor: 'white', borderWidth: 1, borderColor: 'black'}}
      />
      <Button
        title="navigate"
        onPress={() => {
          navigation.navigate('Second');
        }}
      />
    </View>
  );
}

function Second() {
  return <View style={{flex: 1, backgroundColor: 'red'}} />;
}

function App() {
  return (
    <NavigationContainer>
      <Stack.Navigator
        screenOptions={{headerShown: false}}
        detachInactiveScreens={false}>
        <Stack.Screen name="Home" component={Thing} />
        <Stack.Screen name="Second" component={Second} />
      </Stack.Navigator>
    </NavigationContainer>
  );
}

export default App;

@netlify
Copy link
Copy Markdown

netlify bot commented Feb 1, 2024

Deploy Preview for react-navigation-example ready!

Name Link
🔨 Latest commit fe9c11b
🔍 Latest deploy log https://app.netlify.com/sites/react-navigation-example/deploys/65c349852f95f80008ed33c4
😎 Deploy Preview https://deploy-preview-11806--react-navigation-example.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@satya164
Copy link
Copy Markdown
Member

satya164 commented Feb 1, 2024

Thanks for the PR. Isn't this a bug in React Native if the view nesting level changes like this? Or is this specific interaction with view flattening and react-native-screens?

Also can you add a comment before the line in the code with the details?

@j-piasecki
Copy link
Copy Markdown
Contributor Author

It's caused by this line:

overflow: active ? undefined : 'hidden',

I guess when overflow gets changed to hidden the criteria for collapsing the view are met, so it's removed.

It's reproducible without react-native-screens, the videos and the testing snipped from the PR are all with detachInactiveScreens={false} set on the navigator.

Also can you add a comment before the line in the code with the details?

I've added a high-level explanation, is it good enough or should I go more in-depth?

@satya164
Copy link
Copy Markdown
Member

satya164 commented Feb 7, 2024

I have updated the comment. Will merge the workaround for now, tho I think it'd be good to open this issue in React Native repo as imo view flattening should be only an optimization and not observable like this, and this doesn't happen in the old architecture.

@satya164 satya164 merged commit aa518f1 into react-navigation:main Feb 7, 2024
satya164 added a commit that referenced this pull request Feb 8, 2024
…re (#11807)

The same as
#11806 but
targeting 6.x branch

---------

Co-authored-by: Satyajit Sahoo <satyajit.happy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants