Skip to content

Commit 34c1ba8

Browse files
kkafaralduzy
andauthored
fix(Android,Fabric): pressable on Screen loses focus on pointer movement (#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](#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 #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 <[email protected]> Co-authored-by: Alex Duży <[email protected]>
1 parent 80dcad4 commit 34c1ba8

File tree

9 files changed

+228
-138
lines changed

9 files changed

+228
-138
lines changed

android/src/fabric/java/com/swmansion/rnscreens/FabricEnabledViewGroup.kt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ abstract class FabricEnabledViewGroup(
2424
protected fun updateScreenSizeFabric(
2525
width: Int,
2626
height: Int,
27-
headerHeight: Double,
27+
headerHeight: Int,
2828
) {
2929
updateState(width, height, headerHeight)
3030
}
@@ -33,10 +33,11 @@ abstract class FabricEnabledViewGroup(
3333
fun updateState(
3434
width: Int,
3535
height: Int,
36-
headerHeight: Double,
36+
headerHeight: Int,
3737
) {
3838
val realWidth: Float = PixelUtil.toDIPFromPixel(width.toFloat())
3939
val realHeight: Float = PixelUtil.toDIPFromPixel(height.toFloat())
40+
val realHeaderHeight: Float = PixelUtil.toDIPFromPixel(headerHeight.toFloat())
4041

4142
// Check incoming state values. If they're already the correct value, return early to prevent
4243
// infinite UpdateState/SetState loop.
@@ -54,7 +55,7 @@ abstract class FabricEnabledViewGroup(
5455
putDouble("frameWidth", realWidth.toDouble())
5556
putDouble("frameHeight", realHeight.toDouble())
5657
putDouble("contentOffsetX", 0.0)
57-
putDouble("contentOffsetY", headerHeight)
58+
putDouble("contentOffsetY", realHeaderHeight.toDouble())
5859
}
5960
mStateWrapper?.updateState(map)
6061
}

android/src/main/java/com/swmansion/rnscreens/Screen.kt

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import android.content.pm.ActivityInfo
55
import android.graphics.Paint
66
import android.os.Parcelable
77
import android.util.SparseArray
8-
import android.util.TypedValue
98
import android.view.View
109
import android.view.ViewGroup
1110
import android.view.WindowManager
@@ -15,7 +14,6 @@ import androidx.core.view.children
1514
import androidx.fragment.app.Fragment
1615
import com.facebook.react.bridge.GuardedRunnable
1716
import com.facebook.react.bridge.ReactContext
18-
import com.facebook.react.uimanager.PixelUtil
1917
import com.facebook.react.uimanager.UIManagerHelper
2018
import com.facebook.react.uimanager.UIManagerModule
2119
import com.facebook.react.uimanager.events.EventDispatcher
@@ -141,17 +139,14 @@ class Screen(
141139
val width = r - l
142140
val height = b - t
143141

144-
val headerHeight = calculateHeaderHeight()
145-
val totalHeight =
146-
headerHeight.first + headerHeight.second // action bar height + status bar height
147142
if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
148-
updateScreenSizeFabric(width, height, totalHeight)
143+
updateScreenSizeFabric(width, height, t)
149144
} else {
150145
updateScreenSizePaper(width, height)
151146
}
152147

153148
footer?.onParentLayout(changed, l, t, r, b, container!!.height)
154-
notifyHeaderHeightChange(totalHeight)
149+
notifyHeaderHeightChange(t)
155150
}
156151
}
157152

@@ -400,32 +395,7 @@ class Screen(
400395
}
401396
}
402397

