Commit c436fbb
authored
## 📜 Description
Fixed a problem with non-working `maintainVisibleScrollPosition` when
using `inverted` prop of `KeyboardChatScrollView`.
## 💡 Motivation and Context
The issue stems from the fact that we use additional `View` in
`KeyboardChatScrollView`:
```tsx
<ScrollViewComponent ref={ref} animatedProps={animatedProps} {...rest}>
{inverted ? (
// The only thing it can break is `StickyHeader`, but it's already broken in FlatList and other lists
// don't support this functionality, so we can add additional view here
// The correct fix would be to add a new prop in ScrollView that allows
// to customize children extraction logic and skip custom view
<View collapsable={false} nativeID="container">
{children}
</View>
) : (
children
)}
</ScrollViewComponent>
```
It totally breaks `maintainVisibleScrollPosition` behavior. The
algorithm is similar on both iOS/Android, but we'll use Android code
snippets for demonstration. First of all it iterates over **direct
children** of `contentView`:
```kt
for (i in config.minIndexForVisible until contentView.childCount) {
val child = contentView.getChildAt(i)
val position = child.y + child.height
if (position > currentScroll || i == contentView.childCount - 1) {
firstVisibleViewRef = WeakReference(child)
prevFirstVisibleFrame = frame
break
}
}
```
So when we draw additional `View` - the algorithm becomes completely
broken. Further the code does something like this:
```kt
val deltaY = newFrame.top - prevFirstVisibleFrame.top
if (deltaY != 0) {
scrollView.scrollToPreservingMomentum(scrollView.scrollX, scrollY + deltaY)
}
```
But this code will never work properly, because on first step we will
not be able to compute proper `prevFirstVisibleFrame` (since we have
only single `View` and this `View` is always visible).
First idea that I had in my mind was adding `TransientView` that would
substitute `childCount`/`getChildAt` and forward them down to inner
children. However it significantly increases the complexity of the app.
And I decided to dig deeper into "why exactly I added this code?".
The reason was very simple. My virtualized list example was flickering
during keyboard animation (pay attention to "So far it looks cool!"
message in the bottom of the screen):
https://github.com/user-attachments/assets/e66d119d-3818-4b10-b921-4d84af3d414c
If we check debugger we can see that this item has been recycled:
<img width="1156" height="159" alt="Screenshot 2026-04-17 at 10 57 35"
src="https://github.com/user-attachments/assets/696150da-2f44-48ba-b93b-fa8ce02ac6de"
/>
> [!IMPORTANT]
> This issue seems to be reproducible only with `FlashList` (not
`FlatList`). If I remember correctly `FlatList` tries to mount all items
if we don't provide a function for layout prediction (just do it in
batch updates). `FlashList` actually recycles items (and because of that
it has better memory footprint). So the issue is reproducible only in
`FlashList` since it recycles items.
> [!WARNING]
> I also managed to reproduce it on new arch only. Old arch works well
even without `drawDistance` specification 🤷♂️
I think this problem is caused by the fact that we change `translateY`
of the content and perform scroll. In this case visible items are not
the same that `FlashList` thinks are visible (because `FlashList`
doesn't know anything about `translateY` that was modified natively).
So the fix is to remove additional view and simply increase `windowSize`
in example app (I also mentioned this side effect in documentation).
Additionally I think that this fake `View` may completely break
recycling of `FlashList` because we render a single big `View` which
technically is always visible 🤷♂️
<hr />
So in general: rendering additional `View` may break many things
(maintainVisibleScrollPosition, recycling) so it's not a correct
solution. Current solution - remove that fake view AND increase
rendering buffer for virtualized lists.
Closes
#1423
## 📢 Changelog
<!-- High level overview of important changes -->
<!-- For example: fixed status bar manipulation; added new types
declarations; -->
<!-- If your changes don't affect one of platform/language below - then
remove this platform/language -->
### Docs
- mention that `inverted` prop may require enhanced `drawDistance`;
### JS
- removed additional `View` when rendering `inverted` list;
### Android
- use `contentView` directly (no first child of `contentView` anymore)
for modifying `translateY` posiiton;
## 🤔 How Has This Been Tested?
Tested manually on Pixel 7 Pro (API 36).
## 📸 Screenshots (if appropriate):
### Flickering
|Before|After|
|-------|----|
|<video
src="https://github.com/user-attachments/assets/e66d119d-3818-4b10-b921-4d84af3d414c">|<video
src="https://github.com/user-attachments/assets/16b5439b-4fb6-4c61-9921-d9704f28cf6e">|
### Auto-scrolling
|Before|After|
|-------|----|
|<video
src="https://github.com/user-attachments/assets/1390ab14-7359-4d21-b391-25a59f51bb0a">|<video
src="https://github.com/user-attachments/assets/4d2c0884-ff07-4bd8-a1da-aaa505c4ec2c">|
## 📝 Checklist
- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
1 parent 15111a4 commit c436fbb
10 files changed
Lines changed: 42 additions & 23 deletions
File tree
- FabricExample
- src/screens/Examples/KeyboardChatScrollView
- android/src/main/java/com/reactnativekeyboardcontroller/views
- docs
- docs/api/components
- versioned_docs/version-1.21.0/api/components
- example/src/screens/Examples/KeyboardChatScrollView
- src/components/ScrollViewWithBottomPadding
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
25 | | - | |
| 25 | + | |
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
| |||
Lines changed: 3 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
72 | | - | |
73 | | - | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
74 | 75 | | |
75 | 76 | | |
76 | 77 | | |
| |||
Lines changed: 14 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
| |||
34 | 35 | | |
35 | 36 | | |
36 | 37 | | |
| 38 | + | |
37 | 39 | | |
38 | 40 | | |
39 | 41 | | |
| |||
117 | 119 | | |
118 | 120 | | |
119 | 121 | | |
120 | | - | |
121 | | - | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
122 | 134 | | |
123 | 135 | | |
124 | 136 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1909 | 1909 | | |
1910 | 1910 | | |
1911 | 1911 | | |
1912 | | - | |
1913 | | - | |
1914 | | - | |
1915 | | - | |
| 1912 | + | |
| 1913 | + | |
| 1914 | + | |
| 1915 | + | |
1916 | 1916 | | |
1917 | 1917 | | |
1918 | 1918 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
51 | | - | |
| 51 | + | |
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
43 | 47 | | |
44 | 48 | | |
45 | 49 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
43 | 47 | | |
44 | 48 | | |
45 | 49 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
72 | | - | |
| 72 | + | |
73 | 73 | | |
74 | 74 | | |
75 | 75 | | |
| |||
Lines changed: 8 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
| |||
127 | 128 | | |
128 | 129 | | |
129 | 130 | | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
130 | 138 | | |
131 | 139 | | |
132 | 140 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
| 2 | + | |
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| |||
125 | 125 | | |
126 | 126 | | |
127 | 127 | | |
128 | | - | |
129 | | - | |
130 | | - | |
131 | | - | |
132 | | - | |
133 | | - | |
134 | | - | |
135 | | - | |
136 | | - | |
137 | | - | |
138 | | - | |
| 128 | + | |
139 | 129 | | |
140 | 130 | | |
141 | 131 | | |
| |||
0 commit comments