KeyboardChatScrollView: Fix scroll gestures on Android when scrollview content is shorter than the scrollview's viewport#1455
Draft
trcoffman wants to merge 2 commits into
Conversation
Contributor
📊 Package size report
|
7efbe56 to
67f3405
Compare
Rework the Android blank-space / extraContentPadding / keyboard-lift
behavior so it matches iOS. Four separate bugs were stacked on top of
each other; fixing one at a time required fixing the rest.
1. Drag-gesture ignored on short content (the original symptom).
When the conversation was shorter than the viewport but was
scrollable via blankSpace + extraContentPadding, Android's
ScrollView.onInterceptTouchEvent refused to steal drags that
originated inside the message area — only drags starting in the
blank region scrolled. Root cause: short content's child.bottom is
below the viewport, and canScrollVertically(1) at scrollY=0 returns
false (range=child.bottom, extent=ScrollView.getHeight()). The
decorator now extends child.bottom by the applied inset (instead of
extending the scroll range via scrollView.paddingBottom), so
scrollRange > viewport and intercept fires everywhere.
2. Scroll position snapping to 0 mid-stream when the AI reply grew.
Both our decorator and RN's ReactScrollView need to react to content
relayout: RN registers an OnLayoutChangeListener on the content view
(at ReactScrollView.java:1145) whose clamp (line 1275) uses the
current child.getHeight(). Our extension of `child.bottom` is
overwritten by each Yoga relayout, so if RN's listener fires before
we re-apply the extension, the clamp uses the un-extended natural
height and snaps scrollY toward 0 as each reply chunk arrives.
`decorateScrollView()` now removes RN's listener from the content
view and re-adds it after our own so ours runs first, and our
listener re-applies the `child.bottom` extension on every relayout.
3. `isScrollAtEnd` / visibleFraction / max-scroll math all wrong on
Android. On Android, native scroll events report
contentSize.height = contentView.getHeight() = bottom - top, which
now includes the extension — whereas iOS reports pure natural
content size independent of contentInset. All helpers that treated
size.value.height as "natural content" were giving wrong answers.
useChatKeyboard's Android variant now computes
naturalContentHeight = size.value.height - appliedInset (where
appliedInset = max(blankSpace, padding + extraContentPadding)) and
feeds that into isScrollAtEnd, getVisibleMinimumPaddingFraction,
clampScrollIfNeeded, clampedScrollTarget, and the persistent-close
maxScroll. useEndVisible does the same adjustment Platform-gated.
4. Keyboard-lift math for short content was wildly wrong. Two sub-bugs:
a) onStart used a binary-gate `visibleFraction >= 1` to decide
whether to absorb blankSpace into the keyboard. For short content
blankSpace extends past the viewport so fraction is never 1, and
we'd shift by the full keyboard height (pushing content off the
top). Switched to proportional absorption matching iOS's per-
frame math: absorbed = max(0, visibleFraction * blankSpace -
extraContentPadding).
b) onMove used a different formula than onStart:
(blankSpace - extraContent) * fraction
versus onStart's
fraction * blankSpace - extraContent.
The two only agree when fraction == 1; at fraction=0.5 the onMove
formula subtracted only half of extraContent worth of composer
reservation, so the shift was ~(1 - fraction) * extraContent too
small and the composer clipped the last message's timestamp.
Replaced minimumPaddingFractionOnOpen with a visiblePaddingOnOpen
snapshot so onMove uses the iOS formula with the open-time
visiblePadding.
Also fixed two smaller bugs that fell out of the above:
- Non-inverted onMove did not skip `e.duration === -1` events.
When the user dragged the scroll view with the keyboard open,
Android emitted snap-back keyboard frames that ran our scrollTo and
overrode the in-progress drag — on finger-up the scroll position
snapped back to where the drag started. Now guarded, matching the
inverted path.
- persistent-close used `keepAt = offsetBefore + padding.value`,
which scrolled to the full-keyboard-height target even though the
actual open shift was smaller (blankSpace absorbed part of the
keyboard). When closing, content would jump UP instead of staying in
place. Now uses offsetBefore + actualOpenShift — the real clamped
shift captured at the end of the open animation.
67f3405 to
644ed8f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
Fixes #1453
Reworks the Android blank-space /
extraContentPadding/ keyboard-liftbehavior in
KeyboardChatScrollViewso it matches iOS. Four separate bugswere stacked on top of each other; fixing one at a time required fixing
the rest.
💡 Motivation and Context
The original symptom was that on Android, a drag gesture started inside
the message area was ignored when the conversation was shorter than the
viewport — only drags starting inside the blank-space region scrolled.
Investigating that uncovered a chain of related bugs in the Android
blank-space path: mid-stream snap-to-0 as AI replies grew, wrong
isScrollAtEnd/visibleFraction/ max-scroll math, and wildly wrongkeyboard-lift math on short content. The fixes bring Android's behavior
in line with the iOS per-frame model, where
contentInset.bottomcleanlyseparates the applied inset from the natural content size.
📢 Changelog
JS
useChatKeyboard: introduced anaturalContentHeight()helper onAndroid to subtract the applied inset (
max(blankSpace, padding + extraContentPadding)) fromsize.value.height. Android reportscontentSizeascontentView.getHeight()which now includes thedecorator's extension, whereas iOS reports natural content size
independent of
contentInset. Fed the adjusted value intoisScrollAtEnd,clampScrollIfNeeded,getVisibleMinimumPaddingFraction,clampedScrollTarget, and thepersistent-closemaxScroll.useChatKeyboard: replaced thevisibleFraction >= 1binary gate inonStartwith proportional absorption matching iOS:absorbed = max(0, visibleFraction * blankSpace - extraContentPadding).Fixes short content being pushed off the top when the keyboard opened
(blankSpace extends past the viewport on short content, so the
fraction is never 1 and the gate was always skipping absorption).
useChatKeyboard: replacedminimumPaddingFractionOnOpenwith avisiblePaddingOnOpensnapshot soonMoveuses the same iOS formulaas
onStart(max(0, visiblePadding - extraContent)instead of(blankSpace - extraContent) * fraction). The two formulas onlyagreed at
fraction === 1; at partial fractions the composer clippedthe last message's timestamp.
useChatKeyboard: non-invertedonMovenow skipse.duration === -1snap-back events (matching the inverted path), so dragging the scroll
view with the keyboard open no longer snaps the scroll position back
on finger-up.
useChatKeyboard:persistent-close now usesoffsetBefore + actualOpenShiftinstead ofoffsetBefore + padding.value, so closing the keyboard doesn't jumpcontent up when blankSpace absorbed part of the keyboard height.
useEndVisible: added anappliedInsetoption and a Platform-gatednatural-content-height adjustment so
isScrollAtEndreports correctlyon Android.
KeyboardChatScrollView: moved thetotalPaddingderived value aboveuseEndVisibleand threaded it through asappliedInset.ScrollViewWithBottomPadding: minor comment cleanup clarifying thatcontentInset/scrollIndicatorInsetsare iOS-only andcontentInsetBottom/contentInsetTopare the AndroidKeyboardControllerScrollViewprops.Android
ClippingScrollViewDecoratorView: the scroll-range extension nowextends
contentView.bottomdirectly (by the applied inset) insteadof using
scrollView.paddingBottom. Base AndroidScrollView.computeVerticalScrollRange()derives fromchild.getBottom()and ignores padding, sopaddingBottomalonedidn't make
canScrollVertically(1)return true inside the messagearea on short content — drags started there weren't intercepted.
Extending
child.bottomfixes that.ClippingScrollViewDecoratorView: installs anOnLayoutChangeListeneron the content view that re-applies thechild.bottomextension on every Yoga relayout, and re-orders RN'sown listener to fire after ours (RN's listener clamps
scrollYagainst the current
child.getHeight()atReactScrollView.java:1275— if it ran first it would use the un-extended natural height and
snap
scrollYtoward 0 as each AI reply chunk arrived). The listeneris cleaned up in
onDetachedFromWindow.iOS
existing per-frame model (see "JS" above).
🤔 How Has This Been Tested?
📸 Screenshots (if appropriate):
📝 Checklist