403-
private fun calculateHeaderHeight(): Pair<Double, Double> {
404-
val actionBarTv = TypedValue()
405-
val resolvedActionBarSize =
406-
context.theme.resolveAttribute(android.R.attr.actionBarSize, actionBarTv, true)
407-
408-
// Check if it's possible to get an attribute from theme context and assign a value from it.
409-
// Otherwise, the default value will be returned.
410-
val actionBarHeight =
411-
TypedValue
412-
.complexToDimensionPixelSize(actionBarTv.data, resources.displayMetrics)
413-
.takeIf { resolvedActionBarSize && headerConfig?.isHeaderHidden != true && headerConfig?.isHeaderTranslucent != true }
414-
?.let { PixelUtil.toDIPFromPixel(it.toFloat()).toDouble() } ?: 0.0
415-
416-
val statusBarHeight =
417-
context.resources
418-
.getIdentifier("status_bar_height", "dimen", "android")
419-
// Count only status bar when action bar is visible and status bar is not hidden
420-
.takeIf { it > 0 && isStatusBarHidden != true && actionBarHeight > 0 }
421-
?.let { (context.resources::getDimensionPixelSize)(it) }
422-
?.let { PixelUtil.toDIPFromPixel(it.toFloat()).toDouble() }
423-
?: 0.0
424-
425-
return actionBarHeight to statusBarHeight
426-
}
427-
428-
private fun notifyHeaderHeightChange(headerHeight: Double) {
398+
private fun notifyHeaderHeightChange(headerHeight: Int) {
429399
val screenContext = context as ReactContext
430400
val surfaceId = UIManagerHelper.getSurfaceId(screenContext)
431401
UIManagerHelper

android/src/main/java/com/swmansion/rnscreens/events/HeaderHeightChangeEvent.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,16 @@ import com.facebook.react.uimanager.events.Event
77
class HeaderHeightChangeEvent(
88
surfaceId: Int,
99
viewId: Int,
10-
private val headerHeight: Double,
10+
private val headerHeight: Int,
1111
) : Event<HeaderHeightChangeEvent>(surfaceId, viewId) {
1212
override fun getEventName() = EVENT_NAME
1313

1414
// As the same header height could appear twice, use header height as a coalescing key.
15-
override fun getCoalescingKey(): Short = headerHeight.toInt().toShort()
15+
override fun getCoalescingKey(): Short = headerHeight.toShort()
1616

1717
override fun getEventData(): WritableMap? =
1818
Arguments.createMap().apply {
19-
putDouble("headerHeight", headerHeight)
19+
putDouble("headerHeight", headerHeight.toDouble())
2020
}
2121

2222
companion object {

android/src/paper/java/com/swmansion/rnscreens/FabricEnabledViewGroup.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ abstract class FabricEnabledViewGroup(
1212
protected fun updateScreenSizeFabric(
1313
width: Int,
1414
height: Int,
15-
headerHeight: Double,
15+
headerHeight: Int,
1616
) {
1717
// do nothing
1818
}

apps/src/tests/Test1975.tsx

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import * as React from 'react';
2+
import { NavigationContainer } from '@react-navigation/native';
3+
import { View, Text, StyleSheet, Pressable } from 'react-native';
4+
import { createNativeStackNavigator } from '@react-navigation/native-stack';
5+
6+
const Stack = createNativeStackNavigator();
7+
8+
function App() {
9+
const [count, setCount] = React.useState(0);
10+
11+
return (
12+
<NavigationContainer>
13+
<Stack.Navigator screenOptions={{ statusBarTranslucent: false }}>
14+
<Stack.Screen
15+
name="Screen"
16+
component={Screen}
17+
options={{
18+
headerRight: () => (
19+
<Pressable
20+
onPress={() => setCount(prev => prev + 1)}
21+
style={({ pressed }) => [
22+
styles.pressable,
23+
pressed && { backgroundColor: 'goldenrod' },
24+
]}>
25+
<Text>Press (+)</Text>
26+
</Pressable>
27+
),
28+
title: count.toString(),
29+
headerTitleAlign: 'center',
30+
}}
31+
/>
32+
<Stack.Screen
33+
name="Details"
34+
component={DetailsScreen}
35+
options={({ navigation }) => ({
36+
title: 'Details',
37+
headerRight: () => (
38+
<Pressable
39+
onPress={navigation.goBack}
40+
style={({ pressed }) => [
41+
styles.pressable,
42+
pressed && { backgroundColor: 'goldenrod' },
43+
]}>
44+
<Text>Go Back</Text>
45+
</Pressable>
46+
),
47+
})}
48+
/>
49+
</Stack.Navigator>
50+
</NavigationContainer>
51+
);
52+
}
53+
54+
function Screen({ navigation }: any) {
55+
return (
56+
<View style={styles.container}>
57+
<Pressable
58+
onPress={() => navigation.navigate('Details')}
59+
style={({ pressed }) => [
60+
styles.pressable,
61+
pressed && { backgroundColor: 'goldenrod' },
62+
]}>
63+
<Text>Go to Details</Text>
64+
</Pressable>
65+
</View>
66+
);
67+
}
68+
69+
function DetailsScreen() {
70+
let counter = React.useRef(0);
71+
72+
return (
73+
<View style={{ ...styles.container, backgroundColor: 'beige' }}>
74+
<Pressable
75+
onPressIn={() => {
76+
counter.current += 1;
77+
console.log(`[${counter.current}] Details: onPressIn`);
78+
}}
79+
onPress={() => {
80+
console.log(`[${counter.current}] Details: onPress`);
81+
}}
82+
onPressOut={() => {
83+
console.log(`[${counter.current}] Details: onPressOut`);
84+
}}
85+
style={({ pressed }) => [
86+
styles.pressable,
87+
pressed && { backgroundColor: 'goldenrod' },
88+
]}>
89+
<Text>Press me</Text>
90+
</Pressable>
91+
</View>
92+
);
93+
}
94+
95+
const styles = StyleSheet.create({
96+
pressable: {
97+
paddingVertical: 5,
98+
paddingHorizontal: 10,
99+
backgroundColor: 'red',
100+
},
101+
container: {
102+
flex: 1,
103+
alignItems: 'center',
104+
justifyContent: 'center',
105+
gap: 24,
106+
},
107+
});
108+
109+
export default App;

apps/src/tests/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ export { default as Test1829 } from './Test1829';
9393
export { default as Test1844 } from './Test1844';
9494
export { default as Test1864 } from './Test1864';
9595
export { default as Test1970 } from './Test1970';
96+
export { default as Test1975 } from './Test1975';
9697
export { default as Test1981 } from './Test1981';
9798
export { default as Test2002 } from './Test2002';
9899
export { default as Test2008 } from './Test2008';

common/cpp/react/renderer/components/rnscreens/RNSScreenComponentDescriptor.h

Lines changed: 4 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,6 @@ class RNSScreenComponentDescriptor final
1919
public:
2020
using ConcreteComponentDescriptor::ConcreteComponentDescriptor;
2121

22-
static constexpr const char *kScreenDummyLayoutHelperClass =
23-
"com/swmansion/rnscreens/utils/ScreenDummyLayoutHelper";
24-
2522
void adopt(ShadowNode &shadowNode) const override {
2623
react_native_assert(dynamic_cast<RNSScreenShadowNode *>(&shadowNode));
2724
auto &screenShadowNode = static_cast<RNSScreenShadowNode &>(shadowNode);
@@ -39,8 +36,8 @@ class RNSScreenComponentDescriptor final
3936
#ifdef ANDROID
4037
if (stateData.frameSize.width != 0 && stateData.frameSize.height != 0) {
4138
// When we receive dimensions from JVM side we can remove padding used for
42-
// correction, and we can stop applying height correction for the frame.
43-
// We want to leave top offset correction though intact.
39+
// correction, and we can stop applying height and offset corrections for
40+
// the frame.
4441
// TODO: In future, when we have dynamic header height we might want to
4542
// update Y offset correction here.
4643

@@ -67,36 +64,11 @@ class RNSScreenComponentDescriptor final
6764
screenShadowNode.setPadding({0, 0, 0, 0});
6865
screenShadowNode.getFrameCorrectionModes().unset(
6966
FrameCorrectionModes::Mode::FrameHeightCorrection);
67+
screenShadowNode.getFrameCorrectionModes().unset(
68+
FrameCorrectionModes::Mode::FrameOriginCorrection);
7069

7170
layoutableShadowNode.setSize(
7271
Size{stateData.frameSize.width, stateData.frameSize.height});
73-
} else {
74-
// This code path should be executed only on the very first (few)
75-
// layout(s), when we haven't received state update from JVM side yet.
76-
77-
auto headerConfigChildOpt = findHeaderConfigChild(layoutableShadowNode);
78-
79-
// During creation of the shadow node children are not attached yet.
80-
// We also do not want to set any padding in case.
81-
if (headerConfigChildOpt) {
82-
const auto &headerConfigChild = headerConfigChildOpt->get();
83-
const auto &headerProps =
84-
*std::static_pointer_cast<const RNSScreenStackHeaderConfigProps>(
85-
headerConfigChild->getProps());
86-
87-
const auto headerHeight = headerProps.hidden
88-
? 0.f
89-
: findHeaderHeight(
90-
headerProps.titleFontSize, headerProps.title.empty())
91-
.value_or(0.f);
92-
93-
screenShadowNode.setPadding({0, 0, 0, headerHeight});
94-
screenShadowNode.setHeaderHeight(headerHeight);
95-
screenShadowNode.getFrameCorrectionModes().set(
96-
FrameCorrectionModes::Mode(
97-
FrameCorrectionModes::Mode::FrameHeightCorrection |
98-
FrameCorrectionModes::Mode::FrameOriginCorrection));
99-
}
10072
}
10173
#else
10274
if (stateData.frameSize.width != 0 && stateData.frameSize.height != 0) {
@@ -106,72 +78,6 @@ class RNSScreenComponentDescriptor final
10678
#endif // ANDROID
10779
ConcreteComponentDescriptor::adopt(shadowNode);
10880
}
109-
110-
std::optional<std::reference_wrapper<const ShadowNode::Shared>>
111-
findHeaderConfigChild(
112-
const YogaLayoutableShadowNode &screenShadowNode) const {
113-
for (const ShadowNode::Shared &child : screenShadowNode.getChildren()) {
114-
if (std::strcmp(
115-
child->getComponentName(), "RNSScreenStackHeaderConfig") == 0) {
116-
return {std::cref(child)};
117-
}
118-
}
119-
return {};
120-
}
121-
122-
#ifdef ANDROID
123-
std::optional<float> findHeaderHeight(
124-
const int fontSize,
125-
const bool isTitleEmpty) const {
126-
JNIEnv *env = facebook::jni::Environment::current();
127-
128-
if (env == nullptr) {
129-
LOG(ERROR) << "[RNScreens] Failed to retrieve env\n";
130-
return {};
131-
}
132-
133-
jclass layoutHelperClass = env->FindClass(kScreenDummyLayoutHelperClass);
134-
135-
if (layoutHelperClass == nullptr) {
136-
LOG(ERROR) << "[RNScreens] Failed to find class with id "
137-
<< kScreenDummyLayoutHelperClass;
138-
return {};
139-
}
140-
141-
jmethodID computeDummyLayoutID =
142-
env->GetMethodID(layoutHelperClass, "computeDummyLayout", "(IZ)F");
143-
144-
if (computeDummyLayoutID == nullptr) {
145-
LOG(ERROR)
146-
<< "[RNScreens] Failed to retrieve computeDummyLayout method ID";
147-
return {};
148-
}
149-
150-
jmethodID getInstanceMethodID = env->GetStaticMethodID(
151-
layoutHelperClass,
152-
"getInstance",
153-
"()Lcom/swmansion/rnscreens/utils/ScreenDummyLayoutHelper;");
154-
155-
if (getInstanceMethodID == nullptr) {
156-
LOG(ERROR) << "[RNScreens] Failed to retrieve getInstanceMethodID";
157-
return {};
158-
}
159-
160-
jobject packageInstance =
161-
env->CallStaticObjectMethod(layoutHelperClass, getInstanceMethodID);
162-
163-
if (packageInstance == nullptr) {
164-
LOG(ERROR)
165-
<< "[RNScreens] Failed to retrieve packageInstance or the package instance was null on JVM side";
166-
return {};
167-
}
168-
169-
jfloat headerHeight = env->CallFloatMethod(
170-
packageInstance, computeDummyLayoutID, fontSize, isTitleEmpty);
171-
172-
return {headerHeight};
173-
}
174-
#endif // ANDROID
17581
};
17682

17783
} // namespace react

0 commit comments

Comments
 (0)