Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: clicking on Pressable located in screen header #2466

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

coado
Copy link

@coado coado commented Oct 29, 2024

Description

This PR fixes clicking on Pressables that are added to the native header. Previously, Yoga had incorrect information about the location of the content in the header.

The first step was to remove top: "-100%" style from the ScreenStackHeaderConfig which made Yoga think, that the content is pushed up by the size of its parent (Screen size).

The second step involved updating layoutMetrics of the RNSScreenStackHeaderConfigShadowNode. The entire app content is pushed down by the size of the header, so it also has an impact on the header config in Yoga metrics. To mitigate this, the origin of RNSScreenStackHeaderConfigShadowNode is decreased by the size of the header which will zero out eventually leaving the header content in the desired position. On iOS this position is actually moved by the top inset size, so we also have to take it into account when setting header config layout metrics.

Fixes #2219

Changes

Updated ScreenShadowNode to decrease origin.y of the HeaderConfigShadowNode by the size of the header. Added paddingTop to HeaderConfigState and set it as origin offset on iOS.

Screenshots / GIFs

Before

iOS Android
ios-before.mov
android-before.mov

After

iOs Android
ios-after.mov
android-after.mov

Test code and steps to reproduce

Tested on this example:

code
import { NavigationContainer } from "@react-navigation/native";
import { createNativeStackNavigator, NativeStackNavigationProp } from "@react-navigation/native-stack";
import React, { ForwardedRef, forwardRef } from "react";
import { findNodeHandle, Pressable, PressableProps, StyleSheet, Text, View, } from "react-native";

type StackParamList = {
  Home: undefined,
}

type RouteProps = {
  navigation: NativeStackNavigationProp<StackParamList>;
}

type PressableState = 'pressed-in' | 'pressed' | 'pressed-out'


const Stack = createNativeStackNavigator<StackParamList>();


const PressableWithFeedback = forwardRef((props: PressableProps, ref: ForwardedRef<View>): React.JSX.Element => {
  const [pressedState, setPressedState] = React.useState<PressableState>('pressed-out');

  const onPressInCallback = React.useCallback((e) => {
    console.log('Pressable onPressIn', {
      locationX: e.nativeEvent.locationX,
      locationY: e.nativeEvent.locationY,
      pageX: e.nativeEvent.pageX,
      pageY: e.nativeEvent.pageY,
    });
    setPressedState('pressed-in');
    props.onPressIn?.();
  }, []);

  const onPressCallback = React.useCallback(() => {
    console.log('Pressable onPress');
    setPressedState('pressed');
  }, []);

  const onPressOutCallback = React.useCallback(() => {
    console.log('Pressable onPressOut');
    setPressedState('pressed-out');
  }, []);

  const onResponderMoveCallback = React.useCallback(() => {
    console.log('Pressable onResponderMove');
  }, []);

  const contentsStyle = pressedState === 'pressed-out'
    ? styles.pressablePressedOut
    : (pressedState === 'pressed'
      ? styles.pressablePressed
      : styles.pressablePressedIn);

  return (
    <View ref={ref} style={[contentsStyle, { width: "100%"}]}>
      <Pressable
        onPressIn={onPressInCallback}
        onPress={onPressCallback}
        onPressOut={onPressOutCallback}
        onResponderMove={onResponderMoveCallback}
      >
        {props.children}
      </Pressable>
    </View>

  );
})

function HeaderTitle(): React.JSX.Element {

  return (
    <PressableWithFeedback
      onPressIn={() => {
        console.log('Pressable onPressIn')
      }}
      onPress={() => console.log('Pressable onPress')}
      onPressOut={() => console.log('Pressable onPressOut')}
      onResponderMove={() => console.log('Pressable onResponderMove')}
      ref={ref => {
        console.log(findNodeHandle(ref));
        ref?.measure((x, y, width, height, pageX, pageY) => {
          console.log('header component measure', { x, y, width, height, pageX, pageY });
        })
      }}
    >
      <View style={{ height: 40, justifyContent: 'center', alignItems: 'center' }}>
        <Text style={{ alignItems: 'center' }}>Regular Pressable</Text>
      </View>
    </PressableWithFeedback>
  )
}

