Skip to content

Commit 0547b19

Browse files
authored
fix(Android,Fabric): pressables losing focus on screens with formSheet presentation (#2788)
## Description This PR relates to new architecture only. Currently, the pressables on form sheet lose focus on `move` action. This is the same problem we had with `Pressables` in screen & header on new architecture. See: #2466 Basically the information about sheet position is different between `ShadowTree` (ST) and `HostTree` (HT), which leads to losing focus due to how pressables are now handled on new architecture. **Simplified description** of basically what happens when you click a pressable on new arch is as follows (Android): 1. The gesture is detected after the host platform dispatches touch event (touch events on Android are dispatched in top-down manner) [(link)](https://github.com/facebook/react-native/blob/d6ca25b0c1a0aeed3507ec3c2f65e453e2700dc2/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/JSTouchDispatcher.java#L81-L105) 2. The JS responder is set & touch event dispatched - therefore pressable is always clickable, 3. Moreover, the JS responder is [measured **IN THE ST**](https://github.com/facebook/react-native/blob/d6ca25b0c1a0aeed3507ec3c2f65e453e2700dc2/packages/react-native/Libraries/Pressability/Pressability.js#L805-L810) 3. When you start moving your finger, the platform is still the source of motion events, and target [coordinates are **read from HOST TREE**](#2466) 4. Motion event (with platform measurement (HT)!!!) is then [compared in JS with responder measurements based on ST](https://github.com/facebook/react-native/blob/d6ca25b0c1a0aeed3507ec3c2f65e453e2700dc2/packages/react-native/Libraries/Pressability/Pressability.js#L831-L8750) Therefore, any inconsistency here leads to responder losing focus. Reference: 1. [`TouchesHelper.createPointersArray`](https://github.com/facebook/react-native/blob/ac97177651cf369783ca93fac50c2824b484abef/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/TouchesHelper.kt#L37) 2. [Responder measurement](https://github.com/facebook/react-native/blob/d6ca25b0c1a0aeed3507ec3c2f65e453e2700dc2/packages/react-native/Libraries/Pressability/Pressability.js#L805-L810) 3. [Gesture start detection (Android)](https://github.com/facebook/react-native/blob/d6ca25b0c1a0aeed3507ec3c2f65e453e2700dc2/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/JSTouchDispatcher.java#L81-L105) 4. [Gesture move handling (Android)](https://github.com/facebook/react-native/blob/d6ca25b0c1a0aeed3507ec3c2f65e453e2700dc2/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/JSTouchDispatcher.java#L137-L147) 5. [Checking whether native touch is in responder region](https://github.com/facebook/react-native/blob/d6ca25b0c1a0aeed3507ec3c2f65e453e2700dc2/packages/react-native/Libraries/Pressability/Pressability.js#L831-L875) What is bewildering is that it works on iOS, despite the fact, that e.g. when using React inspector the views are completely out of place in ShadowTree. I haven't debugged the iOS part to the very bottom, but my suspicion is as follows: 1. The form sheet on Android is mounted in subtree under react root view, while on iOS it is mounted under separate `UITransitionView` (different subtree of window), 2. additionally we use `RNSModalScreen` component on iOS, which has shadow node with `RootNodeKind` (measurements in subtree of its shadow node are done relative to it), 3. There is no root view / any react view above it -> native measurement (done by react-native) might also think that its positioned at (0, 0) (same as in shadow tree). This is something to verify in the future. Opened ticked on the board for it. ## Changes The sheet now sends updates to the shadow tree at following moments: 1. layout (including initial one), 2. sheet detent change to any stable state (could consider here not sending the update when the sheet is hidden, added to project board), I'm not sending updates on every sheet move (`View.top` change), to avoid clogging the JS thread (and later UI), whole advantage of our sheet is that it feels smooth (given sensible Android device). However, it could be considered in case of any further issues. Please note, that between the event syncs or in case the JS thread is blocked / clogged, this still will be a problem. However, any JS would lag, not only pressables and it's not down to us. Commits: - **Prevent modal content from disappearing** - **Add comment that createShadowNode is used only on old arch** - **Add comment on threading on NativeProxy mechanisms** - **Simplify code in RNSModalScreenShadowNode.cpp** - **Make modal screen component selection more clear in Screen.tsx** - **Add `headerHeight` to diff prop list when determining need for shadow state update** - **Add pressables to form sheet example!** - **Send updates to shadow tree on form sheet layout changes in appropriate moments** ### Recordings | before | after | | -- | -- | | <video src="https://github.com/user-attachments/assets/b5bbb149-61da-41d1-b6a4-73dd89eb1f92" alt="before" /> | <video src="https://github.com/user-attachments/assets/ce5c89c0-7215-4ebb-a347-fa50946308f4" alt="after" /> | ## Test code and steps to reproduce `TestFormSheet`, open the sheet, play with it & see that pressables work! ## Checklist - [x] Included code example that can be used to test this change - [ ] Ensured that CI passes
1 parent 65f6e92 commit 0547b19

File tree

10 files changed

+95
-31
lines changed

10 files changed

+95
-31
lines changed

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ abstract class FabricEnabledViewGroup(
1414
) : ViewGroup(context) {
1515
private var mStateWrapper: StateWrapper? = null
1616

17-
private var lastSetWidth = 0f
18-
private var lastSetHeight = 0f
17+
private var lastWidth = 0f
18+
private var lastHeight = 0f
19+
private var lastHeaderHeight = 0f
1920

2021
fun setStateWrapper(wrapper: StateWrapper?) {
2122
mStateWrapper = wrapper
@@ -42,14 +43,16 @@ abstract class FabricEnabledViewGroup(
4243
// Check incoming state values. If they're already the correct value, return early to prevent
4344
// infinite UpdateState/SetState loop.
4445
val delta = 0.9f
45-
if (abs(lastSetWidth - realWidth) < delta &&
46-
abs(lastSetHeight - realHeight) < delta
46+
if (abs(lastWidth - realWidth) < delta &&
47+
abs(lastHeight - realHeight) < delta &&
48+
abs(lastHeaderHeight - realHeaderHeight) < delta
4749
) {
4850
return
4951
}
5052

51-
lastSetWidth = realWidth
52-
lastSetHeight = realHeight
53+
lastWidth = realWidth
54+
lastHeight = realHeight
55+
lastHeaderHeight = realHeaderHeight
5356
val map: WritableMap =
5457
WritableNativeMap().apply {
5558
putDouble("frameWidth", realWidth.toDouble())

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ class NativeProxy {
4141
}
4242
}
4343

44-
// Called from native
44+
// Called from native. Currently this method is called from MountingCoordinator thread,
45+
// which usually is not UI thread.
4546
@DoNotStrip
4647
public fun notifyScreenRemoved(screenTag: Int) {
4748
// Since RN 0.78 the screenTag we receive as argument here might not belong to a screen

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

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,9 @@ class Screen(
103103
field = value
104104
}
105105

106+
private val isNativeStackScreen: Boolean
107+
get() = container is ScreenStack
108+
106109
init {
107110
// we set layout params as WindowManager.LayoutParams to workaround the issue with TextInputs
108111
// not displaying modal menus (e.g., copy/paste or selection). The missing menus are due to the
@@ -173,23 +176,31 @@ class Screen(
173176
r: Int,
174177
b: Int,
175178
) {
176-
if (container is ScreenStack && changed) {
179+
// In case of form sheet we get layout notification a bit later, in `onBottomSheetBehaviorDidLayout`
180+
// after the attached behaviour laid out this view.
181+
if (changed && isNativeStackScreen && !usesFormSheetPresentation()) {
177182
val width = r - l
178183
val height = b - t
179184

180-
if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
181-
updateScreenSizeFabric(width, height, t)
182-
} else {
183-
updateScreenSizePaper(width, height)
184-
}
185+
dispatchShadowStateUpdate(width, height, t)
185186

186-
footer?.onParentLayout(changed, l, t, r, b, container!!.height)
187+
// FormSheet has no header in current model.
187188
notifyHeaderHeightChange(t)
189+
}
190+
}
188191

189-
if (!BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
190-
maybeTriggerPostponedTransition()
191-
}
192+
internal fun onBottomSheetBehaviorDidLayout(coordinatorLayoutDidChange: Boolean) {
193+
if (!usesFormSheetPresentation()) {
194+
return
195+
}
196+
if (coordinatorLayoutDidChange && isNativeStackScreen) {
197+
dispatchShadowStateUpdate(width, height, top)
192198
}
199+
if (!BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
200+
maybeTriggerPostponedTransition()
201+
}
202+
203+
footer?.onParentLayout(coordinatorLayoutDidChange, left, top, right, bottom, container!!.height)
193204
}
194205

195206
private fun maybeTriggerPostponedTransition() {
@@ -214,6 +225,21 @@ class Screen(
214225
)
215226
}
216227

228+
/**
229+
* @param offsetY ignored on old architecture
230+
*/
231+
private fun dispatchShadowStateUpdate(
232+
width: Int,
233+
height: Int,
234+
offsetY: Int,
235+
) {
236+
if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
237+
updateScreenSizeFabric(width, height, offsetY)
238+
} else {
239+
updateScreenSizePaper(width, height)
240+
}
241+
}
242+
217243
val headerConfig: ScreenStackHeaderConfig?
218244
get() = children.find { it is ScreenStackHeaderConfig } as? ScreenStackHeaderConfig
219245

@@ -487,6 +513,11 @@ class Screen(
487513
isStable: Boolean,
488514
) {
489515
dispatchSheetDetentChanged(detentIndex, isStable)
516+
// There is no need to update shadow state for transient sheet states -
517+
// we are unsure of the exact sheet position anyway.
518+
if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED && isStable) {
519+
updateScreenSizeFabric(width, height, top)
520+
}
490521
}
491522

492523
private fun dispatchSheetDetentChanged(

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,14 @@ class ScreenStackFragment :
543543
}
544544
}
545545

546+
override fun onLayout(changed: Boolean, l: Int, t: Int, r: Int, b: Int) {
547+
super.onLayout(changed, l, t, r, b)
548+
549+
if (fragment.screen.usesFormSheetPresentation()) {
550+
fragment.screen.onBottomSheetBehaviorDidLayout(changed)
551+
}
552+
}
553+
546554
// override fun reactTagForTouch(touchX: Float, touchY: Float): Int {
547555
// throw IllegalStateException("Screen wrapper should never be asked for the view tag")
548556
// }

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ class ScreenStackViewManager :
6161
index: Int,
6262
): View = parent.getScreenAt(index)
6363

64+
// Old architecture only.
6465
override fun createShadowNodeInstance(context: ReactApplicationContext): LayoutShadowNode = ScreensShadowNode(context)
6566

6667
override fun needsCustomLayoutForChildren() = true

apps/src/tests/Test2223.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from 'react';
2-
import { View, Text, Button, Pressable, StyleSheet, Alert } from 'react-native';
2+
import { Alert, Button, Pressable, StyleSheet, Text, View } from 'react-native';
33
import { NavigationContainer, useNavigation } from '@react-navigation/native';
44
import { createNativeStackNavigator } from '@react-navigation/native-stack';
55

apps/src/tests/TestFormSheet.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { NavigationContainer, RouteProp } from '@react-navigation/native';
22
import { NativeStackNavigationProp, createNativeStackNavigator } from '@react-navigation/native-stack';
33
import React from 'react';
44
import { Button, FlatList, ScrollView, Text, TextInput, View } from 'react-native';
5+
import PressableWithFeedback from '../shared/PressableWithFeedback';
56

67
type ItemData = {
78
id: number,
@@ -35,6 +36,11 @@ function Home({ navigation }: RouteProps<'Home'>) {
3536
<Button title="Open Second" onPress={() => navigation.navigate('Second')} />
3637
<Button title="Open sheet with FlatList" onPress={() => navigation.navigate('FormSheetWithFlatList')} />
3738
<Button title="Open sheet with ScrollView" onPress={() => navigation.navigate('FormSheetWithScrollView')} />
39+
<PressableWithFeedback>
40+
<View style={{ alignItems: 'center', height: 40, justifyContent: 'center' }}>
41+
<Text>Pressable</Text>
42+
</View>
43+
</PressableWithFeedback>
3844
</View>
3945
);
4046
}
@@ -56,6 +62,11 @@ function FormSheet({ navigation }: RouteProps<'FormSheet'>) {
5662
<Button title="Go back" onPress={() => navigation.goBack()} />
5763
<Button title="Open Second" onPress={() => navigation.navigate('Second')} />
5864
<Button title="Open SecondFormSheet" onPress={() => navigation.navigate('SecondFormSheet')} />
65+
<PressableWithFeedback>
66+
<View style={{ alignItems: 'center', height: 40, justifyContent: 'center' }}>
67+
<Text>Pressable</Text>
68+
</View>
69+
</PressableWithFeedback>
5970
</View>
6071
<View style={{ alignItems: 'center' }}>
6172
<TextInput style={{ marginVertical: 12, paddingVertical: 8, backgroundColor: 'lavender', borderRadius: 24, width: '80%' }} placeholder="Trigger keyboard..." />
@@ -140,6 +151,7 @@ function FormSheetWithScrollView() {
140151
);
141152
}
142153

154+
// @ts-ignore // uncomment the usage down below if needed
143155
// eslint-disable-next-line @typescript-eslint/no-unused-vars
144156
function FormSheetFooter() {
145157
return (

common/cpp/react/renderer/components/rnscreens/RNSModalScreenShadowNode.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ extern const char RNSModalScreenComponentName[] = "RNSModalScreen";
77

88
Point RNSModalScreenShadowNode::getContentOriginOffset(
99
bool /*includeTransform*/) const {
10-
auto stateData = getStateData();
11-
auto contentOffset = stateData.contentOffset;
12-
return {contentOffset.x, contentOffset.y};
10+
return getStateData().contentOffset;
1311
}
1412

1513
} // namespace react

cpp/RNSScreenRemovalListener.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ std::optional<MountingTransaction> RNSScreenRemovalListener::pullTransaction(
88
const TransactionTelemetry &telemetry,
99
ShadowViewMutationList mutations) const {
1010
for (const ShadowViewMutation &mutation : mutations) {
11+
// When using RNSModalScreen on Android it should be added here.
1112
if (mutation.type == ShadowViewMutation::Type::Remove &&
1213
mutation.oldChildShadowView.componentName != nullptr &&
13-
strcmp(mutation.oldChildShadowView.componentName, "RNSScreen") == 0) {
14+
std::strcmp(mutation.oldChildShadowView.componentName, "RNSScreen") ==
15+
0) {
1416
// We call the listener function even if this screen has not been owned
1517
// by RNSScreenStack as since RN 0.78 we do not have enough information
1618
// here. This final filter is applied later in NativeProxy.

src/components/Screen.tsx

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -213,15 +213,23 @@ export const InnerScreen = React.forwardRef<View, ScreenProps>(
213213
sheetInitialDetentIndex,
214214
resolvedSheetAllowedDetents.length - 1,
215215
);
216-
// Due to how Yoga resolves layout, we need to have different components for modal nad non-modal screens
217-
const AnimatedScreen =
218-
Platform.OS === 'android' ||
219-
stackPresentation === undefined ||
220-
stackPresentation === 'push' ||
221-
stackPresentation === 'containedModal' ||
222-
stackPresentation === 'containedTransparentModal'
223-
? AnimatedNativeScreen
224-
: AnimatedNativeModalScreen;
216+
217+
// Due to how Yoga resolves layout, we need to have different components for modal nad non-modal screens (there is a need for different
218+
// shadow nodes).
219+
const shouldUseModalScreenComponent = Platform.select({
220+
ios: !(
221+
stackPresentation === undefined ||
222+
stackPresentation === 'push' ||
223+
stackPresentation === 'containedModal' ||
224+
stackPresentation === 'containedTransparentModal'
225+
),
226+
android: false,
227+
default: false,
228+
});
229+
230+
const AnimatedScreen = shouldUseModalScreenComponent
231+
? AnimatedNativeModalScreen
232+
: AnimatedNativeScreen;
225233

226234
let {
227235
// Filter out active prop in this case because it is unused and

0 commit comments

Comments
 (0)