PedroBern / react-native-collapsible-tab-view

A cross-platform Collapsible Tab View component for React Native
MIT License
836 stars 162 forks source link

fix: ios snap behaviour #65

Closed andreialecu closed 3 years ago

andreialecu commented 3 years ago

On iOS, onMomentumEnd is not called unless the user is scrolling with momentum.

This ensures that the snap logic is called properly when momentum scrolling is not in use.

Additionally, it reenables overscroll/bouncing the scroll view.

Fixes #59

andreialecu commented 3 years ago

Note that I didn't test Android at all, so please test before merging.

Also in the DiffClamp + Snap Example, it seems to snap over the article tab covering it. But I'm not sure if this is a bug introduced by this PR or not. It works fine on the last tab where there's more content:

Recording: https://streamable.com/gqfbbd

andreialecu commented 3 years ago

Longer video that shows things working (besides #66): https://streamable.com/nez289

andreialecu commented 3 years ago

A comment on 05ea6af (#65):

https://docs.swmansion.com/react-native-reanimated/docs/next/api/useAnimatedReaction/

I believe using the empty array fixed some small jittering in the diffclamp demo.

I noticed there are other usages of useAnimatedReaction that don't define a dependency array. Those may need optimizing too (in a separate PR)

PedroBern commented 3 years ago

Also in the DiffClamp + Snap Example, it seems to snap over the article tab covering it. But I'm not sure if this is a bug introduced by this PR or not. It works fine on the last tab where there's more content:

I saw it in other recordings, it's weird, here is fine

I noticed there are other usages of useAnimatedReaction that don't define a dependency array. Those may need optimizing too (in a separate PR)

I will review it

alexpchin commented 3 years ago

Hi, @andreialecu and @PedroBern. I've just pulled this and ran as per https://github.com/PedroBern/react-native-collapsible-tab-view/issues/59#issuecomment-770209467 I can confirm that the Snap is fixed but think we can achieve without setTimeout?

PedroBern commented 3 years ago

closed in favor of #67