function Home(_: RouteProps): React.JSX.Element {
  return (
    <View style={{ flex: 1, backgroundColor: 'rgba(0, 0, 0, .8)' }}
    >
      <View style={{ flex: 1, alignItems: 'center', marginTop: 48 }}>
        <PressableWithFeedback
          onPressIn={() => console.log('Pressable onPressIn')}
          onPress={() => console.log('Pressable onPress')}
          onPressOut={() => console.log('Pressable onPressOut')}
        >
          <View style={{ height: 40, width: 200, justifyContent: 'center', alignItems: 'center' }}>
            <Text style={{ alignItems: 'center' }}>Regular Pressable</Text>
          </View>
        </PressableWithFeedback>
      </View>
    </View>
  );
}

function App(): React.JSX.Element {
  return (
    <NavigationContainer>
      <Stack.Navigator>
        <Stack.Screen
          name="Home"
          component={Home}
          options={{
            headerTitle: HeaderTitle,
          }}
        />
      </Stack.Navigator>
    </NavigationContainer>
  );
}

const styles = StyleSheet.create({
  pressablePressedIn: {
    backgroundColor: 'lightsalmon',
  },
  pressablePressed: {
    backgroundColor: 'crimson',
  },
  pressablePressedOut: {
    backgroundColor: 'lightseagreen',
  }
});


export default App;

@alduzy
Copy link
Member

alduzy commented Oct 30, 2024

Hi @coado I like this approach. However, I'm afraid in current form it may not respect the placement of the elements in regards to flex layout of the header. Have you tested it with smaller elements, headerLeft and/or headerRight for example?

You should be able to use the Element Inspector from the dev menu to inspect the actual placement of the pressables laid out by yoga. I remember using it in #2292

@kkafar kkafar self-requested a review October 30, 2024 11:51
@coado
Copy link
Author

coado commented Oct 31, 2024

Hi @coado I like this approach. However, I'm afraid in current form it may not respect the placement of the elements in regards to flex layout of the header. Have you tested it with smaller elements, headerLeft and/or headerRight for example?

You should be able to use the Element Inspector from the dev menu to inspect the actual placement of the pressables laid out by yoga. I remember using it in #2292

Hey @alduzy, thanks for the reply! This is how it looks when I set Pressable on headerRight only or on both:

right both

Please let me know if this is desired behaviour.

Also I've checked a placement using inspector as you proposed and it seems like Pressable boundary in Yoga is not perfectly aligned with what is displayed.

with-inspector

This is something that I will have a closer look at!

code
import { NavigationContainer } from "@react-navigation/native";
import { createNativeStackNavigator, NativeStackNavigationProp } from "@react-navigation/native-stack";
import React, { ForwardedRef, forwardRef } from "react";
import { findNodeHandle, Pressable, PressableProps, StyleSheet, Text, View, Button } from "react-native";

type StackParamList = {
  Home: undefined,
}

type RouteProps = {
  navigation: NativeStackNavigationProp<StackParamList>;
}

type PressableState = 'pressed-in' | 'pressed' | 'pressed-out'


const Stack = createNativeStackNavigator<StackParamList>();


const PressableWithFeedback = forwardRef((props: PressableProps, ref: ForwardedRef<View>): React.JSX.Element => {
  const [pressedState, setPressedState] = React.useState<PressableState>('pressed-out');

  const onPressInCallback = React.useCallback((e) => {
    console.log('Pressable onPressIn', {
      locationX: e.nativeEvent.locationX,
      locationY: e.nativeEvent.locationY,
      pageX: e.nativeEvent.pageX,
      pageY: e.nativeEvent.pageY,
    });
    setPressedState('pressed-in');
    props.onPressIn?.();
  }, []);

  const onPressCallback = React.useCallback(() => {
    console.log('Pressable onPress');
    setPressedState('pressed');
  }, []);

  const onPressOutCallback = React.useCallback(() => {
    console.log('Pressable onPressOut');
    setPressedState('pressed-out');
  }, []);

  const onResponderMoveCallback = React.useCallback(() => {
    console.log('Pressable onResponderMove');
  }, []);

  const contentsStyle = pressedState === 'pressed-out'
    ? styles.pressablePressedOut
    : (pressedState === 'pressed'
      ? styles.pressablePressed
      : styles.pressablePressedIn);

  return (
    <View ref={ref} style={[contentsStyle]}>
      <Pressable
        onPressIn={onPressInCallback}
        onPress={onPressCallback}
        onPressOut={onPressOutCallback}
        onResponderMove={onResponderMoveCallback}
      >
        {props.children}
      </Pressable>
    </View>

  );
})

