Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/header touchables and horizontal scroll #254

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tiagocorreiaalmeida
Copy link
Contributor

@tiagocorreiaalmeida tiagocorreiaalmeida commented May 2, 2022

The idea of the PR is to allow the header to contain Touchable elements and horizontal views that don't affect the vertical scroll behaviour of the tab view, this means we don't have to manually add pointerEvents to all views that are not target of the touch events, it also means we can have both the behaviours, having the element being target of a touch event as press and still be able to scroll vertically on the same element.

Improvements that need to be made:

  • Changing tab when the header is scrolling is not perfect yet
  • Further test reveal header on scroll and snapping
  • Improve snapping, at the moment the snap only happens at the end of the decay which takes a bit too long in terms of UX, haven't tested it yet, but I believe one solution could be handling the snap logic on the onScrollEnd end event

Also, as mentioned on the feature request, I'm not sure if I missed any of the project internals that could introduce new bugs.

ScrollOnHeader.mp4

@tiagocorreiaalmeida
Copy link
Contributor Author

Hey @andreialecu @PedroBern, any update on this? If the approach doesn't feel the best taking into consideration the project design or if there's anything else that can be done to improve it, let me know

@andreialecu
Copy link
Collaborator

This doesn't currently handle gliding when the finger is released, correct?

I did think about this implementation in the past, but it felt a bit hacky because of missing gliding and momentum support.

I wonder if we can instead provide some extension points that allows users to handle this in their own app instead of in the library.

@tiagocorreiaalmeida
Copy link
Contributor Author

@andreialecu Is the discord still up? Maybe we can go trough it there

@Synapse96
Copy link

Synapse96 commented Sep 3, 2022

any chance this PR would be resolved soon? Would love to have this working as well

@tobzy
Copy link

tobzy commented Oct 15, 2022

Please can this be merged soon? I'm looking forward to it.

@DSKonstantin
Copy link

Please merge this.

@DreamakerDimas
Copy link

Hello @andreialecu. Issue with touchable will be fixed soon or currently this PR changes is the best what I can do to make it work in my project?

@deflexable
Copy link

@andreialecu it almost a year and would love if this PR could be merge

@andreialecu
Copy link
Collaborator

As the Author mentioned, the feature is currently incomplete and has some issues that need to be addressed before it can be merged.

Additionally, I have concerns about adding this feature directly to the library due to the potential complexity it could introduce. However, I am open to exploring alternative solutions, such as implementing an extension point that would allow users to easily add this functionality themselves.

If any of you are still interested in pursuing this feature, I would appreciate it if you could help address the issues with the current pull request and explore the possibility of an extension point.

@tiagocorreiaalmeida
Copy link
Contributor Author

tiagocorreiaalmeida commented Apr 27, 2023

@andreialecu Hey, I actually made a fork of the project and adapt it to allow scroll on header content on the project I'm working, I did remove things that I didn't need, like for instance snap points, but maybe if it makes sense I can try to take some time to review what I did to see if it can be improved and implemented without directly affecting the existent code base (like an extension as you mentioned).
Is the project discord still up? Might be easier to discuss some internals, also noticed you are once again maintaining the project, thank you for that 👍

@HananoshikaYomaru
Copy link

HananoshikaYomaru commented May 3, 2023

Although this is not fully implemented, this feature is quite important and highly requested. I think should accept a working approach first, warn the users of known limitation and then refine it afterwards.

@nggonzalez
Copy link

Also needed this feature and this PR was helpful in unblocking the use of this library. I tried pointerEvents=box-none, but couldn't get it working. I can't use pointerEvents=none because the majority of my headers' contents are touchable.

To help others that may stumble on this PR, here's a patch I created using patch-package that uses some of this code to get header scrolling working on both Android and iOS. The patch is created on v6.1.4. Thank you @tiagocorreiaalmeida for the initial PR and work that this patch is based on!

I understand that PanGestureHandler isn't perfect, but it would be helpful to make it easier to add header scroll. Exposing the scroll sync function and the scrollY might be sufficient to get started so this code can live in user land.

react-native-collapsible-tab-view+6.1.4.patch

@ahmetkuslular
Copy link

@andreialecu hi. It seems to have been a long time since this PR. The problem still persists. Is there any work on this?

@deflexable
Copy link

@ahmetkuslular you can try out this work around https://www.npmjs.com/package/react-native-tab-view-header

@ahmetkuslular
Copy link

@ahmetkuslular you can try out this work around https://www.npmjs.com/package/react-native-tab-view-header

Although this package is successful in header, it does not meet my requests in general. Is it possible to import the properly working header component here into this project?

@nggonzalez
Copy link

nggonzalez commented Sep 1, 2023

@ahmetkuslular did you try the patch above? It should help unblock you

