Jittery behaviour? #59
Comments
🎉 Hi @alexpchin thanks for running the example. Looks like it's iOS-specific. It's perfect on android here. Can you please do me a favor? Experiment with |
|
@PedroBern Will do! I haven't gotten the whole way through reading the updates yet! You've definitely been busy. I need to experiment with adding a different |
|
@alexpchin thanks! yes quite a work, but was fun! The default tab bar isn't documented yet, but it's easy to add a custom |
|
@alexpchin I added |
|
Just working through integrating into my project which does not use
I am just going through everything at the moment... |
I will add it to the readme
Where? On the quick start code?
This is weird, here is all working 😕 Not sure if I understand it. As the snap works for me, I can't see this bug.
Good question, I didn't test it but should work
I added a ref in #58 to control the back behavior when navigating back. See it here. This ref can be used to programmatically navigate between tabs, like below. It's not documented/released yet. const ref = React.useRef<RefType>()
const goToTab = () => {
ref.current?.setIndex(name)
}
<Tabs.Container ref={ref} {...} /> |
|
@PedroBern Interesting. Snap isn't working for me. Can you uninstall I might be missing something really obvious... I've re-installed |
|
It's correct, I get the same warning, it's because SDK 40 is not supposed to use reanimated v2 (yet) but expo says it's ok I've just done a fresh install, everything is working, including the snap. Does the snap work on android for you? |
|
@PedroBern I just ran the example on the Android emulator and can confirm that the snap does work. Does it work for you on iOS at your end? |
|
I have left some related comments here: #58 (comment) Unfortunately there are a lot of problems on iOS with v3. Going to take a look at the implementation myself today. |
|
Also, just another few thoughts:
|
|
Here are some of the issues I found: Scroll position doesn't sync between tabs: RPReplay_Final1611995320.MP4Snap only works when momentum scrolling rests, but not if you lift the finger while scrolling (zip because it's over the 10 mb limit for videos): RPReplay_Final1611995394.MP4.zip Tapping the tab bar to switch tabs only works when first opening a screen, as soon as you scroll a bit, they don't respond any more: RPReplay_Final1611995710.MP4Elastic scrolling via momentum, or by overscrolling with the finger doesn't work. It just stops abruptly at either edge (example of what I mean here: https://medium.com/@wcandillon/ios-bounce-list-effect-with-react-native-5102e3a83999): RPReplay_Final1611995774.MP4 |
|
On iOS react-native-collapsible-tab-view/src/createCollapsibleTabs.tsx Lines 529 to 531 in d9642e1 It doesn't trigger for touches that drag the view. |
|
Moved to #68 |
|
It appears that most of the issues I mentioned above are related to the The reason is that One way to potentially solve it is to set Also, the overscroll situation seems to be resolved if this is omitted or set to Not yet sure why it was forced to |
diff --git a/src/createCollapsibleTabs.tsx b/src/createCollapsibleTabs.tsx
index 08f97d8..8c1c560 100644
--- a/src/createCollapsibleTabs.tsx
+++ b/src/createCollapsibleTabs.tsx
@@ -15,6 +15,7 @@ import Animated, {
scrollTo,
withTiming,
runOnJS,
+ runOnUI,
} from 'react-native-reanimated'
import MaterialTabBar, {
@@ -509,6 +510,56 @@ const createCollapsibleTabs = <
tabNames.value.findIndex((n) => n === name)
)
+ const onMomentumEnd = () => {
+ 'worklet'
+ if (snapEnabled) {
+ if (diffClampEnabled && accDiffClamp.value > 0) {
+ if (scrollYCurrent.value > headerHeight) {
+ if (accDiffClamp.value <= headerHeight * snapThreshold) {
+ // snap down
+ isSnapping.value = true
+ accDiffClamp.value = withTiming(0, undefined, () => {
+ isSnapping.value = false
+ })
+ } else if (accDiffClamp.value < headerHeight) {
+ // snap up
+ isSnapping.value = true
+ accDiffClamp.value = withTiming(headerHeight, undefined, () => {
+ isSnapping.value = false
+ })
+ }
+ } else {
+ isSnapping.value = true
+ accDiffClamp.value = withTiming(0, undefined, () => {
+ isSnapping.value = false
+ })
+ }
+ } else {
+ if (scrollYCurrent.value <= headerHeight * snapThreshold) {
+ // snap down
+ snappingTo.value = 0
+ // @ts-ignore
+ scrollTo(refMap[name], 0, 0, true)
+ } else if (scrollYCurrent.value <= headerHeight) {
+ // snap up
+ snappingTo.value = headerHeight
+ // @ts-ignore
+ scrollTo(refMap[name], 0, headerHeight, true)
+ }
+ isSnapping.value = false
+ }
+ }
+ isGliding.value = false
+ }
+
+ const maybeGlide = () => {
+ setTimeout(() => {
+ if (isGliding.value !== true) {
+ runOnUI(onMomentumEnd)()
+ }
+ }, 50)
+ }
+
const scrollHandler = useAnimatedScrollHandler(
{
onScroll: (event) => {
@@ -524,53 +575,13 @@ const createCollapsibleTabs = <
isScrolling.value = true
},
onEndDrag: () => {
- isGliding.value = true
isScrolling.value = false
+ runOnJS(maybeGlide)()
},
- onMomentumEnd: () => {
- if (snapEnabled) {
- if (diffClampEnabled && accDiffClamp.value > 0) {
- if (scrollYCurrent.value > headerHeight) {
- if (accDiffClamp.value <= headerHeight * snapThreshold) {
- // snap down
- isSnapping.value = true
- accDiffClamp.value = withTiming(0, undefined, () => {
- isSnapping.value = false
- })
- } else if (accDiffClamp.value < headerHeight) {
- // snap up
- isSnapping.value = true
- accDiffClamp.value = withTiming(
- headerHeight,
- undefined,
- () => {
- isSnapping.value = false
- }
- )
- }
- } else {
- isSnapping.value = true
- accDiffClamp.value = withTiming(0, undefined, () => {
- isSnapping.value = false
- })
- }
- } else {
- if (scrollYCurrent.value <= headerHeight * snapThreshold) {
- // snap down
- snappingTo.value = 0
- // @ts-ignore
- scrollTo(refMap[name], 0, 0, true)
- } else if (scrollYCurrent.value <= headerHeight) {
- // snap up
- snappingTo.value = headerHeight
- // @ts-ignore
- scrollTo(refMap[name], 0, headerHeight, true)
- }
- isSnapping.value = false
- }
- }
- isGliding.value = false
+ onMomentumBegin: () => {
+ isGliding.value = true
},
+ onMomentumEnd,
},
[headerHeight, name, diffClampEnabled, snapEnabled]
)
@@ -677,7 +688,7 @@ const createCollapsibleTabs = <
return (
<Animated.ScrollView
ref={refMap[name] as any}
- bounces={false}
+ //bounces={false}
bouncesZoom={false}
style={[_style, style]}
contentContainerStyle={[This diff seems to fix all of the issues I mentioned above, but it needs some cleanup for the I notice that the |
|
Im not at home right now, when I get there I will iterate over all comments. Just one thought, I saw some issues on reaninated, related to scroll on iOS that were fixed in rc.1 and rc.2, but the example folder is using rc.0 |
|
I opened #65 which fixes all of the issues I reported from my initial testing. @alexpchin if you could test it as well it would be great. I'm not sure about the jittery behaviour, seems to work fine on my physical device (iPhone 12 Pro Max). |
|
Oh, and good job @PedroBern on the rewrite 🏆 . The implementation looks much better now with Reanimated. |
@andreialecu thanks man 👍
@alexpchin better to open a new issue so we can discuss it without noise... But you can disable swiping with |
|
@PedroBern @andreialecu I've gone through and moved unrelated comments to their own issues and updated my comments to reduce noise and make them easier to fix. I will now start looking at them. |
Hi @PedroBern, I've finally gotten around to have a look at the new v3 implementation.
On running the example, I see:
You specifiedonScrollon a <ScrollView> but notscrollEventThrottle. You will only receive one event. Using16you get all the events but be aware that it may cause frame drops, use a bigger number if you don't need as much precision.I don't know if it's related yet, but I'm also seeing quite jittery behaviour. Please see video:
simulator1.mov.zip
I haven't fully investigated this and will add more information if/when I find out more.
The text was updated successfully, but these errors were encountered: