Skip to content

Commit

Permalink
feat: replaced onScrollBegin with onScrollStart
Browse files Browse the repository at this point in the history
The onScrollBegin event is triggered only when a finger touches down, which isn't consistent with
the onScrollEnd events. The correct approach would be to use the onScrollStart callback, ensuring
that both callbacks are invoked at the start and end of a scroll respectively.

BREAKING CHANGE: The onScrollBegin property has been deprecated. Please use onScrollStart as its
replacement.

fix #506
  • Loading branch information
dohooo committed Jan 13, 2024
1 parent 139d5e5 commit b654f43
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 49 deletions.
5 changes: 5 additions & 0 deletions .changeset/slow-donkeys-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'react-native-reanimated-carousel': patch
---

Replaced 'onScrollBegin' with 'onScrollStart' to better match gesture callback.
3 changes: 3 additions & 0 deletions example/app/src/pages/normal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ function Index() {
autoPlay={isAutoPlay}
autoPlayInterval={isFast ? 100 : 2000}
data={data}
onScrollStart={()=>{console.log('===1')}}
onScrollEnd={()=>{console.log('===2')}}

onConfigurePanGesture={g => g.enabled(false)}
pagingEnabled={isPagingEnabled}
onSnapToItem={index => console.log("current index:", index)}
Expand Down
2 changes: 1 addition & 1 deletion example/app/src/pages/press-swipe/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function Index() {
style={{ width: PAGE_WIDTH, height: 240 }}
width={PAGE_WIDTH}
data={[...ImageItems, ...ImageItems]}
onScrollBegin={() => {
onScrollStart={() => {
pressAnim.value = withTiming(1);
}}
onScrollEnd={() => {
Expand Down
2 changes: 1 addition & 1 deletion example/website/pages/Examples/press-swipe.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function Index() {
style={{ width: PAGE_WIDTH, height: 240 }}
width={PAGE_WIDTH}
data={[...ImageItems, ...ImageItems]}
onScrollBegin={() => {
onScrollStart={() => {
pressAnim.value = withTiming(1);
}}
onScrollEnd={() => {
Expand Down
4 changes: 2 additions & 2 deletions example/website/pages/props.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,9 @@ Callback fired when navigating to an item
| ------ | ------- | -------- |
| \(index: number\) => void | - ||

### `onScrollBegin`
### `onScrollStart`

Callback fired when scroll begin
Callback fired when scroll start

| type | default | required |
| ------ | ------- | -------- |
Expand Down
12 changes: 6 additions & 6 deletions src/components/Carousel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const Carousel = React.forwardRef<ICarouselInstance, TCarouselProps<any>>(
renderItem,
onScrollEnd,
onSnapToItem,
onScrollBegin,
onScrollStart,
onProgressChange,
customAnimation,
defaultIndex,
Expand Down Expand Up @@ -86,7 +86,7 @@ const Carousel = React.forwardRef<ICarouselInstance, TCarouselProps<any>>(
fixedDirection,
duration: scrollAnimationDuration,
onScrollEnd: () => runOnJS(_onScrollEnd)(),
onScrollBegin: () => !!onScrollBegin && runOnJS(onScrollBegin)(),
onScrollStart: () => !!onScrollStart && runOnJS(onScrollStart)(),
});

const { next, prev, scrollTo, getSharedIndex, getCurrentIndex }
Expand Down Expand Up @@ -123,10 +123,10 @@ const Carousel = React.forwardRef<ICarouselInstance, TCarouselProps<any>>(
onScrollEnd,
]);

const scrollViewGestureOnScrollBegin = React.useCallback(() => {
const scrollViewGestureOnScrollStart = React.useCallback(() => {
pauseAutoPlay();
onScrollBegin?.();
}, [onScrollBegin, pauseAutoPlay]);
onScrollStart?.();
}, [onScrollStart, pauseAutoPlay]);

const scrollViewGestureOnScrollEnd = React.useCallback(() => {
startAutoPlay();
Expand Down Expand Up @@ -173,7 +173,7 @@ const Carousel = React.forwardRef<ICarouselInstance, TCarouselProps<any>>(
: styles.itemsHorizontal,
]}
testID={testID}
onScrollBegin={scrollViewGestureOnScrollBegin}
onScrollStart={scrollViewGestureOnScrollStart}
onScrollEnd={scrollViewGestureOnScrollEnd}
onTouchBegin={scrollViewGestureOnTouchBegin}
onTouchEnd={scrollViewGestureOnTouchEnd}
Expand Down
12 changes: 6 additions & 6 deletions src/components/ScrollViewGesture.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ interface Props {
infinite?: boolean
testID?: string
style?: StyleProp<ViewStyle>
onScrollBegin?: () => void
onScrollStart?: () => void
onScrollEnd?: () => void
onTouchBegin?: () => void
onTouchEnd?: () => void
Expand Down Expand Up @@ -55,7 +55,7 @@ const IScrollViewGesture: React.FC<PropsWithChildren<Props>> = (props) => {
translation,
testID,
style = {},
onScrollBegin,
onScrollStart,
onScrollEnd,
onTouchBegin,
onTouchEnd,
Expand Down Expand Up @@ -259,12 +259,12 @@ const IScrollViewGesture: React.FC<PropsWithChildren<Props>> = (props) => {
return translation;
}

const onGestureBegin = useCallback((_: PanGestureHandlerEventPayload) => {
const onGestureStart = useCallback((_: PanGestureHandlerEventPayload) => {
"worklet";

touching.value = true;
validStart.value = true;
onScrollBegin && runOnJS(onScrollBegin)();
onScrollStart && runOnJS(onScrollStart)();

max.value = (maxPage - 1) * size;
if (!loop && !overscrollEnabled)
Expand All @@ -282,7 +282,7 @@ const IScrollViewGesture: React.FC<PropsWithChildren<Props>> = (props) => {
translation,
overscrollEnabled,
getLimit,
onScrollBegin,
onScrollStart,
]);

const onGestureUpdate = useCallback((e: PanGestureHandlerEventPayload) => {
Expand Down Expand Up @@ -380,7 +380,7 @@ const IScrollViewGesture: React.FC<PropsWithChildren<Props>> = (props) => {

const gesture = usePanGestureProxy({
onConfigurePanGesture,
onGestureBegin,
onGestureStart,
onGestureUpdate,
onGestureEnd,
options: { enabled },
Expand Down
18 changes: 9 additions & 9 deletions src/hooks/useCarouselController.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ interface IOpts {
fixedDirection?: TCarouselProps["fixedDirection"]
duration?: number
defaultIndex?: number
onScrollBegin?: () => void
onScrollStart?: () => void
onScrollEnd?: () => void
}

Expand Down Expand Up @@ -138,8 +138,8 @@ export function useCarouselController(options: IOpts): ICarouselController {
options.onScrollEnd?.();
}, [options]);

const onScrollBegin = React.useCallback(() => {
options.onScrollBegin?.();
const onScrollStart = React.useCallback(() => {
options.onScrollStart?.();
}, [options]);

const scrollWithTiming = React.useCallback(
Expand Down Expand Up @@ -173,7 +173,7 @@ export function useCarouselController(options: IOpts): ICarouselController {
if (!canSliding() || (!loop && index.value >= dataInfo.length - 1))
return;

onScrollBegin?.();
onScrollStart?.();

const nextPage = currentFixedPage() + count;
index.value = nextPage;
Expand All @@ -194,7 +194,7 @@ export function useCarouselController(options: IOpts): ICarouselController {
loop,
index,
dataInfo,
onScrollBegin,
onScrollStart,
handlerOffset,
size,
scrollWithTiming,
Expand All @@ -207,7 +207,7 @@ export function useCarouselController(options: IOpts): ICarouselController {
const { count = 1, animated = true, onFinished } = opts;
if (!canSliding() || (!loop && index.value <= 0)) return;

onScrollBegin?.();
onScrollStart?.();

const prevPage = currentFixedPage() - count;
index.value = prevPage;
Expand All @@ -227,7 +227,7 @@ export function useCarouselController(options: IOpts): ICarouselController {
canSliding,
loop,
index,
onScrollBegin,
onScrollStart,
handlerOffset,
size,
scrollWithTiming,
Expand All @@ -241,7 +241,7 @@ export function useCarouselController(options: IOpts): ICarouselController {
if (i === index.value) return;
if (!canSliding()) return;

onScrollBegin?.();
onScrollStart?.();
// direction -> 1 | -1
const direction = handlerOffsetDirection(handlerOffset, fixedDirection);

Expand Down Expand Up @@ -283,7 +283,7 @@ export function useCarouselController(options: IOpts): ICarouselController {
handlerOffset,
dataInfo.length,
canSliding,
onScrollBegin,
onScrollStart,
scrollWithTiming,
],
);
Expand Down
13 changes: 6 additions & 7 deletions src/hooks/usePanGestureProxy.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,13 @@ describe("Using RNGH v2 gesture API", () => {
onConfigurePanGesture: (gesture: PanGesture) => {
// This is user's customizations
gesture
.onStart(treatStartAsUpdate ? handlers.active : handlers.start)
.onBegin(handlersFromUser.begin)
.onUpdate(handlersFromUser.active)
.onEnd(handlersFromUser.end)
.onFinalize(handlers.finish)
.withTestId("pan");
},
onGestureBegin: handlers.begin,
onGestureStart: treatStartAsUpdate ? handlers.active : handlers.start,
onGestureUpdate: handlers.active,
onGestureEnd: handlers.end,
options: { enabled: true },
Expand Down Expand Up @@ -82,10 +81,11 @@ describe("Using RNGH v2 gesture API", () => {
const pan = usePanGestureProxy({
onConfigurePanGesture: (_: PanGesture) => {
_
.onBegin(panHandlers.begin)
.onFinalize(panHandlers.finish)
.withTestId("pan");
},
onGestureBegin: panHandlers.begin,
onGestureStart: panHandlers.start,
onGestureUpdate: panHandlers.active,
onGestureEnd: panHandlers.end,
options: { enabled: true },
Expand Down Expand Up @@ -153,14 +153,13 @@ describe("Event list validation", () => {
const pan = usePanGestureProxy({
onConfigurePanGesture: (_: PanGesture) => {
_
.onStart(treatStartAsUpdate ? handlers.active : handlers.start)
.onBegin(handlersFromUser.begin)
.onUpdate(handlersFromUser.active)
.onEnd(handlersFromUser.end)
.onFinalize(handlers.finish)
.withTestId("pan");
},
onGestureBegin: handlers.begin,
onGestureStart: treatStartAsUpdate ? handlers.active : handlers.start,
onGestureUpdate: handlers.active,
onGestureEnd: handlers.end,
options: { enabled: true },
Expand Down Expand Up @@ -233,11 +232,11 @@ describe("Filling event list with defaults", () => {
const pan = usePanGestureProxy({
onConfigurePanGesture: (_: PanGesture) => {
_
.onStart(treatStartAsUpdate ? handlers.active : handlers.start)
.onBegin(handlers.begin)
.onFinalize(handlers.finish)
.withTestId("pan");
},
onGestureBegin: handlers.begin,
onGestureStart: treatStartAsUpdate ? handlers.active : handlers.start,
onGestureUpdate: handlers.active,
onGestureEnd: handlers.end,
options: { enabled: true },
Expand Down
30 changes: 15 additions & 15 deletions src/hooks/usePanGestureProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ import { useUpdateGestureConfig } from "./useUpdateGestureConfig";
export const usePanGestureProxy = (
customization: {
onConfigurePanGesture?: (gesture: PanGesture) => void
onGestureBegin: (event: GestureStateChangeEvent<PanGestureHandlerEventPayload>) => void
onGestureStart: (event: GestureStateChangeEvent<PanGestureHandlerEventPayload>) => void
onGestureUpdate: (event: GestureUpdateEvent<PanGestureHandlerEventPayload>) => void
onGestureEnd: (event: GestureStateChangeEvent<PanGestureHandlerEventPayload>, success: boolean) => void
options?: GestureConfig
},
) => {
const {
onConfigurePanGesture,
onGestureBegin,
onGestureStart,
onGestureUpdate,
onGestureEnd,
options = {},
Expand All @@ -27,25 +27,25 @@ export const usePanGestureProxy = (

// Save the original gesture callbacks
const originalGestures = {
onBegin: gesture.onBegin,
onStart: gesture.onStart,
onUpdate: gesture.onUpdate,
onEnd: gesture.onEnd,
};

// Save the user defined gesture callbacks
const userDefinedConflictGestures: {
onBegin?: Parameters<(typeof gesture)["onBegin"]>[0]
onStart?: Parameters<(typeof gesture)["onStart"]>[0]
onUpdate?: Parameters<(typeof gesture)["onUpdate"]>[0]
onEnd?: Parameters<(typeof gesture)["onEnd"]>[0]
} = {
onBegin: undefined,
onStart: undefined,
onUpdate: undefined,
onEnd: undefined,
};

const fakeOnBegin: typeof gesture.onBegin = (cb) => {
// Using fakeOnBegin to save the user defined callback
userDefinedConflictGestures.onBegin = cb;
const fakeOnStart: typeof gesture.onStart = (cb) => {
// Using fakeOnStart to save the user defined callback
userDefinedConflictGestures.onStart = cb;
return gesture;
};

Expand All @@ -62,7 +62,7 @@ export const usePanGestureProxy = (
};

// Setup the fake callbacks
gesture.onBegin = fakeOnBegin;
gesture.onStart = fakeOnStart;
gesture.onUpdate = fakeOnUpdate;
gesture.onEnd = fakeOnEnd;

Expand All @@ -71,17 +71,17 @@ export const usePanGestureProxy = (
onConfigurePanGesture(gesture);

// Restore the original callbacks
gesture.onBegin = originalGestures.onBegin;
gesture.onStart = originalGestures.onStart;
gesture.onUpdate = originalGestures.onUpdate;
gesture.onEnd = originalGestures.onEnd;

// Setup the original callbacks with the user defined callbacks
gesture
.onBegin((e) => {
onGestureBegin(e);
.onStart((e) => {
onGestureStart(e);

if (userDefinedConflictGestures.onBegin)
userDefinedConflictGestures.onBegin(e);
if (userDefinedConflictGestures.onStart)
userDefinedConflictGestures.onStart(e);
})
.onUpdate((e) => {
onGestureUpdate(e);
Expand All @@ -98,7 +98,7 @@ export const usePanGestureProxy = (

return gesture;
}, [
onGestureBegin,
onGestureStart,
onGestureUpdate,
onGestureEnd,
onConfigurePanGesture,
Expand Down
4 changes: 2 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,9 @@ export type TCarouselProps<T = any> = {
*/
onSnapToItem?: (index: number) => void
/**
* On scroll begin
* On scroll start
*/
onScrollBegin?: () => void
onScrollStart?: () => void
/**
* On scroll end
*/
Expand Down

0 comments on commit b654f43

Please sign in to comment.