octopitus / rn-sliding-up-panel

Draggable sliding up panel implemented in React Native https://octopitus.github.io/rn-sliding-up-panel/
MIT License
928 stars 157 forks source link

Snapping points and momentum events #110

Closed octopitus closed 5 years ago

octopitus commented 5 years ago

Too lazy to create separated PRs 🤦‍♂️

Published under next tag.

yarn add rn-sliding-up-panel@next

TODO:

Usage:

<SlidingUpPanel
  snappingPoints={[160, 480]} // Must be an increment array and values must be within top & bottom of draggableRange
  onMomentumDragStart={(value) => {}}
  onMomentumDragEnd={(value) => {}}
  // Other props
>
  // ...
</SlidingUpPanel>

Thanks to @Eralyne, @epurban, @invig and some others for suggesting and reporting the issues.

epurban commented 5 years ago

Currently when trying to use snappingPoints I get error Undefined is not a function (near '...this._flick.predictNextPosition...')

in _onPanResponderRelease of SlidingUpPanel. Trying to find a solution on my own rn

Edit: clearing my expo cache fixed the issue :)

epurban commented 5 years ago

Alright , so snappingPoints works as intended, but the panel still fails to reach the snapping points. Its because onDragEnd is called and in my function I change the state, to change what is rendered in the panel. It seems to have a negative effect on the animation and keeps it from reaching the snapping points.

So I think when the state changes while the _flick has started, it causes it to not reach its target.

octopitus commented 5 years ago

Does the setState change draggableRange or any props passing to the panel?

epurban commented 5 years ago

Does the setState change draggableRange or any props passing to the panel?

No, it only changes booleans in the state that determine what is rendered in the panel. It doesn't change anything passed into the panel or the draggableRange! :(

sebqq commented 5 years ago

@octopitus if I change draggableRange dynamically and then call show() method (after draggableRange state change is done inside setState callback), sliding up animation isn't visible, so it looks like the height is just changed without sliding animation. Any suggestions?

// edit: My bad 🤦‍♂️solved using snapping point 👍

sebqq commented 5 years ago

@octopitus point to @next version

When I close the the screen (where sliding panel is rendered) with sliding-panel opened (I have panelRef.hide() method called in componentWillUnmount()) the red box is shown. So in the end I cannot hide sliding-panel when screen is destroyed and panel will be opened when I launch the screen second time.

Screenshot 2019-04-15 at 10 15 58 Screenshot 2019-04-15 at 10 16 49

The red screen seems to appear only when the panel is in "shown" state. If it is hidden, no red screen will appear

Screenshot 2019-04-15 at 10 29 45

Maybe something like initialDraggableRange prop should be a solution to enable users to hide panel at initial render. Then no custom componentWillUnmount() should be needed.

octopitus commented 5 years ago

Thanks @sinodko test this branch out. For the "red screen", it should be fixed in v2.2.0-rc3. You can update the package by using the same command above (yarn add rn-sliding-up-panel@next).

For your real problem, if you want to reset the animation to initial state, I recommend you implement it your own way. Like this:

this._animatedValue = new Animated.Value(0)

componentWillUnmount() {
  this._animatedValue.resetAnimation()
}

render() {
  return (
    <SlidingUpPanel
      animatedValue={this._animatedValue}
      // Other props
    />
  )
}

I can add it into the panel but it may affect other people. For some reasons they don't want to reset the panel to its initial position. Also, you should wait for the animation to be finished (use InteractionManager) before doing anything else. For example, if you want to navigate back to previous screen after the panel has been hidden:

onPressNavigateBack() {
  this._panel.hide() // _panel is a ref to SlidingUpPanel component

  InteractionManager. runAfterInteractions(() => {
    // Call navigator's goBack() here
  })
}
sebqq commented 5 years ago

@octopitus rc3 looks promising! DraggableRange object updating now works flawlessly with animation so I don't even need snappingPoints :D

this._animatedValue = new Animated.Value(0)

componentWillUnmount() {
  this._animatedValue.resetAnimation()
}

render() {
  return (
    <SlidingUpPanel
      animatedValue={this._animatedValue}
      // Other props
    />
  )
}

this fragment also works for hiding the panel! In next few days I will test the branch on android and ios devices and I will let you know if I find something else :) Thanks @octopitus 🥇 you're great!

samiede commented 5 years ago

Hey guys! Two issues with the @latest: How can I prevent the panel from hooking to the hardwareBackButton press in the first place? Using the prop to overwrite the behaviour with () => {} doesn't seem to work. I need to change the screen without the panel being hidden first. Additionally, it does not seem to unmount the backButtonHandler until the panel hides. When I change screens, the App doesn't react to the back button press once until I press it again.

Additionally, I have problems with the anchor points. The panel sometimes does not completely move to the lowest anchor point if the user doesn't use enough velocity when flicking downwards. Is there something I can do about that?

Best wishes!

octopitus commented 5 years ago

@samiede For the hardware back button, have you tried the onBackButtonPress prop? It should return true to tell the panel that "I'll take over this, you don't need to do anything". Ex:

<SlidingUpPanel onBackButtonPress={() => true}>
// Children
</SlidingUpPanel>

For the other issue, I need a few more time to investigate. It quite similar to @epurban problem.

samiede commented 5 years ago

I have, but then it deactivates the back button altogether. I need the usual behaviour, switching screens etc. :)

octopitus commented 5 years ago

Hmm. This due to the fact that the back handler inside component returns true if the onBackButtonPress is defined (But it should return whatever the onBackButtonPress returns).

https://github.com/octopitus/rn-sliding-up-panel/blob/6680c50124b93d8564bd2572c78e23e48a4a3d89/SlidingUpPanel.js#L327-L329

samiede commented 5 years ago

In our app, I have simply removed the backButtonHandler and patched the library with patch-package, so that works for me :)

I've also decreased the TIME_CONSTANT to make the animation more snappy. This would be nice to have as prop as well! :)

octopitus commented 5 years ago

Should be fixed in 2.2.0-rc4. Now if the onBackButtonPress returns false, it will propagate the event to upper listeners.

samiede commented 5 years ago

Still not working for me with the back button, nothing happens at all. I've tried a console log in the _onBackButtonPress but it doesn't seem to trigger at all:


_onBackButtonPress() {
    if (this.props.onBackButtonPress !== null) {
      return this.props.onBackButtonPress()
    }

    console.log('Back Pressed')

    const value = this.props.animatedValue.__getValue()

    if (this._isAtBottom(value)) {
      return false
    }

    this.hide()

    return true
  }
octopitus commented 5 years ago

Wonder how did you use the onBackButtonPress prop? It should be:

<SlidingUpPanel onBackButtonPress={() => false}>
// ...
</SlidingUpPanel>
samiede commented 5 years ago

I'm not sure if I did it this way, I can try it again though. Right now, as I said, I have just removed all mentions of the backButton and also changed the time constant, which makes the animation feel a little more snappy. I still have the problem with the bottom position not snapping to the correct value if the panel is released too close to the position, unfortunately. It seems like the initial velocity is too low to counter the friction.

samiede commented 5 years ago

What do you think about adding TIME_CONSTANT as a prop so the speed of the snapping animation to the snapping points can be controlled? For our app, releasing the finger feels a little bit too floaty, so I'd have to adjust it using patch-package, but it would be a nice prop!