@ahmetkuslular
Copy link

@ahmetkuslular did you try the patch above? It should help unblock you

It's not patch, but I pulled these codes to my locale and tried them together with the current codes. It works seamlessly. I don't know if there are cases that need attention here.

@senghuotlay
Copy link

senghuotlay commented Sep 7, 2023

For anyone who wanted to code snippet without needing patch version for 6.21

@nggonzalez one thing about this is that although we're able to scroll, the scroll with snap stopped working, because it seems like we dont have enough velocity?

const isSlidingTopContainer = useSharedValue(false)
      const isSlidingTopContainerPrev = useSharedValue(false)
      const isTopContainerOutOfSync = useSharedValue(false)
      
 const panGestureScrollYCtx = useSharedValue(0)
      const panGesture = useMemo(
        () =>
          Gesture.Pan()
            .activeOffsetY([-10, 10])
            .onUpdate((event) => {
              if (!isSlidingTopContainer.value) {
                panGestureScrollYCtx.value = scrollYCurrent.value
                isSlidingTopContainer.value = true
                return
              }

              scrollYCurrent.value = interpolate(
                -event.translationY + panGestureScrollYCtx.value,
                [0, headerScrollDistance.value],
                [0, headerScrollDistance.value],
                Extrapolate.CLAMP
              )
            })
            .onEnd((event) => {
              if (!isSlidingTopContainer.value) return

              panGestureScrollYCtx.value = 0
              scrollYCurrent.value = withDecay(
                {
                  velocity: -event.velocityY,
                  clamp: [0, headerScrollDistance.value],
                  deceleration: IS_IOS ? 0.998 : 0.99,
                },
                (finished) => {
                  isSlidingTopContainer.value = false
                  isTopContainerOutOfSync.value = !!finished
                }
              )
            }),
        [
          headerScrollDistance.value,
          isSlidingTopContainer,
          isTopContainerOutOfSync,
          panGestureScrollYCtx,
          scrollYCurrent,
        ]
      )

      useAnimatedReaction(
        () => scrollYCurrent.value - contentInset.value,
        (nextPosition, previousPosition) => {
          if (
            nextPosition !== previousPosition &&
            isSlidingTopContainer.value
          ) {
            resyncTabScroll()
          }
        }
      )

      /* Syncs the scroll of the active tab once we complete the scroll gesture
      on the header and the decay animation completes with success
       */
      useAnimatedReaction(
        () => {
          return (
            isSlidingTopContainer.value !== isSlidingTopContainerPrev.value &&
            isTopContainerOutOfSync.value
          )
        },
        (result) => {
          isSlidingTopContainerPrev.value = isSlidingTopContainer.value

          if (!result) return
          if (isSlidingTopContainer.value === true) return

          resyncTabScroll()

          isTopContainerOutOfSync.value = false
        }
      )
      ```

<GestureDetector gesture={panGesture}>
          <Animated.View
            pointerEvents="box-none"
            style={[
              styles.topContainer,
              headerContainerStyle,
              !cancelTranslation && stylez,
            ]}
          >
            <View
              style={[styles.container, styles.headerContainer]}
              onLayout={getHeaderHeight}
              pointerEvents="box-none"
            >
              {renderHeader &&
                renderHeader({
                  containerRef,
                  index,
                  tabNames: tabNamesArray,
                  focusedTab,
                  indexDecimal,
                  onTabPress,
                  tabProps,
                })}
            </View>
            <View
              style={[styles.container, styles.tabBarContainer]}
              onLayout={getTabBarHeight}
              pointerEvents="box-none"
            >
              {renderTabBar &&
                renderTabBar({
                  containerRef,
                  index,
                  tabNames: tabNamesArray,
                  focusedTab,
                  indexDecimal,
                  width,
                  onTabPress,
                  tabProps,
                })}
            </View>
          </Animated.View>
        </GestureDetector>
        ```

@AugustoAleGon
Copy link

@andreialecu is this something on the roadmap? 👀
This feature is needed.

@bpeck81
Copy link

bpeck81 commented Dec 21, 2023

Also needed this feature and this PR was helpful in unblocking the use of this library. I tried pointerEvents=box-none, but couldn't get it working. I can't use pointerEvents=none because the majority of my headers' contents are touchable.

To help others that may stumble on this PR, here's a patch I created using patch-package that uses some of this code to get header scrolling working on both Android and iOS. The patch is created on v6.1.4. Thank you @tiagocorreiaalmeida for the initial PR and work that this patch is based on!

I understand that PanGestureHandler isn't perfect, but it would be helpful to make it easier to add header scroll. Exposing the scroll sync function and the scrollY might be sufficient to get started so this code can live in user land.

react-native-collapsible-tab-view+6.1.4.patch

The patch didn't work for me unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet