Skip to content

Commit b28ef3b

Browse files
Fix extraContentPadding not adjusting scroll when it changes by large amount (#1371)
## 📜 Description If your chat composer changes height by a small amount, such as adding an additional line, `extraContentPadding` works 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 `extraContentPadding` work the way that it should. ## 📢 Changelog KeyboardChatScrollView: Fix `extraContentPadding` not adjusting scroll consistently. ### JS - - ### iOS - Use contentOffset in implementation of extraContentPadding to synchronously adjust scroll position - ### Android - Use requestAnimationFrame to delay scrollTo until next frame after extraContentPadding change gets committed ## 🤔 How Has This Been Tested? Manually tested on an iPhone (iPhone 16 Pro) and Android device (Pixel 7) ## 📸 Screenshots (if appropriate): ### Before iOS: https://github.com/user-attachments/assets/0404e3c5-9dc3-4d55-b3b7-9483b1db2677 Android: https://github.com/user-attachments/assets/b31a2966-033f-4f04-9196-31ecf44d0e17 ### After iOS: https://github.com/user-attachments/assets/188ced9f-08f5-4952-a525-b996ed3673ac Android: https://github.com/user-attachments/assets/e9438991-0414-4743-948d-6e81a6ce044b ## 📝 Checklist - [ ] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed --------- Co-authored-by: kirillzyusko <zyusko.kirik@gmail.com>
1 parent 9acd790 commit b28ef3b

11 files changed

Lines changed: 279 additions & 32 deletions

File tree

FabricExample/src/screens/Examples/KeyboardChatScrollView/index.tsx

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,7 @@ function KeyboardChatScrollViewPlayground() {
4747
const flashRef = useRef<FlashListRef<MessageProps>>(null);
4848
const flatRef = useRef<FlatList<MessageProps>>(null);
4949
const scrollRef = useRef<VirtualizedListScrollViewRef>(null);
50-
const textInputRef = useRef<TextInput>(null);
51-
const textRef = useRef("");
50+
const [text, setText] = useState("");
5251
const [inputHeight, setInputHeight] = useState(TEXT_INPUT_HEIGHT);
5352
const extraContentPadding = useSharedValue(0);
5453
const { inverted, messages, reversedMessages, addMessage, mode } =
@@ -70,20 +69,19 @@ function KeyboardChatScrollViewPlayground() {
7069
},
7170
[extraContentPadding],
7271
);
73-
const onInput = useCallback((text: string) => {
74-
textRef.current = text;
72+
const onInput = useCallback((value: string) => {
73+
setText(value);
7574
}, []);
7675
const onSend = useCallback(() => {
77-
const message = textRef.current.trim();
76+
const message = text.trim();
7877

7978
if (message === "") {
8079
return;
8180
}
8281

8382
addMessage({ text: message, sender: true });
84-
textInputRef.current?.clear();
85-
textRef.current = "";
86-
}, [addMessage]);
83+
setText("");
84+
}, [addMessage, text]);
8785

8886
useEffect(() => {
8987
legendRef.current?.scrollToOffset({
@@ -177,11 +175,11 @@ function KeyboardChatScrollViewPlayground() {
177175
/>
178176
</View>
179177
<TextInput
180-
ref={textInputRef}
181178
multiline
182179
nativeID="chat-input"
183180
style={styles.input}
184181
testID="chat.input"
182+
value={text}
185183
onChangeText={onInput}
186184
onLayout={onInputLayoutChanged}
187185
/>

src/architecture.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const IS_FABRIC = "nativeFabricUIManager" in global;

src/components/KeyboardChatScrollView/index.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ const KeyboardChatScrollView = forwardRef<
6969
scroll,
7070
layout,
7171
size,
72+
contentOffsetY,
7273
inverted,
7374
keyboardLiftBehavior,
7475
freeze,

src/components/KeyboardChatScrollView/useExtraContentPadding/__fixtures__/setup.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,25 @@
11
import { renderHook } from "@testing-library/react-native";
22
import { useAnimatedRef } from "react-native-reanimated";
33

4+
import { useExtraContentPadding } from "..";
45
import { sv } from "../../../../__fixtures__/sv";
56

6-
import type { useExtraContentPadding } from "..";
77
import type { SharedValue } from "react-native-reanimated";
88
import type Reanimated from "react-native-reanimated";
99

1010
export const mockScrollTo = jest.fn();
1111
export let reactionEffect: (current: number, previous: number | null) => void;
1212

13+
export const flushRAF = () => new Promise((resolve) => setTimeout(resolve, 0));
14+
15+
let mockForceLegacy = false;
16+
17+
jest.mock("../../../../architecture", () => ({
18+
get IS_FABRIC() {
19+
return !mockForceLegacy;
20+
},
21+
}));
22+
1323
jest.mock("react-native-reanimated", () => ({
1424
...require("react-native-reanimated/mock"),
1525
scrollTo: (...args: unknown[]) => mockScrollTo(...args),
@@ -31,15 +41,10 @@ type RenderOptions = Omit<
3141

3242
export const createRender = () => {
3343
return function render(options: RenderOptions) {
34-
// eslint-disable-next-line @typescript-eslint/no-var-requires
35-
const mod = require("..") as {
36-
useExtraContentPadding: typeof useExtraContentPadding;
37-
};
38-
3944
return renderHook(() => {
4045
const ref = useAnimatedRef<Reanimated.ScrollView>();
4146

42-
mod.useExtraContentPadding({
47+
useExtraContentPadding({
4348
scrollViewRef: ref,
4449
blankSpace: options.blankSpace ?? sv(0),
4550
...options,
@@ -49,6 +54,11 @@ export const createRender = () => {
4954
};
5055

5156
beforeEach(() => {
52-
jest.resetModules();
57+
mockForceLegacy = false;
5358
mockScrollTo.mockClear();
5459
});
60+
61+
export const withLegacyArch = (fn: () => void) => {
62+
mockForceLegacy = true;
63+
fn();
64+
};

src/components/KeyboardChatScrollView/useExtraContentPadding/__tests__/always.spec.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import { sv } from "../../../../__fixtures__/sv";
22
import {
33
createRender,
4+
flushRAF,
45
mockScrollTo,
56
reactionEffect,
67
} from "../__fixtures__/setup";
78

89
describe("useExtraContentPadding — always behavior", () => {
9-
it("should scrollTo on grow when at end (non-inverted)", () => {
10+
it("should scrollTo on grow when at end (non-inverted)", async () => {
1011
const render = createRender();
1112

1213
render({
@@ -21,6 +22,7 @@ describe("useExtraContentPadding — always behavior", () => {
2122
});
2223

2324
reactionEffect(20, 0);
25+
await flushRAF();
2426

2527
expect(mockScrollTo).toHaveBeenCalledWith(
2628
expect.anything(),
@@ -30,7 +32,7 @@ describe("useExtraContentPadding — always behavior", () => {
3032
);
3133
});
3234

33-
it("should scrollTo on grow when NOT at end (non-inverted)", () => {
35+
it("should scrollTo on grow when NOT at end (non-inverted)", async () => {
3436
const render = createRender();
3537

3638
render({
@@ -45,11 +47,12 @@ describe("useExtraContentPadding — always behavior", () => {
4547
});
4648

4749
reactionEffect(20, 0);
50+
await flushRAF();
4851

4952
expect(mockScrollTo).toHaveBeenCalledWith(expect.anything(), 0, 120, false);
5053
});
5154

52-
it("should scrollTo on shrink (non-inverted)", () => {
55+
it("should scrollTo on shrink (non-inverted)", async () => {
5356
const render = createRender();
5457

5558
render({
@@ -64,6 +67,7 @@ describe("useExtraContentPadding — always behavior", () => {
6467
});
6568

6669
reactionEffect(0, 20);
70+
await flushRAF();
6771

6872
expect(mockScrollTo).toHaveBeenCalledWith(
6973
expect.anything(),
@@ -73,7 +77,7 @@ describe("useExtraContentPadding — always behavior", () => {
7377
);
7478
});
7579

76-
it("should scrollTo on grow (inverted)", () => {
80+
it("should scrollTo on grow (inverted)", async () => {
7781
const render = createRender();
7882

7983
render({
@@ -88,6 +92,7 @@ describe("useExtraContentPadding — always behavior", () => {
8892
});
8993

9094
reactionEffect(20, 0);
95+
await flushRAF();
9196

9297
expect(mockScrollTo).toHaveBeenCalledWith(expect.anything(), 0, -15, false);
9398
});

src/components/KeyboardChatScrollView/useExtraContentPadding/__tests__/blankSpace.spec.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { sv } from "../../../../__fixtures__/sv";
22
import {
33
createRender,
4+
flushRAF,
45
mockScrollTo,
56
reactionEffect,
67
} from "../__fixtures__/setup";
@@ -29,7 +30,7 @@ describe("useExtraContentPadding — blankSpace floor", () => {
2930
expect(mockScrollTo).not.toHaveBeenCalled();
3031
});
3132

32-
it("should scroll by effective delta when blankSpace partially absorbs", () => {
33+
it("should scroll by effective delta when blankSpace partially absorbs", async () => {
3334
const render = createRender();
3435

3536
render({
@@ -50,11 +51,12 @@ describe("useExtraContentPadding — blankSpace floor", () => {
5051
// maxScroll = max(2000 - 800 + 500, 0) = 1700
5152
// target = min(100 + 100, 1700) = 200
5253
reactionEffect(300, 0);
54+
await flushRAF();
5355

5456
expect(mockScrollTo).toHaveBeenCalledWith(expect.anything(), 0, 200, false);
5557
});
5658

57-
it("blankSpace=0 produces identical behavior to default", () => {
59+
it("blankSpace=0 produces identical behavior to default", async () => {
5860
const render = createRender();
5961

6062
render({
@@ -75,6 +77,7 @@ describe("useExtraContentPadding — blankSpace floor", () => {
7577
// maxScroll = max(2000 - 800 + 320, 0) = 1520
7678
// target = min(1200 + 20, 1520) = 1220
7779
reactionEffect(20, 0);
80+
await flushRAF();
7881

7982
expect(mockScrollTo).toHaveBeenCalledWith(
8083
expect.anything(),
@@ -107,7 +110,7 @@ describe("useExtraContentPadding — blankSpace floor", () => {
107110
expect(mockScrollTo).not.toHaveBeenCalled();
108111
});
109112

110-
it("should scroll when change exceeds blankSpace floor (inverted)", () => {
113+
it("should scroll when change exceeds blankSpace floor (inverted)", async () => {
111114
const render = createRender();
112115

113116
render({
@@ -127,6 +130,7 @@ describe("useExtraContentPadding — blankSpace floor", () => {
127130
// effectiveDelta = 100
128131
// target = max(5 - 100, -500) = max(-95, -500) = -95
129132
reactionEffect(200, 0);
133+
await flushRAF();
130134

131135
expect(mockScrollTo).toHaveBeenCalledWith(expect.anything(), 0, -95, false);
132136
});

0 commit comments

Comments
 (0)