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.

Sign up for GitHub

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

Jump to bottom

Taps often lost on Android real device + Fabric with header enabled #1975

Closed
franksmule opened this issue Nov 20, 2023 · 5 comments · Fixed by #2292
Closed

Taps often lost on Android real device + Fabric with header enabled #1975

franksmule opened this issue Nov 20, 2023 · 5 comments · Fixed by #2292
Assignees
@kkafar @alduzy
Labels
Platform: Android This issue is specific to Android Repro provided A reproduction with a snack or repo is provided

Comments

@franksmule
Copy link

Description

On a real android device, with fabric enabled, taps are often lost when Header is shown on Native Stack.

On an Android emulator, iOS and even using a tool such as scrcpy taps are always registered, using real device and manually tapping is very inconsistent. Have seen across multiple phones.

Steps to reproduce

  1. Checkout react-native-screens repo
  2. go to FabricExample folder
  3. Change App.js to code in linked snack repo.
  4. Run adb logcat | grep ReactNative
  5. Use on real android device - notice how logs do not print '1111' with every tap (inconsistent)
  6. Change headerShown to false logs WILL print with every tap

Snack or a link to a repository

https://snack.expo.dev/X06saJ53E

Screens version

main

React Native version

0.72.4

Platforms

Android

JavaScript runtime

Hermes

Workflow

React Native (without Expo)

Architecture

Fabric (New Architecture)

Build type

Debug mode

Device

Real device

Device model

Nokia TA-1234

Acknowledgements

Yes
@github-actions github-actions bot added Platform: Android This issue is specific to Android Repro provided A reproduction with a snack or repo is provided labels Nov 20, 2023
@franksmule
Copy link
Author

To add some further findings:

TextInput has the same issue - taps are sometimes ignored, the input does not get focus and no keyboard.

If I use react-native-gesture-handler RectButton, TextInput they always work. The react-native imports have the intermittent issues.

@dviluk
Copy link

dviluk commented Dec 25, 2023

Same for me, you need to tap on the top half of the pressable component to trigger the event. I switched to @react-navigation/stack for now.

@dviluk
Copy link

dviluk commented Jan 23, 2024

Does some know what is causing this?

@grgmo
Copy link

grgmo commented Jan 24, 2024

similar issue we're seeing with native stack on react native 0.73.2 only on Fabric Android real device where tap seems to get lost and no onPress callback on header component.

@yosukapro
Copy link

Any solution found for this?

@kkafar kkafar self-assigned this Apr 29, 2024
This was referenced Aug 7, 2024
Open
Open
hyochan referenced this issue in crossplatformkorea/CPK Aug 9, 2024
@hyochan
fix(adhoc): new-arch back button
01b01e4 
https://github.com/software-mansion/react-native-screens/issues/1975\#issuecomment-1819084348
@alduzy alduzy mentioned this issue Aug 9, 2024
Merged
2 tasks
@alduzy alduzy self-assigned this Aug 9, 2024
@hyochan hyochan mentioned this issue Aug 11, 2024
Merged
hyochan added a commit to crossplatformkorea/CPK that referenced this issue Aug 11, 2024
@hyochan
fix: new arch native stack (#10)
fbb914f 
Support new architecture.
software-mansion/react-native-screens#1975
@kkafar kkafar closed this as completed in 34c1ba8 Aug 27, 2024
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this issue Oct 9, 2024
@kkafar @alduzy @ja1ns
fix(Android,Fabric): pressable on Screen loses focus on pointer mov…
32cc13c 
…ement (software-mansion#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>


![Screenshot_1723212300](https://github.com/user-attachments/assets/57e2f4a3-b002-4ca3-9519-45cfece860c4)

</td>
<td>


![Screenshot_1723212331](https://github.com/user-attachments/assets/bd46c8d1-8813-4fae-a8a9-0326193371d2)

</td>
<td>


![Screenshot_1723212382](https://github.com/user-attachments/assets/c7373437-524d-4a0f-951e-ce2689a4fe5c)

</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](software-mansion#2169),
which was supposed to prevent content jumps, but doesn't work since RN
`0.75` sadly.


![Screenshot_1723220034](https://github.com/user-attachments/assets/b0908c23-4667-4ccf-8e5e-5e7e11bca316)

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 software-mansion#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kkafar kkafar

@alduzy alduzy

Labels
Platform: Android This issue is specific to Android Repro provided A reproduction with a snack or repo is provided
Projects
None yet
Milestone
No milestone
6 participants
@franksmule @grgmo @dviluk @yosukapro @kkafar @alduzy