-
-
Notifications
You must be signed in to change notification settings - Fork 512
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(Android,Fabric): pressable on
Screen loses focus on pointer mov…
…ement (#2292) ## Description > [!important] This PR aims to fix only pressables on screen components. This PR does not fix similar pressable issue with pressables in native header. That interaction will be fixed separately. Pressable elements work just fine until there's a gesture involved. On sensitive physical devices even a little movement during the press is treated as a gesture. When the `Pressable` element detects a gesture it calls [onResponderMove](https://github.com/facebook/react-native/blob/82795715aefba07ae9d79278ce3fd4d2e9a928f2/packages/react-native/Libraries/Pressability/Pressability.js#L484) which then checks wether the gesture happened within the element or went outside by comparing the touch coordinates with coordinates of the element using `_isTouchWithinResponderRegion`. The `responderRegion` is obtained from `_responderID` and happens to have unexpected values when the native header is present. It tuns out that the Y origin is slightly off. After some further investigation and comparison of coordinates it turned out that the height of the android status bar is not well calculated in various scenarios: <table> <td> `statusBarHidden: true` </td> <td> `statusBarTranslucent: true` </td> <td> `statusBarTranslucent: false` </td> </tr> <tr> <td>  </td> <td>  </td> <td>  </td> </tr> </table> The `calculateHeaderHeight` used for calculating the header and statusBar height seems to be the problem. Luckily, we don't have to calculate it by ourselves anymore, because the correct `t` value is provided in the `onLayout` function of the `Screen`. Thus we can get rid of the custom function. Another issue found: after navigating to another screen the offset is off again (exactly by 2x). It's caused by changes introduced in [this PR](#2169), which was supposed to prevent content jumps, but doesn't work since RN `0.75` sadly.  I found out that `FrameOriginCorrection` is not being unset when dimensions from JVM are received, while the `FrameHeightCorrection` is. After adding the missing unset for `FrameOriginCorrection` I rolled back to the commit with the mentioned PR merged and RN `0.74` and I can confirm it works. Fixes #1975 ## Changes - removed `calculateHeaderHeight` function - added unset for `FrameOriginCorrection` when dimensions from JVM are received - added `Test1975.tsx` repro - moved code responsible for determining header height during the very first render from component descriptor's `adopt` method to shadow node `appendChild`. ## Test code and steps to reproduce `TestHeader`, `Test1975` ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes --------- Co-authored-by: alduzy <alduzy@gmail.com> Co-authored-by: Alex Duży <91994767+alduzy@users.noreply.github.com>
- Loading branch information
3 people
authored
Aug 27, 2024
1 parent
80dcad4
commit 34c1ba8
Showing
9 changed files
with
228 additions
and
138 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
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
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,109 @@ | ||
| import * as React from 'react'; | ||
| import { NavigationContainer } from '@react-navigation/native'; | ||
| import { View, Text, StyleSheet, Pressable } from 'react-native'; | ||
| import { createNativeStackNavigator } from '@react-navigation/native-stack'; | ||
|
|
||
| const Stack = createNativeStackNavigator(); | ||
|
|
||
| function App() { | ||
| const [count, setCount] = React.useState(0); | ||
|
|
||
| return ( | ||
| <NavigationContainer> | ||
| <Stack.Navigator screenOptions={{ statusBarTranslucent: false }}> | ||
| <Stack.Screen | ||
| name="Screen" | ||
| component={Screen} | ||
| options={{ | ||
| headerRight: () => ( | ||
| <Pressable | ||
| onPress={() => setCount(prev => prev + 1)} | ||
| style={({ pressed }) => [ | ||
| styles.pressable, | ||
| pressed && { backgroundColor: 'goldenrod' }, | ||
| ]}> | ||
| <Text>Press (+)</Text> | ||
| </Pressable> | ||
| ), | ||
| title: count.toString(), | ||
| headerTitleAlign: 'center', | ||
| }} | ||
| /> | ||
| <Stack.Screen | ||
| name="Details" | ||
| component={DetailsScreen} | ||
| options={({ navigation }) => ({ | ||
| title: 'Details', | ||
| headerRight: () => ( | ||
| <Pressable | ||
| onPress={navigation.goBack} | ||
| style={({ pressed }) => [ | ||
| styles.pressable, | ||
| pressed && { backgroundColor: 'goldenrod' }, | ||
| ]}> | ||
| <Text>Go Back</Text> | ||
| </Pressable> | ||
| ), | ||
| })} | ||
| /> | ||
| </Stack.Navigator> | ||
| </NavigationContainer> | ||
| ); | ||
| } | ||
|
|
||
| function Screen({ navigation }: any) { | ||
| return ( | ||
| <View style={styles.container}> | ||
| <Pressable | ||
| onPress={() => navigation.navigate('Details')} | ||
| style={({ pressed }) => [ | ||
| styles.pressable, | ||
| pressed && { backgroundColor: 'goldenrod' }, | ||
| ]}> | ||
| <Text>Go to Details</Text> | ||
| </Pressable> | ||
| </View> | ||
| ); | ||
| } | ||
|
|
||
| function DetailsScreen() { | ||
| let counter = React.useRef(0); | ||
|
|
||
| return ( | ||
| <View style={{ ...styles.container, backgroundColor: 'beige' }}> | ||
| <Pressable | ||
| onPressIn={() => { | ||
| counter.current += 1; | ||
| console.log(`[${counter.current}] Details: onPressIn`); | ||
| }} | ||
| onPress={() => { | ||
| console.log(`[${counter.current}] Details: onPress`); | ||
| }} | ||
| onPressOut={() => { | ||
| console.log(`[${counter.current}] Details: onPressOut`); | ||
| }} | ||
| style={({ pressed }) => [ | ||
| styles.pressable, | ||
| pressed && { backgroundColor: 'goldenrod' }, | ||
| ]}> | ||
| <Text>Press me</Text> | ||
| </Pressable> | ||
| </View> | ||
| ); | ||
| } | ||
|
|
||
| const styles = StyleSheet.create({ | ||
| pressable: { | ||
| paddingVertical: 5, | ||
| paddingHorizontal: 10, | ||
| backgroundColor: 'red', | ||
| }, | ||
| container: { | ||
| flex: 1, | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| gap: 24, | ||
| }, | ||
| }); | ||
|
|
||
| export default App; |
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
Oops, something went wrong.
34c1ba8There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please release this changes