Skip to content

Commit 73d066d

Browse files
notsuhasdohooo
andauthored
fix: non-loop scrollTo offset regression (#891)
* fix: correct non-loop scrollTo offset regression * chore: add changeset for non-loop scrollTo fix * test: cover animated non-loop backward scrollTo * test: cover loop branches for codecov --------- Co-authored-by: Caspian Zhao <caspian.zhao@outlook.com>
1 parent 5289d9d commit 73d066d

3 files changed

Lines changed: 122 additions & 31 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"react-native-reanimated-carousel": patch
3+
---
4+
5+
- Fix non-loop `scrollTo()` so backward jumps keep the correct negative offset instead of briefly rendering a blank frame.
6+
- Add regression tests for non-loop backward `scrollTo()` and returning to index `0`.

src/hooks/useCarouselController.test.tsx

Lines changed: 93 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,76 @@ describe("useCarouselController", () => {
228228
expect(mockHandlerOffset.value).toBe(-900); // size * 3
229229
});
230230

231+
it("should keep negative offsets when scrollTo() moves backward in non-loop mode", () => {
232+
const { result } = renderHook(
233+
() =>
234+
useCarouselController({
235+
...defaultProps,
236+
loop: false,
237+
}),
238+
{ wrapper }
239+
);
240+
241+
act(() => {
242+
result.current.scrollTo({ index: 3, animated: false });
243+
});
244+
expect(mockHandlerOffset.value).toBe(-900);
245+
246+
act(() => {
247+
result.current.scrollTo({ index: 2, animated: false });
248+
});
249+
expect(mockHandlerOffset.value).toBe(-600);
250+
251+
act(() => {
252+
result.current.scrollTo({ index: 1, animated: false });
253+
});
254+
expect(mockHandlerOffset.value).toBe(-300);
255+
});
256+
257+
it("should keep negative offsets when animated backward scrollTo() runs in non-loop mode", () => {
258+
const onFinished = jest.fn();
259+
const { result } = renderHook(
260+
() =>
261+
useCarouselController({
262+
...defaultProps,
263+
loop: false,
264+
}),
265+
{ wrapper }
266+
);
267+
268+
act(() => {
269+
result.current.scrollTo({ index: 3, animated: true });
270+
});
271+
expect(mockHandlerOffset.value).toBe(-900);
272+
273+
act(() => {
274+
result.current.scrollTo({ index: 2, animated: true, onFinished });
275+
});
276+
expect(mockHandlerOffset.value).toBe(-600);
277+
expect(onFinished).toHaveBeenCalled();
278+
});
279+
280+
it("should map scrollTo({ index: 0 }) to zero offset from end in non-loop mode", () => {
281+
const { result } = renderHook(
282+
() =>
283+
useCarouselController({
284+
...defaultProps,
285+
loop: false,
286+
}),
287+
{ wrapper }
288+
);
289+
290+
act(() => {
291+
result.current.scrollTo({ index: 4, animated: false });
292+
});
293+
expect(mockHandlerOffset.value).toBe(-1200);
294+
295+
act(() => {
296+
result.current.scrollTo({ index: 0, animated: false });
297+
});
298+
expect(Math.abs(mockHandlerOffset.value)).toBe(0);
299+
});
300+
231301
it("should handle animation callbacks", () => {
232302
const onFinished = jest.fn();
233303
const { result } = renderHook(() => useCarouselController(defaultProps), { wrapper });
@@ -1004,13 +1074,31 @@ describe("useCarouselController edge cases and uncovered lines", () => {
10041074
expect(typeof currentIndex).toBe("number");
10051075
});
10061076

1007-
it("should handle fixed direction in scrollTo", () => {
1077+
it("should handle positive fixed direction in scrollTo", () => {
1078+
const { result } = renderHook(
1079+
() =>
1080+
useCarouselController({
1081+
...defaultProps,
1082+
loop: true,
1083+
fixedDirection: "positive",
1084+
}),
1085+
{ wrapper }
1086+
);
1087+
1088+
act(() => {
1089+
result.current.scrollTo({ index: 3, animated: false });
1090+
});
1091+
1092+
expect(mockHandlerOffset.value).toBe(900); // size * 3
1093+
});
1094+
1095+
it("should handle negative fixed direction in scrollTo", () => {
10081096
const { result } = renderHook(
10091097
() =>
10101098
useCarouselController({
10111099
...defaultProps,
10121100
loop: true,
1013-
fixedDirection: 1,
1101+
fixedDirection: "negative",
10141102
}),
10151103
{ wrapper }
10161104
);
@@ -1022,7 +1110,7 @@ describe("useCarouselController edge cases and uncovered lines", () => {
10221110
expect(mockHandlerOffset.value).toBe(-900); // size * 3
10231111
});
10241112

1025-
it("should handle complex loop calculations in scrollTo", () => {
1113+
it("should handle scrollTo when loop position is close to next cycle", () => {
10261114
const { result } = renderHook(
10271115
() =>
10281116
useCarouselController({
@@ -1032,14 +1120,13 @@ describe("useCarouselController edge cases and uncovered lines", () => {
10321120
{ wrapper }
10331121
);
10341122

1035-
// Set to a high offset to test loop boundary calculations
1036-
mockHandlerOffset.value = -1800; // 6 * size, beyond data length
1123+
mockHandlerOffset.value = -900; // 3 * size, more than half-way through current cycle
10371124

10381125
act(() => {
10391126
result.current.scrollTo({ index: 1, animated: false });
10401127
});
10411128

1042-
expect(typeof mockHandlerOffset.value).toBe("number");
1129+
expect(mockHandlerOffset.value).toBe(-1800);
10431130
});
10441131

10451132
it("should get shared index correctly", () => {

src/hooks/useCarouselController.tsx

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -352,34 +352,32 @@ export function useCarouselController(options: IOpts): ICarouselController {
352352
if (!canSliding()) return;
353353

354354
onScrollStart?.();
355-
// direction -> 1 | -1
356-
let direction: -1 | 1;
357-
if (fixedDirection === "positive") direction = 1;
358-
else if (fixedDirection === "negative") direction = -1;
359-
else if (!loop) {
360-
const currentPage = currentFixedPage();
361-
direction = i >= currentPage ? -1 : 1;
362-
} else {
363-
direction = handlerOffsetDirection(handlerOffset);
364-
}
365-
366-
// target offset
367-
const offset = i * size * direction;
368-
// page width size * page count
369-
const totalSize = dataInfo.length * size;
370-
371-
let isCloseToNextLoop = false;
355+
let finalOffset: number;
372356

373-
if (loop) {
374-
isCloseToNextLoop = Math.abs(handlerOffset.value % totalSize) / totalSize >= 0.5;
357+
if (!loop) {
358+
// Non-loop mode always uses direct page-to-offset mapping.
359+
// This avoids sign flips when moving to lower indices.
360+
finalOffset = -i * size;
361+
} else {
362+
// direction -> 1 | -1
363+
let direction: -1 | 1;
364+
if (fixedDirection === "positive") direction = 1;
365+
else if (fixedDirection === "negative") direction = -1;
366+
else direction = handlerOffsetDirection(handlerOffset);
367+
368+
// target offset
369+
const offset = i * size * direction;
370+
// page width size * page count
371+
const totalSize = dataInfo.length * size;
372+
const isCloseToNextLoop = Math.abs(handlerOffset.value % totalSize) / totalSize >= 0.5;
373+
374+
finalOffset =
375+
(Math.floor(Math.abs(handlerOffset.value / totalSize)) + (isCloseToNextLoop ? 1 : 0)) *
376+
totalSize *
377+
direction +
378+
offset;
375379
}
376380

377-
const finalOffset =
378-
(Math.floor(Math.abs(handlerOffset.value / totalSize)) + (isCloseToNextLoop ? 1 : 0)) *
379-
totalSize *
380-
direction +
381-
offset;
382-
383381
if (animated) {
384382
index.value = i;
385383
handlerOffset.value = scrollWithTiming(finalOffset, onFinished);

0 commit comments

Comments
 (0)