react-native-gesture-handler icon indicating copy to clipboard operation
react-native-gesture-handler copied to clipboard

Wrong fire onSwipeableWillOpen

Open dmytro-kupriianov opened this issue 4 years ago • 7 comments

Method onSwipeableWillOpen fired only after open panel, but not when start movement. How fix this moment?

dmytro-kupriianov avatar Jan 28 '20 09:01 dmytro-kupriianov

Having the same issue. I would have expected that this event is called on start. How this differ from onSwipeableOpen then?

MoOx avatar Oct 28 '20 10:10 MoOx

The idea in #1101 could be a solution if changing onSwipeableWillOpen is not an option (eg: undesired breaking change).

MoOx avatar Oct 28 '20 10:10 MoOx

#999 should make this possible now.

jakub-gonet avatar Nov 24 '20 15:11 jakub-gonet

Not really as discussed in #1102. #999 is for Drawer only as we are here discussing about Swipeable.

MoOx avatar Nov 24 '20 15:11 MoOx

Btw, what is the difference between onSwipeableWillOpen and onSwipeableOpen. If there is a good reason, I think new lifecycle method are necessary. If there is no good reasons (= not really a bug but still is?) it should be changed, but would be a breaking change. Otherwise, we could add new lifecycle (onSwipeStart+End) as onSwipeableWillOpen might be legit for programmatic opening as you mentioned in https://github.com/software-mansion/react-native-gesture-handler/pull/1102#issuecomment-733060255. I am open for a documented PR if we agree on something :)

MoOx avatar Nov 24 '20 15:11 MoOx

Btw, what is the difference between onSwipeableWillOpen and onSwipeableOpen.

I'd want to know it myself. We can check it in the code but that's not the point – having will/did and plain lifecycle methods is confusing to the developers using this component.

Currently, we have the following description in docs:

onSwipeableOpen

method that is called when action panel gets open (either right or left).

onSwipeableWillOpen

method that is called when action panel starts animating on open (either right or left).

I imagine this is confusing to the new developers using RNGH. onSwipeableWillOpen gets called before opening but at the start of the animation? onSwipeableOpen gets called when opened swipeable or in the same time when will variant? What about people who use TS as their documentation?

Otherwise, we could add new lifecycle (onSwipeStart+End) as onSwipeableWillOpen might be legit for programmatic opening as you mentioned in #1102 (comment).

For me, there's little sense to create another lifecycle method; we added will/did, left/right prefix and it effectively makes the number of lifecycle methods eightfold. Adding onSwipeStart/End makes situation worse.

Ideally, it'd be only one/two callbacks:

  • onSwipeableOpen taking a function with two args: (direction, animating)
  • onSwipeableOpen similar to one above

Or:

  • onSwipeableStateChange taking function with (direction, animating, isOpen)

We could add another method similar to #999 providing slide percentage as well(have you almost finished swiping?).

But we have to work with what we have so I think adding something that was mentioned in #1101 could be beneficial (you can just make a guard for other positions: if(position !== 0) { return; } // (...). It'd be a bit less efficient than the dedicated state change method but it should be good enough. Then we can add new methods and deprecate older so functionality stays the same and people will have time to transition between them.

jakub-gonet avatar Nov 24 '20 16:11 jakub-gonet

onSwipeableWillOpen is incorrectly documented. Currently (I just tried to redo the same thing I did on another project without my patch/hack from another PR) and onSwipeableWillOpen fire when:

  • you have the thing opened
  • AND when you release your finger Then, a few MS later, when animation is totally ended, onSwipeableOpen fires.

Currently I am trying to implement the dumbest thing possible for swipeable: being able to have a row with open options. Just a single row. This means, as soon as you start swiping another row, the opened row closes. It's a behaviour you can find on all iOS swipeable list in mail, messages etc. Super classic thing. Without a patch with a new lifecycle method in Swipeable, this is just impossible to call some code when a swipe is started by the user, even if that's just a few pixels.

I am open to any clean solution that allow this trivial behaviour to be implemented.

MoOx avatar Dec 29 '20 18:12 MoOx