function HeaderTitle(): React.JSX.Element {

  return (
    <PressableWithFeedback
      onPressIn={() => {
        console.log('Pressable onPressIn')
      }}
      onPress={() => console.log('Pressable onPress')}
      onPressOut={() => console.log('Pressable onPressOut')}
      onResponderMove={() => console.log('Pressable onResponderMove')}
      ref={ref => {
        console.log(findNodeHandle(ref));
        ref?.measure((x, y, width, height, pageX, pageY) => {
          console.log('header component measure', { x, y, width, height, pageX, pageY });
        })
      }}
    >
      <View style={{ height: 40, justifyContent: 'center', alignItems: 'center' }}>
        <Text style={{ alignItems: 'center' }}>Regular Pressable</Text>
      </View>
    </PressableWithFeedback>
  )
}

function Home(_: RouteProps): React.JSX.Element {
  return (
    <View style={{ flex: 1, backgroundColor: 'rgba(0, 0, 0, .8)' }}
    >
      <View style={{ flex: 1, alignItems: 'center', marginTop: 48 }}>
        <PressableWithFeedback
          onPressIn={() => console.log('Pressable onPressIn')}
          onPress={() => console.log('Pressable onPress')}
          onPressOut={() => console.log('Pressable onPressOut')}
        >
          <View style={{ height: 40, width: 200, justifyContent: 'center', alignItems: 'center' }}>
            <Text style={{ alignItems: 'center' }}>Regular Pressable</Text>
          </View>
        </PressableWithFeedback>
      </View>
    </View>
  );
}

function App(): React.JSX.Element {
  return (
    <NavigationContainer>
      <Stack.Navigator>
        <Stack.Screen
          name="Home"
          component={Home}
          options={{
            // headerTitle: HeaderTitle,
            headerRight: HeaderTitle,
            // headerLeft: HeaderTitle
          }}
        />
      </Stack.Navigator>
    </NavigationContainer>
  );
}

const styles = StyleSheet.create({
  pressablePressedIn: {
    backgroundColor: 'lightsalmon',
  },
  pressablePressed: {
    backgroundColor: 'crimson',
  },
  pressablePressedOut: {
    backgroundColor: 'lightseagreen',
  }
});


export default App;

@coado
Copy link
Author

coado commented Oct 31, 2024

Actually, when the Pressable is set as the headerRight only it doesn't work 🤔

@jiroscripts
Copy link

Hello @coado

Any news on this issue?

I started upgrading my project to RN 0.76 and this issue is by far the most problematic

@coado
Copy link
Author

coado commented Nov 13, 2024

Hey @thibaultcapelli
Sorry for the delay. I will get back to this issue shortly.

@kkafar
Copy link
Member

kkafar commented Nov 13, 2024

I was just looking through the code and determined that the only reliable solution would be to update position of header elements in ShadowTree (ST) based on their position in HostTree (HT), i. e. you do send additional information on topinset now - maybe let's send whole frame instead and update headersubviews layout metrics in shadow node? I think this is only way to get this at least partially consistent.

@coado coado marked this pull request as draft November 14, 2024 14:31
@acrabb
Copy link

acrabb commented Nov 18, 2024

+1 for this issue 🙏

@coado coado marked this pull request as ready for review November 28, 2024 13:14
@jiroscripts
Copy link

Please, merge this

@jiroscripts
Copy link

@kkafar What are we waiting for ?

I cannot understand there is new release candidates and this several issue is just waiting for weeks.

Native stack looks actually the way to go but there is no way to use it with a fresh RN project if you use a pressable in header which is a pretty common thing.

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.

Pressable elements not working in native-stack on Android devices with new architecture
5 participants