Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ReanimatedSwipeable] Multiple bug fixes and improvements #3149

Merged
merged 18 commits into from
Oct 17, 2024
Merged

[ReanimatedSwipeable] Multiple bug fixes and improvements #3149

Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2b93465
fix progress jumps and delay by applying finer precision and adding v…
latekvo Oct 11, 2024
866425d
fix onClose and onWillClose callbacks calling regardless of the row s…
latekvo Oct 11, 2024
bc54f51
Revert "fix onClose and onWillClose callbacks calling regardless of t…
latekvo Oct 11, 2024
2358efc
fix onClose and onWillClose callbacks calling regardless of the row s…
latekvo Oct 11, 2024
3a9088d
improve velocity equation
latekvo Oct 11, 2024
ae9f3a7
fix invalid swipe directions being supplied to close and willClose ca…
latekvo Oct 11, 2024
224468c
fix open and willOpen callbacks supplying reversed swipe direction ar…
latekvo Oct 11, 2024
040c6f1
fix start drag callbacks receiving invalid directions when opening
latekvo Oct 11, 2024
9366060
add documentation for 2 missing callbacks
latekvo Oct 11, 2024
dfe58de
fix drag start callbacks being called every frame
latekvo Oct 11, 2024
933338f
replace optional and with optional chain expressions
latekvo Oct 11, 2024
efc08b4
replace string literals with enumerized values to clarify meaning of …
latekvo Oct 11, 2024
d7ebdce
simplify dispatch calls
latekvo Oct 11, 2024
61d521c
simplify animate row function, cleanup
latekvo Oct 11, 2024
5284f21
move docs change to separate PR
latekvo Oct 11, 2024
d5dcab8
move callback excessive update fix to separate PR
latekvo Oct 11, 2024
df4b647
Merge branch 'main' into @latekvo/fix-reanimated-swipeable-animation-…
latekvo Oct 14, 2024
a329a6d
move enum declaration to top
latekvo Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 68 additions & 45 deletions src/components/ReanimatedSwipeable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,37 +111,45 @@ export interface SwipeableProps
* Called when action panel gets open (either right or left).
*/
onSwipeableOpen?: (
direction: 'left' | 'right',
direction: SwipeDirection.LEFT | SwipeDirection.RIGHT,
swipeable: SwipeableMethods
) => void;

/**
* Called when action panel is closed.
*/
onSwipeableClose?: (
direction: 'left' | 'right',
direction: SwipeDirection.LEFT | SwipeDirection.RIGHT,
swipeable: SwipeableMethods
) => void;

/**
* Called when action panel starts animating on open (either right or left).
*/
onSwipeableWillOpen?: (direction: 'left' | 'right') => void;
onSwipeableWillOpen?: (
direction: SwipeDirection.LEFT | SwipeDirection.RIGHT
) => void;

/**
* Called when action panel starts animating on close.
*/
onSwipeableWillClose?: (direction: 'left' | 'right') => void;
onSwipeableWillClose?: (
direction: SwipeDirection.LEFT | SwipeDirection.RIGHT
) => void;

/**
* Called when action panel starts being shown on dragging to open.
*/
onSwipeableOpenStartDrag?: (direction: 'left' | 'right') => void;
onSwipeableOpenStartDrag?: (
direction: SwipeDirection.LEFT | SwipeDirection.RIGHT
) => void;

/**
* Called when action panel starts being shown on dragging to close.
*/
onSwipeableCloseStartDrag?: (direction: 'left' | 'right') => void;
onSwipeableCloseStartDrag?: (
direction: SwipeDirection.LEFT | SwipeDirection.RIGHT
) => void;

