Skip to content

Commit 5ab2011

Browse files
authored
fix: rewrite onTextChanged to onSelectionChanged event handler in KeyboardAwareScrollView (#546)
## 📜 Description Rewrite `onTextChanged` to `onSelectionChanged` event handler in `KeyboardAwareScrollView`. ## 💡 Motivation and Context This is the first PR that re-writes `KeyboardAwareScrollView` handlers to `onSelectionChanged` as a main driver. The current problem with `onTextChanged`/`onLayoutChanged` events is that we don't know the coordinates of the caret, and in certain cases (if view is full screen height) it may lead to unpredictable changes. So in this PR I remove `onTextChanged` handler and replace it with `onSelectionChanged`. I fixed e2e tests and overall have a confidence about these changes 😊 ## 📢 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 --> ### E2E - update assets; - update selection code to make it more robust; ### JS - use `onSelectionChanged` instead of `onTextChanged`; ## 🤔 How Has This Been Tested? Tested in e2e tests. ## 📸 Screenshots (if appropriate): |Before|After| |-------|-----| |![image](https://github.com/user-attachments/assets/d5cc152f-4e2d-42d3-a93e-a60891a2bec6)|![image](https://github.com/user-attachments/assets/4fb89683-496b-4174-8b98-4aba8a04a3bd)| ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
1 parent 27fce1c commit 5ab2011

9 files changed

Lines changed: 56 additions & 23 deletions

File tree

e2e/kit/004-aware-scroll-view.e2e.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { expectBitmapsToBeEqual } from "./asserts";
22
import {
33
Env,
44
scrollUpUntilElementIsBarelyVisible,
5+
selectText,
56
tap,
67
typeText,
78
waitAndReplace,
@@ -78,7 +79,7 @@ describe("AwareScrollView test cases", () => {
7879
"aware_scroll_view_container",
7980
"TextInput#4",
8081
);
81-
await element(by.id("TextInput#4")).multiTap(2);
82+
await selectText("TextInput#4");
8283
await waitForExpect(async () => {
8384
await expectBitmapsToBeEqual(
8485
"AwareScrollViewTextSelectionChanged",
@@ -89,7 +90,7 @@ describe("AwareScrollView test cases", () => {
8990
});
9091

9192
it("should auto-scroll when user types a text", async () => {
92-
await element(by.id("aware_scroll_view_container")).scroll(80, "up");
93+
await element(by.id("aware_scroll_view_container")).scroll(40, "up");
9394
await typeText("TextInput#4", "1");
9495
await waitForExpect(async () => {
9596
await expectBitmapsToBeEqual(
6.94 KB
Loading
-28 Bytes
Loading
1.65 KB
Loading
73 Bytes
Loading
5.64 KB
Loading
7.2 KB
Loading

e2e/kit/helpers/actions/index.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,27 @@ export const scrollUpUntilElementIsBarelyVisible = async (
145145
await element(by.id(elementId)).tap({ x: 0, y: 25 });
146146
}
147147
} catch (e) {
148-
await element(by.id(scrollViewId)).scroll(35, "down", 0.01, 0.5);
148+
await element(by.id(scrollViewId)).scroll(50, "down", 0.01, 0.5);
149149
break;
150150
}
151151
}
152152
};
153153

154+
export const selectText = async (id: string) => {
155+
console.debug(
156+
"---------------------------------\n",
157+
"Select text with id:",
158+
colors.magenta(id),
159+
);
160+
161+
if (device.getPlatform() === "ios") {
162+
// on Android multiTap sometimes may not work properly
163+
await element(by.id(id)).multiTap(2);
164+
} else {
165+
await element(by.id(id)).longPress();
166+
}
167+
};
168+
154169
export const closeKeyboard = async (textInputId: string) => {
155170
if (device.getPlatform() === "android") {
156171
await device.pressBack();

src/components/KeyboardAwareScrollView/index.tsx

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import React, { forwardRef, useCallback, useEffect, useMemo } from "react";
22
import Reanimated, {
3+
clamp,
34
interpolate,
45
runOnUI,
56
scrollTo,
@@ -127,6 +128,8 @@ const KeyboardAwareScrollView = forwardRef<
127128
const scrollBeforeKeyboardMovement = useSharedValue(0);
128129
const { input } = useReanimatedFocusedInput();
129130
const layout = useSharedValue<FocusedInputLayoutChangedEvent | null>(null);
131+
const lastSelection =
132+
useSharedValue<FocusedInputSelectionChangedEvent | null>(null);
130133

131134
const { height } = useWindowDimensions();
132135

@@ -223,7 +226,7 @@ const KeyboardAwareScrollView = forwardRef<
223226
);
224227

225228
const scrollFromCurrentPosition = useCallback(
226-
(customHeight?: number) => {
229+
(customHeight: number) => {
227230
"worklet";
228231

229232
const prevScrollPosition = scrollPosition.value;
@@ -238,7 +241,9 @@ const KeyboardAwareScrollView = forwardRef<
238241
...input.value,
239242
layout: {
240243
...input.value.layout,
241-
height: customHeight ?? input.value.layout.height,
244+
// when we have multiline input with limited amount of lines, then custom height can be very big
245+
// so we clamp it to max input height
246+
height: clamp(customHeight, 0, input.value.layout.height),
242247
},
243248
};
244249
scrollPosition.value = position.value;
@@ -248,39 +253,51 @@ const KeyboardAwareScrollView = forwardRef<
248253
},
249254
[maybeScroll],
250255
);
251-
const onChangeText = useCallback(() => {
252-
"worklet";
253-
254-
// if typing a text caused layout shift, then we need to ignore this handler
255-
// because this event will be handled in `useAnimatedReaction` below
256-
if (layout.value?.layout.height !== input.value?.layout.height) {
257-
return;
258-
}
259-
260-
scrollFromCurrentPosition();
261-
}, [scrollFromCurrentPosition]);
262-
const onSelectionChange = useCallback(
263-
(e: FocusedInputSelectionChangedEvent) => {
256+
const onChangeText = useCallback(
257+
(customHeight: number) => {
264258
"worklet";
265259

266-
if (e.selection.start.position !== e.selection.end.position) {
267-
scrollFromCurrentPosition(e.selection.end.y);
260+
// if typing a text caused layout shift, then we need to ignore this handler
261+
// because this event will be handled in `useAnimatedReaction` below
262+
if (layout.value?.layout.height !== input.value?.layout.height) {
263+
return;
268264
}
265+
266+
scrollFromCurrentPosition(customHeight);
269267
},
270268
[scrollFromCurrentPosition],
271269
);
272-
273270
const onChangeTextHandler = useMemo(
274271
() => debounce(onChangeText, 200),
275272
[onChangeText],
276273
);
274+
const onSelectionChange = useCallback(
275+
(e: FocusedInputSelectionChangedEvent) => {
276+
"worklet";
277+
278+
const lastTarget = lastSelection.value?.target;
279+
280+
lastSelection.value = e;
281+
282+
if (e.target !== lastTarget) {
283+
// ignore this event, because "focus changed" event handled in `useSmoothKeyboardHandler`
284+
return;
285+
}
286+
287+
if (e.selection.start.position !== e.selection.end.position) {
288+
return scrollFromCurrentPosition(e.selection.end.y);
289+
}
290+
291+
onChangeTextHandler(e.selection.end.y);
292+
},
293+
[scrollFromCurrentPosition, onChangeTextHandler],
294+
);
277295

278296
useFocusedInputHandler(
279297
{
280-
onChangeText: onChangeTextHandler,
281298
onSelectionChange: onSelectionChange,
282299
},
283-
[onChangeTextHandler, onSelectionChange],
300+
[onSelectionChange],
284301
);
285302

286303
useSmoothKeyboardHandler(

0 commit comments

Comments
 (0)