Fix extraContentPadding not adjusting scroll when it changes by large amount#1371
Conversation
trcoffman
force-pushed
the
fix/extraContentPadding
branch
from
b182104 to
d9c4087
Compare
📊 Package size report
|
On Android, scrollTo executes as an immediate native method call while animatedProps inset updates go through Fabric's async pipeline. When extraContentPadding changes, the scrollTo fires against the old (smaller) scrollable range, so the native ScrollView clamps the target and the scroll correction is lost. This is not an issue for keyboard animations because onStart (which sets the inset via padding.value) fires in an earlier frame than onMove (which calls scrollTo), giving the Fabric commit time to land. For extraContentPadding, both the inset update and scrollTo are triggered by the same shared value change in the same worklet frame. Fix: wrap the scrollTo fallback path in requestAnimationFrame so it executes one frame after the animatedProps inset commit.
trcoffman
force-pushed
the
fix/extraContentPadding
branch
from
d9c4087 to
8545cf7
Compare
kirillzyusko
added
🐛 bug
|
I left only one comment! Other than that the PR looks solid and I think we can merge it! |
trcoffman
force-pushed
the
fix/extraContentPadding
branch
from
a4c51dd to
b696c63
Compare
…namic require jest.resetModules() was causing React to be reloaded, creating multiple React instances and violating the Rules of Hooks. The tests used top-level jest.mock() which doesn't require module resetting between tests - the mock is already applied globally. The dynamic require pattern was cargo-culted from useChatKeyboard tests which use jest.doMock() and legitimately need jest.resetModules(), but it served no purpose here and was actively harmful.
trcoffman
force-pushed
the
fix/extraContentPadding
branch
from
4bf8ae5 to
836b547
Compare
|
Right now on Android there is a significant delay when input grows vs when scroll happens. I think it's caused by many factors: async events to JS, additional frame wait etc. I know you @trcoffman have some ideas how to improve that, so looking forward to your second PR! |
|
@trcoffman sorry, on iOS I found one bug: Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-03-19.at.10.08.45.movAndroid doesn't have this bug |
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2026-03-19.at.15.13.16.mp4I wasn't able to reproduce this on the AI chat screen. The chat composer on the Chat Kit screen never expands for me. Something is really wrong with this chat composer I think. |
trcoffman
force-pushed
the
fix/extraContentPadding
branch
from
3b579c5 to
a48311b
Compare
| scrollTo(scrollViewRef, 0, target, false); | ||
| }); | ||
| } else { | ||
| scrollTo(scrollViewRef, 0, target, false); |
There was a problem hiding this comment.
For future me - fabric works well. Paper (especially iOS) still have problems when we paste multiline input.
Since this PR resolves issue on fabric I will merge this PR as is. paper arch can be fixed in future versions 🤞
2 participants
📜 Description
If your chat composer changes height by a small amount, such as adding an additional line,
extraContentPaddingworks fine. However, if you paste a large blob of text in, the scroll position adjustment doesn't happen and your content gets occluded by the chat composer.💡 Motivation and Context
It makes
extraContentPaddingwork the way that it should.📢 Changelog
KeyboardChatScrollView: Fix
extraContentPaddingnot adjusting scroll consistently.JS
iOS
Android
🤔 How Has This Been Tested?
Manually tested on an iPhone (iPhone 16 Pro) and Android device (Pixel 7)
📸 Screenshots (if appropriate):
Before
iOS:
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2026-03-16.at.15.41.20.mp4
Android:
138.mp4
After
iOS:
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2026-03-16.at.15.40.50.mp4
Android:
139.mp4
📝 Checklist