search

PedroBern / react-native-collapsible-tab-view

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

perf: use snapToOffsets instead of custom snap code #11

Closed PedroBern closed 3 years ago

PedroBern commented 3 years ago

related to #9

BREAKING CHANGE: remove snapThreshold prop

github-actions[bot] commented 3 years ago

The Expo app for the example from this branch is ready!

expo.io/@pedrobern/react-native-collapsible-tab-view-demos?release-channel=pr-11

andreialecu commented 3 years ago

Hey @PedroBern, I took a look at this and it seems the reason it works with minHeight: 0 is because minHeight is being set to 1012, which is too big, by the change in https://github.com/PedroBern/react-native-collapsible-tab-view/commit/98f7b7e4f44e25f9c225d29782c8d55fabdbc83f#diff-9cbdb93af845f8d4c83a1783ce090969769880a9f8f4f9fecf504ae01f1055e3R56

The bug is introduced by that commit, and seems to also happen without the changes in this PR (visible on main).

PS: instead of using Dimensions I think it is preferrable to use the useWindowDimensions() hook: https://reactnative.dev/docs/usewindowdimensions just in case the dimensions update somehow (maybe by rotating the screen)

andreialecu commented 3 years ago

One thing that isn't great with snapToOffsets is that the initial momentum scrolling when swiping down seems to be stopped at the snap point, so it needs to be scrolled twice to get further down than the initial point.

For some reason this doesn't seem to happen on the iOS simulator on my end, but just on a real device.

andreialecu commented 3 years ago

I was able to fix this with the current snapping code in #12 - which I prefer, because of the behavior in the comment above.

PedroBern commented 3 years ago

Hey @PedroBern, I took a look at this and it seems the reason it works with minHeight: 0 is because minHeight is being set to 1012, which is too big, by the change in 98f7b7e#diff-9cbdb93af845f8d4c83a1783ce090969769880a9f8f4f9fecf504ae01f1055e3R56

The bug is introduced by that commit, and seems to also happen without the changes in this PR (visible on main).

PS: instead of using Dimensions I think it is preferrable to use the useWindowDimensions() hook: https://reactnative.dev/docs/usewindowdimensions just in case the dimensions update somehow (maybe by rotating the screen)

@andreialecu Instead of fixing it (#8), I'm adding an example of how to solve it #13

PedroBern commented 3 years ago

closing in favor of #12