/**
* `progress`: Equals `0` when `swipeable` is closed, `1` when `swipeable` is opened.
Expand Down Expand Up @@ -197,6 +205,11 @@ export interface SwipeableMethods {
reset: () => void;
}

enum SwipeDirection {
LEFT = 'left',
RIGHT = 'right',
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd move this declaration at the top of the file (somewhere around SwipeableExcludes type). I know it is JS and it works, but I think it's a good practice. Also if you read code from the top you know what SwipeDirection is (at first I thought it was a number, then I saw the declaration)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in a329a6d

const Swipeable = forwardRef<SwipeableMethods, SwipeableProps>(
function Swipeable(
props: SwipeableProps,
Expand Down Expand Up @@ -274,16 +287,6 @@ const Swipeable = forwardRef<SwipeableMethods, SwipeableProps>(
rightWidth.value = Math.max(0, rowWidth.value - rightOffset.value);
};

const calculateCurrentOffset = useCallback(() => {
'worklet';
updateRightElementWidth();
return rowState.value === 1
? leftWidth.value
: rowState.value === -1
? -rowWidth.value - rightOffset.value!
: 0;
}, [leftWidth, rightOffset, rowState, rowWidth]);

const updateAnimatedEvent = () => {
'worklet';

Expand Down Expand Up @@ -349,27 +352,30 @@ const Swipeable = forwardRef<SwipeableMethods, SwipeableProps>(

const dispatchImmediateEvents = useCallback(
(fromValue: number, toValue: number) => {
if (toValue > 0 && onSwipeableWillOpen) {
onSwipeableWillOpen('left');
} else if (toValue < 0 && onSwipeableWillOpen) {
onSwipeableWillOpen('right');
} else if (onSwipeableWillClose) {
const closingDirection = fromValue > 0 ? 'left' : 'right';
onSwipeableWillClose(closingDirection);
if (toValue > 0) {
onSwipeableWillOpen?.(SwipeDirection.RIGHT);
} else if (toValue < 0) {
onSwipeableWillOpen?.(SwipeDirection.LEFT);
} else {
onSwipeableWillClose?.(
fromValue > 0 ? SwipeDirection.LEFT : SwipeDirection.RIGHT
);
}
},
[onSwipeableWillClose, onSwipeableWillOpen]
);

const dispatchEndEvents = useCallback(
(fromValue: number, toValue: number) => {
if (toValue > 0 && onSwipeableOpen) {
onSwipeableOpen('left', swipeableMethods.current);
} else if (toValue < 0 && onSwipeableOpen) {
onSwipeableOpen('right', swipeableMethods.current);
} else if (onSwipeableClose) {
const closingDirection = fromValue > 0 ? 'left' : 'right';
onSwipeableClose(closingDirection, swipeableMethods.current);
if (toValue > 0) {
onSwipeableOpen?.(SwipeDirection.RIGHT, swipeableMethods.current);
} else if (toValue < 0) {
onSwipeableOpen?.(SwipeDirection.LEFT, swipeableMethods.current);
} else {
onSwipeableClose?.(
fromValue > 0 ? SwipeDirection.LEFT : SwipeDirection.RIGHT,
swipeableMethods.current
);
}
},
[onSwipeableClose, onSwipeableOpen]
Expand All @@ -378,9 +384,8 @@ const Swipeable = forwardRef<SwipeableMethods, SwipeableProps>(
const animationOptionsProp = animationOptions;

const animateRow = useCallback(
(fromValue: number, toValue: number, velocityX?: number) => {
(toValue: number, velocityX?: number) => {
'worklet';
rowState.value = Math.sign(toValue);

const translationSpringConfig = {
duration: 1000,
Expand All @@ -391,17 +396,34 @@ const Swipeable = forwardRef<SwipeableMethods, SwipeableProps>(
...animationOptionsProp,
};

const isClosing = toValue === 0;
const moveToRight = isClosing ? rowState.value < 0 : toValue > 0;

const usedWidth = isClosing
? moveToRight
? rightWidth.value
: leftWidth.value
: moveToRight
? leftWidth.value
: rightWidth.value;

const progressSpringConfig = {
...translationSpringConfig,
velocity: 0,
restDisplacementThreshold: 0.01,
restSpeedThreshold: 0.01,
velocity:
velocityX &&
interpolate(velocityX, [-usedWidth, usedWidth], [-1, 1]),
};

const frozenRowState = rowState.value;

appliedTranslation.value = withSpring(
toValue,
translationSpringConfig,
(isFinished) => {
if (isFinished) {
runOnJS(dispatchEndEvents)(fromValue, toValue);
runOnJS(dispatchEndEvents)(frozenRowState, toValue);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all events executed on the JS thread?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe all of them are, couldn't find any that aren't

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can update the calling logic to check whether the callback is a worklet and run it on the UI thread if it is (that's for another PR though).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea, will do that.

}
}
);
Expand All @@ -417,7 +439,9 @@ const Swipeable = forwardRef<SwipeableMethods, SwipeableProps>(
? withSpring(progressTarget, progressSpringConfig)
: 0;

runOnJS(dispatchImmediateEvents)(fromValue, toValue);
runOnJS(dispatchImmediateEvents)(frozenRowState, toValue);

rowState.value = Math.sign(toValue);
},
[
rowState,
Expand Down Expand Up @@ -447,15 +471,15 @@ const Swipeable = forwardRef<SwipeableMethods, SwipeableProps>(
swipeableMethods.current = {
close() {
'worklet';
animateRow(calculateCurrentOffset(), 0);
animateRow(0);
},
openLeft() {
'worklet';
animateRow(calculateCurrentOffset(), leftWidth.value);
animateRow(leftWidth.value);
},
openRight() {
'worklet';
animateRow(calculateCurrentOffset(), -rightWidth.value);
animateRow(-rightWidth.value);
},
reset() {
'worklet';
Expand Down Expand Up @@ -533,7 +557,6 @@ const Swipeable = forwardRef<SwipeableMethods, SwipeableProps>(
const leftThreshold = leftThresholdProp ?? leftWidth.value / 2;
const rightThreshold = rightThresholdProp ?? rightWidth.value / 2;

const startOffsetX = calculateCurrentOffset() + userDrag.value / friction;
const translationX = (userDrag.value + DRAG_TOSS * velocityX) / friction;

let toValue = 0;
Expand All @@ -556,12 +579,12 @@ const Swipeable = forwardRef<SwipeableMethods, SwipeableProps>(
}
}

animateRow(startOffsetX, toValue, velocityX / friction);
animateRow(toValue, velocityX / friction);
};

const close = () => {
'worklet';
animateRow(calculateCurrentOffset(), 0);
animateRow(0);
};

const tapGesture = Gesture.Tap().onStart(() => {
Expand All @@ -576,12 +599,12 @@ const Swipeable = forwardRef<SwipeableMethods, SwipeableProps>(

const direction =
rowState.value === -1
? 'right'
? SwipeDirection.RIGHT
: rowState.value === 1
? 'left'
? SwipeDirection.LEFT
: event.translationX > 0
? 'left'
: 'right';
? SwipeDirection.RIGHT
: SwipeDirection.LEFT;

if (rowState.value === 0 && onSwipeableOpenStartDrag) {
runOnJS(onSwipeableOpenStartDrag)(direction);
Expand Down
Loading