Skip to content

Commit 8545cf7

Browse files
trcoffmantrcoffman
committed
Defer Android scrollTo in useExtraContentPadding to next frame
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.
1 parent ba42552 commit 8545cf7

8 files changed

Lines changed: 44 additions & 16 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import type Reanimated from "react-native-reanimated";
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+
1315
jest.mock("react-native-reanimated", () => ({
1416
...require("react-native-reanimated/mock"),
1517
scrollTo: (...args: unknown[]) => mockScrollTo(...args),

‎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
});

‎src/components/KeyboardChatScrollView/useExtraContentPadding/__tests__/contentOffsetY.spec.ts‎

Lines changed: 3 additions & 1 deletion
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";
@@ -118,7 +119,7 @@ describe("useExtraContentPadding — contentOffsetY (iOS atomic path)", () => {
118119
expect(mockScrollTo).not.toHaveBeenCalled();
119120
});
120121

121-
it("should fall back to scrollTo when contentOffsetY is undefined", () => {
122+
it("should fall back to scrollTo when contentOffsetY is undefined", async () => {
122123
const render = createRender();
123124

124125
render({
@@ -134,6 +135,7 @@ describe("useExtraContentPadding — contentOffsetY (iOS atomic path)", () => {
134135
});
135136

136137
reactionEffect(20, 0);
138+
await flushRAF();
137139

138140
expect(mockScrollTo).toHaveBeenCalledWith(expect.anything(), 0, 120, false);
139141
});

‎src/components/KeyboardChatScrollView/useExtraContentPadding/__tests__/edge-cases.spec.ts‎

Lines changed: 5 additions & 2 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";
@@ -63,7 +64,7 @@ describe("useExtraContentPadding — edge cases", () => {
6364
expect(mockScrollTo).not.toHaveBeenCalled();
6465
});
6566

66-
it("should clamp to maxScroll (non-inverted)", () => {
67+
it("should clamp to maxScroll (non-inverted)", async () => {
6768
const render = createRender();
6869

6970
render({
@@ -79,6 +80,7 @@ describe("useExtraContentPadding — edge cases", () => {
7980

8081
// delta = 50, scroll + delta = 1540, maxScroll = 2000 - 800 + 300 + 50 = 1550
8182
reactionEffect(50, 0);
83+
await flushRAF();
8284

8385
expect(mockScrollTo).toHaveBeenCalledWith(
8486
expect.anything(),
@@ -88,7 +90,7 @@ describe("useExtraContentPadding — edge cases", () => {
8890
);
8991
});
9092

91-
it("should clamp to -totalPadding (inverted)", () => {
93+
it("should clamp to -totalPadding (inverted)", async () => {
9294
const render = createRender();
9395

9496
render({
@@ -104,6 +106,7 @@ describe("useExtraContentPadding — edge cases", () => {
104106

105107
// delta = 50, target = -280 - 50 = -330, clamp to -350
106108
reactionEffect(50, 0);
109+
await flushRAF();
107110

108111
expect(mockScrollTo).toHaveBeenCalledWith(
109112
expect.anything(),

‎src/components/KeyboardChatScrollView/useExtraContentPadding/__tests__/persistent.spec.ts‎

Lines changed: 5 additions & 2 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 — persistent behavior", () => {
9-
it("should scrollTo on grow", () => {
10+
it("should scrollTo on grow", async () => {
1011
const render = createRender();
1112

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

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

2527
expect(mockScrollTo).toHaveBeenCalledWith(expect.anything(), 0, 120, false);
2628
});
@@ -44,7 +46,7 @@ describe("useExtraContentPadding — persistent behavior", () => {
4446
expect(mockScrollTo).not.toHaveBeenCalled();
4547
});
4648

47-
it("should scrollTo on shrink when at end", () => {
49+
it("should scrollTo on shrink when at end", async () => {
4850
const render = createRender();
4951

5052
render({
@@ -59,6 +61,7 @@ describe("useExtraContentPadding — persistent behavior", () => {
5961
});
6062

6163
reactionEffect(0, 20);
64+
await flushRAF();
6265

6366
expect(mockScrollTo).toHaveBeenCalled();
6467
});

‎src/components/KeyboardChatScrollView/useExtraContentPadding/__tests__/whenAtEnd.spec.ts‎

Lines changed: 5 additions & 2 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 — whenAtEnd behavior", () => {
9-
it("should scrollTo when at end (non-inverted)", () => {
10+
it("should scrollTo when at end (non-inverted)", async () => {
1011
const render = createRender();
1112

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

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

2527
expect(mockScrollTo).toHaveBeenCalled();
2628
});
@@ -44,7 +46,7 @@ describe("useExtraContentPadding — whenAtEnd behavior", () => {
4446
expect(mockScrollTo).not.toHaveBeenCalled();
4547
});
4648

47-
it("should scrollTo when at end (inverted)", () => {
49+
it("should scrollTo when at end (inverted)", async () => {
4850
const render = createRender();
4951

5052
render({
@@ -59,6 +61,7 @@ describe("useExtraContentPadding — whenAtEnd behavior", () => {
5961
});
6062

6163
reactionEffect(20, 0);
64+
await flushRAF();
6265

6366
expect(mockScrollTo).toHaveBeenCalled();
6467
});

‎src/components/KeyboardChatScrollView/useExtraContentPadding/index.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,11 @@ function useExtraContentPadding(options: UseExtraContentPaddingOptions): void {
111111
// eslint-disable-next-line react-compiler/react-compiler
112112
contentOffsetY.value = target;
113113
} else {
114-
scrollTo(scrollViewRef, 0, target, false);
114+
// Defer scrollTo so the animatedProps inset commit lands first;
115+
// otherwise the native ScrollView clamps to the old range.
116+
requestAnimationFrame(() => {
117+
scrollTo(scrollViewRef, 0, target, false);
118+
});
115119
}
116120
} else {
117121
const maxScroll = Math.max(
@@ -123,7 +127,9 @@ function useExtraContentPadding(options: UseExtraContentPaddingOptions): void {
123127
if (contentOffsetY) {
124128
contentOffsetY.value = target;
125129
} else {
126-
scrollTo(scrollViewRef, 0, target, false);
130+
requestAnimationFrame(() => {
131+
scrollTo(scrollViewRef, 0, target, false);
132+
});
127133
}
128134
}
129135
},

0 commit comments

Comments
 (0)