Make FlingHandler use velocity as the activation metric.

[latekvo]

Description

Fixed fling component handler to activate based on fling velocity instead of fling distance.
The fling also no longer has to be performed from start to finish entirely within the component's bounds.
This feature was tested on both emulated and live android phones.

Test plan

• clone this branch
• run yarn
• go to example
• copy fling example from the docs into the example/src/empty/EmptyExample.tsx file

Web

• run yarn start

Android

• go to example/android
• run yarn start

IOS

• IOS remains unchanged, it already implements this mechanism in a different way.

Reference

Screen.Recording.2024-03-08.at.14.42.42.mov

[m-bert]

Good job! 😄

There are some things that we should change. Since both implementations are identical, I've left comments with explanations only in .ts files.

The fling also no longer has to be performed from start to finish entirely within the component's bounds.

I'm not sure if that's what we actually want 😅 Handlers have property called shouldCancelWhenOutside, so behavior of moving out of the component's bounds should match this property, cc @j-piasecki.

Also please change name of this PR and in Web section you can change yarn start to yarn web

[j-piasecki]

I'm not sure if that's what we actually want 😅 Handlers have property called shouldCancelWhenOutside, so behavior of moving out of the component's bounds should match this property

I haven't looked at the code yet, but is there a reason why we cannot have both behaviors depending on shouldCancelWhenOutside?

If we can only choose one behavior I'd argue that, especially in the case of fling, having it not cancel when moving outside the view would be more desirable but having both would be great.

[j-piasecki]

Soon...

Most of what I wrote also applies to the web version, but I didn't want to copy&paste it.

[m-bert]

This time I won't leave comments on android side because implementations are identical.

[m-bert]

Looks good! Left some comments, this time only cosmetic things 😄

[m-bert]

It looks much better now! Please change name of this PR and make sure that CI passes 😄

[latekvo]

Give me one minute, i forgot to rewrite the JS vector as well.

[j-piasecki]

Looks good, besides the little thing with VelocityTracker!

I have one more thing: right now the the vectors are considered aligned when the angle between them is less than ~41 degrees. It works great, but when setting the direction of UP | RIGHT (or any other 2 adjacent directions) it leaves a cone of ~18 degrees where the gesture doesn't activate. I think we can change the DEFAULT_MIN_DIRECTION_ALIGNMENT to 0.707 which would mean considering two vectors aligned when the angle between them is less than 45.008 degrees, so there would not be a no-gesture-recognized cone in-between. What do you think?

Ideally, we would want to reduce the cone for a particular direction to like 60-45 deg and expand them when 2 adjacent directions are set. We may also consider going in the direction of iOS, which does a weird thing of rotating the cone (I think? cc. @m-bert) when adjacent directions are set. That's something up for discussion though, and definitely for another PR.

[latekvo]

Looks good, besides the little thing with VelocityTracker!

I have one more thing: right now the the vectors are considered aligned when the angle between them is less than ~41 degrees. It works great, but when setting the direction of UP | RIGHT (or any other 2 adjacent directions) it leaves a cone of ~18 degrees where the gesture doesn't activate. I think we can change the DEFAULT_MIN_DIRECTION_ALIGNMENT to 0.707 which would mean considering two vectors aligned when the angle between them is less than 45.008 degrees, so there would not be a no-gesture-recognized cone in-between. What do you think?

Ideally, we would want to reduce the cone for a particular direction to like 60-45 deg and expand them when 2 adjacent directions are set. We may also consider going in the direction of iOS, which does a weird thing of rotating the cone (I think? cc. m-bert) when adjacent directions are set. That's something up for discussion though, and definitely for another PR.

I believe when user enables more than 1 direction, it is very likely they intend them to serve separate functions.
We're still waiting for @m-bert 's opinion, and his approve on changes.

[m-bert]

I don't think that trying to mimic iOS behavior is a good idea. We will have some cross-platform inconsistency, but still we will achieve something what I believe can be called "expected behavior" on web and android.

I think we can change the DEFAULT_MIN_DIRECTION_ALIGNMENT to 0.707 which would mean considering two vectors aligned when the angle between them is less than 45.008 degrees, so there would not be a no-gesture-recognized cone in-between.

I agree with that 👍

Ideally, we would want to reduce the cone for a particular direction to like 60-45 deg and expand them when 2 adjacent directions are set.

If we consider expanding the cone, 45 deg sounds a bit too much for me. In that case I would set it to ~30 deg for one direction and expand it when we set adjacent directions. We can discuss it next time though.

[latekvo]

Alright, I'm increasing the constant to 0.86 (30 deg), and adding angle expansion for adjacent enabled directions.
(we're moving this issue to a different PR)

[m-bert]

LGTM 😄 I've left one last comment, but overall it looks fine. Thanks for your hard work ❤️

@j-piasecki, I'd like to know your opinion about magnitude getter before we merge it 😄

[m-bert]