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.

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

fix: pass correct params to animateToPosition #610

Merged
merged 2 commits into from
Aug 30, 2021

Conversation

vonovak
Copy link
Contributor

@vonovak vonovak commented Aug 30, 2021

Motivation

animateToPosition accepts:

export type AnimateToPositionType = (
  position: number,
  source: ANIMATION_SOURCE,
  velocity?: number,
  configs?: Animated.WithTimingConfig | Animated.WithSpringConfig
) => void;

it was incorrectly called like this animateToPosition(context.initialPosition, velocityY / 2); - so velocityY / 2 was used in place of ANIMATION_SOURCE which is an enum.

According to this SO answer, TS does not distinguish between numbers and numeric Enums, which is why the error came to be in the first place. Let me know if you wanna change the enums to strings, I can add that to this PR (or open a new one).

There is one more change in src/hooks/useGestureEventsHandlersDefault.tsx - in this block I removed the first condtion and slightly simplified the second.

In the original code, if you look at the second condition you can see it is the same condition as the first one, just relaxed by the context.initialPosition === highestSnapPoint part. That means the second condition is a "superset" of the first and so the first can be removed (I hope I'm not wrong here :D).

if (
  (type === GESTURE_SOURCE.SCROLLABLE
    ? animatedScrollableContentOffsetY.value
    : 0) > 0 &&
  context.initialPosition === highestSnapPoint &&
  animatedPosition.value === highestSnapPoint
) {
  return;
}

/**
 * if gesture started by scrollable dragging the sheet than continue scrolling,
 * then exit the method to prevent snapping.
 */
if (
  type === GESTURE_SOURCE.SCROLLABLE &&
  animatedScrollableContentOffsetY.value > 0 &&
  animatedPosition.value === highestSnapPoint
) {
  return;
}

Thanks!

@vonovak vonovak marked this pull request as ready for review August 30, 2021 17:27
@gorhom gorhom added the v4 Written in Reanimated v2 label Aug 30, 2021
Copy link
Owner

@gorhom gorhom left a comment

Choose a reason for hiding this comment

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

everything looks fine, except this removed line

context.initialPosition === highestSnapPoint &&

src/hooks/useGestureEventsHandlersDefault.tsx Show resolved Hide resolved
@gorhom
Copy link
Owner

gorhom commented Aug 30, 2021

thanks @vonovak for spotting this issue 👏

@gorhom
Copy link
Owner

gorhom commented Aug 30, 2021

@all-contributors please add @vonovak for code

@allcontributors
Copy link
Contributor

@gorhom

I've put up a pull request to add @vonovak! 🎉

@gorhom gorhom merged commit 01883fb into gorhom:master Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v4 Written in Reanimated v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants