Commit 0547b19
authored
fix(Android,Fabric): pressables losing focus on screens with
## 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 passesformSheet presentation (#2788)1 parent 65f6e92 commit 0547b19
10 files changed
Lines changed: 95 additions & 31 deletions
File tree
- android/src
- fabric/java/com/swmansion/rnscreens
- main/java/com/swmansion/rnscreens
- apps/src/tests
- common/cpp/react/renderer/components/rnscreens
- cpp
- src/components
Lines changed: 9 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | | - | |
18 | | - | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
19 | 20 | | |
20 | 21 | | |
21 | 22 | | |
| |||
42 | 43 | | |
43 | 44 | | |
44 | 45 | | |
45 | | - | |
46 | | - | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
47 | 49 | | |
48 | 50 | | |
49 | 51 | | |
50 | 52 | | |
51 | | - | |
52 | | - | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
53 | 56 | | |
54 | 57 | | |
55 | 58 | | |
| |||
Lines changed: 2 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
41 | 41 | | |
42 | 42 | | |
43 | 43 | | |
44 | | - | |
| 44 | + | |
| 45 | + | |
45 | 46 | | |
46 | 47 | | |
47 | 48 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
103 | 103 | | |
104 | 104 | | |
105 | 105 | | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
106 | 109 | | |
107 | 110 | | |
108 | 111 | | |
| |||
173 | 176 | | |
174 | 177 | | |
175 | 178 | | |
176 | | - | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
177 | 182 | | |
178 | 183 | | |
179 | 184 | | |
180 | | - | |
181 | | - | |
182 | | - | |
183 | | - | |
184 | | - | |
| 185 | + | |
185 | 186 | | |
186 | | - | |
| 187 | + | |
187 | 188 | | |
| 189 | + | |
| 190 | + | |
188 | 191 | | |
189 | | - | |
190 | | - | |
191 | | - | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
192 | 198 | | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
193 | 204 | | |
194 | 205 | | |
195 | 206 | | |
| |||
214 | 225 | | |
215 | 226 | | |
216 | 227 | | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
217 | 243 | | |
218 | 244 | | |
219 | 245 | | |
| |||
487 | 513 | | |
488 | 514 | | |
489 | 515 | | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
490 | 521 | | |
491 | 522 | | |
492 | 523 | | |
| |||
Lines changed: 8 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
543 | 543 | | |
544 | 544 | | |
545 | 545 | | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
546 | 554 | | |
547 | 555 | | |
548 | 556 | | |
| |||
Lines changed: 1 addition & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
61 | 61 | | |
62 | 62 | | |
63 | 63 | | |
| 64 | + | |
64 | 65 | | |
65 | 66 | | |
66 | 67 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
| 2 | + | |
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
| |||
35 | 36 | | |
36 | 37 | | |
37 | 38 | | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
38 | 44 | | |
39 | 45 | | |
40 | 46 | | |
| |||
56 | 62 | | |
57 | 63 | | |
58 | 64 | | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
59 | 70 | | |
60 | 71 | | |
61 | 72 | | |
| |||
140 | 151 | | |
141 | 152 | | |
142 | 153 | | |
| 154 | + | |
143 | 155 | | |
144 | 156 | | |
145 | 157 | | |
| |||
Lines changed: 1 addition & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
11 | | - | |
12 | | - | |
| 10 | + | |
13 | 11 | | |
14 | 12 | | |
15 | 13 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | | - | |
| 14 | + | |
| 15 | + | |
14 | 16 | | |
15 | 17 | | |
16 | 18 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
213 | 213 | | |
214 | 214 | | |
215 | 215 | | |
216 | | - | |
217 | | - | |
218 | | - | |
219 | | - | |
220 | | - | |
221 | | - | |
222 | | - | |
223 | | - | |
224 | | - | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
225 | 233 | | |
226 | 234 | | |
227 | 235 | | |
| |||
0 commit comments