-
-
Notifications
You must be signed in to change notification settings - Fork 545
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
Conversation
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
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. This is something that I will have a closer look at! codeimport { 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; |
Actually, when the Pressable is set as the |
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 |
Hey @thibaultcapelli |
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. |
+1 for this issue 🙏 |
Okay, so the |
@kkafar no still using the 4.5.0. Tomorrow I'll try the PR and give feedback if needed. |
@wal3d122 PR hash should be sufficient. However I recently received some reports that when installing from hash the |
@kkafar On the debug version of the android, the behaviour is similar to what’s shown on the iOS device video below. Though on the release version, all seems to be responsive except for deeply nested pages where the button is responsive only when I aim to click it at the top half, when I press the bottom half, it doesn’t respond at all. RPReplay_Final1737015042.mov |
@wal3d122 thanks for testing this! I'm trying now to reproduce the behaviour you showcased on the video but I fail to. On my end the pressables now work reliably - moreover, their reliability was never the problem for me, but rather they could not be pressed at all (only onPressIn would be dispatched). This PR adds |
I moved the state update logic from screen stack to config & subviews. I've also made HeaderConfig implement `RCTMountingTransactionObserving` protocol where it now requests layout if necessary. I've also discovered a bug where after removal of one of header subviews the layout is not reset to the same one as it would be when rendered w/o the removed subview in the first place.
I think this PR is ready to roll. I'll release beta / rc version today's evening or tomorrow forenoon. |
@kkafar tested Test2466 on both iOS and Android. Seem to work perfectly. I'll update our header and test again with a pressable instead and see if that behaviour still persists. |
@wal3d122 thanks for the info! I'll test |
I've released 4.6.0-beta.0 |
…g header subviews (#2623) ## Description In #2466 I've introduced `RCTMountingTransactionObserving` for `RNSScreenStackHeaderConfig` component. Now we can use this to reduce amount of calls to `updateViewControllerIfNeeded` (causing layout passes) when adding subviews. I'm not changing the `unmount` path of the code as we have some more additional logic there which requires some more careful consideration. This also aligns us with Paper implementation. Note that it has been only implemented this way, because at the implementation time there was no way to run this code on transaction completion. ## Changes `- [RNSScreenStackHeaderConfig updateViewControllerIfNeeded]` is now called only once per transaction when adding subviews to header config. ## Test code and steps to reproduce Test432, TestHeaderTitle, Test2552 - see that there are no regressions. > [!note] During testing I've found that there is a bug with subview layout when modifying subviews after first render. This is not a regression however. Notice the wrong position of header right on video below 👇🏻 (Test432) https://github.com/user-attachments/assets/7f8653e8-a7d9-4fb8-a875-182e7deb0495 ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
@kkafar Thank you very much for the effort! Seems like header right still does not work on iPhone XS. I am using TouchableOpacity and 4.6.0-beta.0 |
@JcbPrn thanks for the report, I'll look into this. If you had capacity to provide me with snack/repo/snippet where I can reproduce the issue directly, that would be awesome. |
Now, while the changes with 4.6.0. seem to have fixed the pressable issue, I am discovering a new issue, when setting the header right button dynamically from within a component: useFocusEffect(
React.useCallback(() => {
const stackNav = navigation.getParent();
stackNav?.setOptions({
title: ScreenName,
headerRight: () => (
<View
style={{
width: 20,
height: 20,
display: 'none',
}}></View>
... When using any kind of sized view, it works as expected on initial render, but after navigating (in my case to a different tab) and back, the header ~tripples in height, regardless of what the headerRight element is, or how its sized. What could help identify the issue is, when setting the headerRight element to a view with display: none (see snippet) the header increases in height slowly, without stopping as far as i can see. I assume this has to do with some of the size/position refactoring i saw mentioned for this issue. Using: Android, react-native 0.77.0, "react-native-screens": "^4.6.0", "@react-navigation/native": "^7.0.14", "@react-navigation/native-stack": "^7.2.0", |
Thanks @mikeswann for the detailed report. I'll look into it. |
I'm on v4.6.0 and found an issue on Android only with I was able to pin down the issue caused by this diff This is what it looks like with the current v4.6.0 version from the layout inspector: ![]() Clicking on the top half of the card doesn't register because of it. Happy to provide more details in a separate issue if needed, but at first glance the cause is clear. |
Hey @robertying, I've tried to reproduce your setup and tested on both architectures (I see you're using Paper in the screenshot, though) but I do not confirm the issue. Here's the snippet I've tested on: Snippetimport { NavigationContainer, RouteProp } from '@react-navigation/native';
import { createNativeStackNavigator, NativeStackNavigationProp } from '@react-navigation/native-stack';
import React from 'react';
import { findNodeHandle, Text, View } from 'react-native';
import PressableWithFeedback from '../shared/PressableWithFeedback';
import { createBottomTabNavigator } from '@react-navigation/bottom-tabs';
type StackParamList = {
Home: { marginTop?: number },
NestedTabsHost: undefined;
}
type RouteProps = {
navigation: NativeStackNavigationProp<StackParamList>;
route: RouteProp<StackParamList>;
}
const Stack = createNativeStackNavigator<StackParamList>();
const Tabs = createBottomTabNavigator();
const NestedStack = createNativeStackNavigator();
function NestedStackHost() {
return (
<NestedStack.Navigator>
<NestedStack.Screen name="NestedHome" component={Home} initialParams={{ marginTop: 0 }} />
</NestedStack.Navigator>
);
}
function NestedTabsHost() {
return (
<Tabs.Navigator>
<Tabs.Screen name="NestedStackHost" component={NestedStackHost} options={{ headerShown: false }} />
</Tabs.Navigator>
);
}
function HeaderTitle(): React.JSX.Element {
return (
<PressableWithFeedback
onLayout={event => {
const { x, y, width, height } = event.nativeEvent.layout;
console.log('Title onLayout', { x, y, width, height });
}}
onPressIn={() => {
console.log('Pressable onPressIn');
}}
onPress={() => console.log('Pressable onPress')}
onPressOut={() => console.log('Pressable onPressOut')}
onResponderMove={() => console.log('Pressable onResponderMove')}
ref={node => {
console.log(findNodeHandle(node));
node?.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 HeaderLeft(): React.JSX.Element {
return (
<HeaderTitle />
);
}
function Home({ navigation, route }: RouteProps): React.JSX.Element {
return (
<View style={{ flex: 1, backgroundColor: 'rgba(0, 0, 0, .8)' }}
>
<View style={{ flex: 1, alignItems: 'center', marginTop: route.params?.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>
<PressableWithFeedback onPress={() => navigation.navigate('NestedTabsHost')}>
<View style={{ height: 40, width: 200, justifyContent: 'center', alignItems: 'center' }}>
<Text style={{ alignItems: 'center' }}>Show tabs</Text>
</View>
</PressableWithFeedback>
</View>
</View>
);
}
function App(): React.JSX.Element {
return (
<NavigationContainer>
<Stack.Navigator>
<Stack.Screen
name="Home"
component={Home}
options={{
headerTitle: HeaderTitle,
headerLeft: HeaderLeft,
headerRight: HeaderLeft,
}}
/>
<Stack.Screen name="NestedTabsHost" component={NestedTabsHost} options={{ headerShown: false }} />
</Stack.Navigator>
</NavigationContainer>
);
}
export default App; If you could modify it in such way that I can reproduce this issue & let me know that would be great. Preferably open a new ticket, but here is also fine. |
@kkafar sorry for the late response. Can you try using this App.tsximport {createBottomTabNavigator} from '@react-navigation/bottom-tabs';
import {NavigationContainer} from '@react-navigation/native';
import {createNativeStackNavigator} from '@react-navigation/native-stack';
import {Text, View} from 'react-native';
const NoticeStackNavigator = createNativeStackNavigator();
const MainNavigator = createBottomTabNavigator();
const RootNavigator = createNativeStackNavigator();
const NoticeStack = () => (
<NoticeStackNavigator.Navigator>
<NoticeStackNavigator.Screen
name="Notices"
component={() => <View></View>}
options={{
headerTitle: () => <Text>Title</Text>,
}}
/>
</NoticeStackNavigator.Navigator>
);
const MainTab = () => (
<MainNavigator.Navigator>
<MainNavigator.Screen
name="NoticeStack"
component={NoticeStack}
options={{
title: 'Notices',
}}
/>
</MainNavigator.Navigator>
);
const App = () => {
return (
<NavigationContainer>
<RootNavigator.Navigator>
<RootNavigator.Screen name="MainTab" component={MainTab} />
</RootNavigator.Navigator>
</NavigationContainer>
);
};
export default App; ![]() |
I believe this PR might have introduced regressions. The ripple effect from my buttons inside of the header are now truncated ( Screen.Recording.2025-02-19.at.4.20.19.PM.mov |
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 theScreenStackHeaderConfig
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 theRNSScreenStackHeaderConfigShadowNode
. 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 ofRNSScreenStackHeaderConfigShadowNode
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 decreaseorigin.y
of theHeaderConfigShadowNode
by the size of the header. AddedpaddingTop
toHeaderConfigState
and set it as origin offset on iOS.Screenshots / GIFs
Before
ios-before.mov
android-before.mov
After
ios-after.mov
android-after.mov
Test code and steps to reproduce
Tested on this example:
code