Skip to content

Commit a2f7e7e

Browse files
authored
fix: non-scrollable ScrollView when blankSpace is big should make it scrollable (#1487)
## 📜 Description This fixes an Android-only `KeyboardChatScrollView` edge case where short content can become scrollable only because of `blankSpace`, but a scroll gesture that starts on the content is not intercepted by the `ScrollView`. ## 💡 Motivation and Context The root cause is not Fabric and not React Native “overriding touch events” in the usual sense. The issue comes from Android `ScrollView` having two different scrollability calculations when padding is used as synthetic scrollable space. [AOSP ScrollView.java](https://android.googlesource.com/platform/frameworks/base/+/refs/heads/main/core/java/android/widget/ScrollView.java#581) `ScrollView.onInterceptTouchEvent` exits early when it thinks the view cannot scroll down: ```java if (getScrollY() == 0 && !canScrollVertically(1)) { return false; } ``` `canScrollVertically()` is implemented on [View](https://android.googlesource.com/platform/frameworks/base/+/refs/heads/main/core/java/android/view/View.java#22167) and uses: ```java computeVerticalScrollRange() - computeVerticalScrollExtent() ``` For [ScrollView](https://android.googlesource.com/platform/frameworks/base/+/refs/heads/main/core/java/android/widget/ScrollView.java#1338), `computeVerticalScrollRange()` is based on the content child bounds: ```java int scrollRange = getChildAt(0).getBottom(); ``` That does not include `paddingBottom`. But the actual scroll/drag range uses [`getScrollRange()`](https://android.googlesource.com/platform/frameworks/base/+/refs/heads/main/core/java/android/widget/ScrollView.java#1036): ```java child.getHeight() - (getHeight() - mPaddingBottom - mPaddingTop) ``` That does include padding, because padding reduces the viewport height. So with short content plus padding-backed `blankSpace`, Android can get into this contradictory state: ```text canScrollVertically(1) -> false actual getScrollRange() -> positive ``` React Native is not the primary source of the bug. `ReactScrollView` adds RN event/state handling, but still delegates interception and touch handling to AOSP `ScrollView`. The package triggers the Android edge case because `KeyboardChatScrollView` uses native padding to model an iOS-like `contentInset`/`blankSpace`. There are a few possible ways to fix this: 1. Patch Android or React Native `ScrollView` so `canScrollVertically()` and the actual scroll range agree when padding is used as scrollable inset. This would be the most fundamental fix, but it is outside this package. 2. Own a custom Android `ReactScrollView` subclass and override the relevant scrollability/range methods. This is more correct in isolation, but it is a large maintenance burden because we would need to preserve React Native ScrollView props, events, commands, refs, Paper/Fabric behavior, and Reanimated compatibility. 3. Represent `blankSpace` as real content layout, for example with a spacer/footer or `contentContainerStyle.paddingBottom`. This makes Android’s range calculations agree naturally, but it changes content layout semantics and can interfere with virtualized lists (it changes layout, JS update may overwrite it etc., not really "safe" way of doing this) 4. Keep the current `ClippingScrollView` approach and avoid pathological blank ranges. This PR chooses option 4. The native workaround fixes the short-content gesture issue without forking React Native’s ScrollView or changing user content layout. On top of that, we clamp `blankSpace` in JS so it cannot exceed one ScrollView viewport. Otherwise we have this bug: https://github.com/user-attachments/assets/5311381b-76ad-4f0e-95b5-f235741ab754 The JS clamp is needed because values larger than the viewport create a scroll range made mostly or entirely of empty space. That oversized empty range is not useful for the chat use case and can expose Android momentum/range desynchronization during fast gestures. One viewport of `blankSpace` is enough to make short content scrollable and position it correctly; larger values only allow scrolling to a fully blank screen: #1497 Closes #1497 #1453 #1455 ## 📢 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 --> ## 📢 Changelog ### JS - clamp `KeyboardChatScrollView` `blankSpace` to the ScrollView viewport height; - prevent oversized blank scroll ranges that can cause Android momentum flicker; ### Android - adjust `ClippingScrollViewDecoratorView` touch handling to allow short content with padding-backed `blankSpace` to start scrolling from the content area; - keep the temporary content range expansion scoped to the touch dispatch path and restore it immediately after dispatch; ## 🤔 How Has This Been Tested? Tested manually on Pixel 7 Pro (API 36). ## 📸 Screenshots (if appropriate): |Before|After| |-------|-----| |<video src="https://github.com/user-attachments/assets/2c9aabdd-7252-4d23-95a2-9de944ecdafa">|<video src="https://github.com/user-attachments/assets/8e64eefd-55a8-49ce-8f53-ac32d6b36598">| ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
1 parent cac5e2d commit a2f7e7e

2 files changed

Lines changed: 106 additions & 1 deletion

File tree

android/src/main/java/com/reactnativekeyboardcontroller/views/ClippingScrollViewDecoratorView.kt

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package com.reactnativekeyboardcontroller.views
22

33
import android.annotation.SuppressLint
4+
import android.view.MotionEvent
45
import android.view.View
56
import android.view.ViewGroup
67
import android.widget.ScrollView
78
import com.facebook.react.uimanager.ThemedReactContext
89
import com.facebook.react.views.view.ReactViewGroup
910
import com.reactnativekeyboardcontroller.extensions.px
11+
import kotlin.math.max
1012

1113
@SuppressLint("ViewConstructor")
1214
class ClippingScrollViewDecoratorView(
@@ -15,13 +17,42 @@ class ClippingScrollViewDecoratorView(
1517
private var insetBottom = 0.0
1618
private var insetTop = 0.0
1719
private var appliedTopInsetPx = 0
20+
private var paddingScrollWorkaroundActive = false
1821

1922
override fun onAttachedToWindow() {
2023
super.onAttachedToWindow()
2124

2225
decorateScrollView()
2326
}
2427

28+
override fun dispatchTouchEvent(event: MotionEvent): Boolean {
29+
val scrollView = findScrollView(this)
30+
31+
if (scrollView == null) {
32+
return super.dispatchTouchEvent(event)
33+
}
34+
35+
if (event.actionMasked == MotionEvent.ACTION_DOWN) {
36+
paddingScrollWorkaroundActive = shouldUsePaddingScrollWorkaround(scrollView, event)
37+
}
38+
39+
val handled =
40+
if (paddingScrollWorkaroundActive) {
41+
dispatchWithExpandedContentRange(scrollView, event)
42+
} else {
43+
super.dispatchTouchEvent(event)
44+
}
45+
46+
if (
47+
event.actionMasked == MotionEvent.ACTION_UP ||
48+
event.actionMasked == MotionEvent.ACTION_CANCEL
49+
) {
50+
paddingScrollWorkaroundActive = false
51+
}
52+
53+
return handled
54+
}
55+
2556
fun setContentInsetBottom(value: Double) {
2657
insetBottom = value
2758
decorateScrollView()
@@ -68,6 +99,65 @@ class ClippingScrollViewDecoratorView(
6899
appliedTopInsetPx = newTopInsetPx
69100
}
70101

102+
private fun shouldUsePaddingScrollWorkaround(
103+
scrollView: ScrollView,
104+
event: MotionEvent,
105+
): Boolean {
106+
val contentView = scrollView.getChildAt(0) ?: return false
107+
val viewportHeight =
108+
scrollView.height - scrollView.paddingTop - scrollView.paddingBottom
109+
val paddingCreatesScrollRange = contentView.height > viewportHeight
110+
111+
return scrollView.scrollY == 0 &&
112+
paddingCreatesScrollRange &&
113+
!scrollView.canScrollVertically(1) &&
114+
isTouchInScrollContent(scrollView, event)
115+
}
116+
117+
private fun dispatchWithExpandedContentRange(
118+
scrollView: ScrollView,
119+
event: MotionEvent,
120+
): Boolean {
121+
val contentView = scrollView.getChildAt(0)
122+
val originalBottom = contentView?.bottom ?: 0
123+
val expandedBottom =
124+
max(originalBottom, scrollView.height + scrollView.scrollY + MIN_SCROLL_RANGE_PX)
125+
126+
if (contentView == null || expandedBottom == originalBottom) {
127+
return super.dispatchTouchEvent(event)
128+
}
129+
130+
return try {
131+
contentView.bottom = expandedBottom
132+
super.dispatchTouchEvent(event)
133+
} finally {
134+
contentView.bottom = originalBottom
135+
}
136+
}
137+
138+
private fun isTouchInScrollContent(
139+
scrollView: ScrollView,
140+
event: MotionEvent,
141+
): Boolean {
142+
val contentView = scrollView.getChildAt(0) ?: return false
143+
val thisLocation = IntArray(COORDINATES_SIZE)
144+
val scrollViewLocation = IntArray(COORDINATES_SIZE)
145+
146+
getLocationOnScreen(thisLocation)
147+
scrollView.getLocationOnScreen(scrollViewLocation)
148+
149+
val x = event.x + thisLocation[0] - scrollViewLocation[0]
150+
val y = event.y + thisLocation[1] - scrollViewLocation[1]
151+
val scrollY = scrollView.scrollY
152+
153+
return !(
154+
y < contentView.top - scrollY ||
155+
y >= contentView.bottom - scrollY ||
156+
x < contentView.left ||
157+
x >= contentView.right
158+
)
159+
}
160+
71161
private fun findScrollView(view: View?): ScrollView? {
72162
var result: ScrollView? = null
73163

@@ -83,4 +173,9 @@ class ClippingScrollViewDecoratorView(
83173

84174
return result
85175
}
176+
177+
companion object {
178+
private const val COORDINATES_SIZE = 2
179+
private const val MIN_SCROLL_RANGE_PX = 2
180+
}
86181
}

src/components/KeyboardChatScrollView/index.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,18 @@ const KeyboardChatScrollView = forwardRef<
8888
onEndVisible,
8989
});
9090

91+
// intentionally clamp `blankSpace` at one ScrollView viewport. The Android
92+
// `ClippingScrollView` workaround temporarily substitutes padding/range
93+
// during touch handling, and oversized blank ranges can de-sync during fast
94+
// momentum gestures. One viewport is enough for the short-content case;
95+
// larger values only allow scrolling to a fully blank screen which is not
96+
// the use case for this library. If you found this comment and it causes
97+
// a bug for you, please open an issue.
9198
const totalPadding = useDerivedValue(() =>
92-
Math.max(blankSpace.value, padding.value + extraContentPadding.value),
99+
Math.min(
100+
layout.value.height,
101+
Math.max(blankSpace.value, padding.value + extraContentPadding.value),
102+
),
93103
);
94104

95105
// Scroll indicator inset = keyboard + extraContentPadding (excludes blankSpace).

0 commit comments

Comments
 (0)