Skip to content
This repository has been archived by the owner on Nov 27, 2022. It is now read-only.

fix: exported Pager, PagerConsumer, PagerContext, and PagerContextType #964

Merged
merged 1 commit into from Jun 4, 2020
Merged

fix: exported Pager, PagerConsumer, PagerContext, and PagerContextType #964

merged 1 commit into from Jun 4, 2020

Conversation

immackay
Copy link
Contributor

@immackay immackay commented Feb 18, 2020

Motivation

This allows child PanGestureHandlers to call addGestureHandlerRef using PagerConsumer (and PagerContext), and additionally allows them to waitFor the Pager PanGestureHandler through the addition of gestureHandlerRef to Pager.providerVal

Problems Solved

Using a PanGestureHandler inside a MaterialTopTabNavigator from react-navigation steals pan gesture input from the Pager used by TabView.

Example Usage

Inside a MaterialTopTapNavigator with swipeEnabled = true

import React, { Component } from 'react';
import { PanGestureHandler } from 'react-native-gesture-handler';
import Animated from 'react-native-reanimated';
import { PagerConsumer, PagerContextType } from 'react-native-tab-view';

class Example extends Component {
  private pager: PagerContextType | undefined;
  private panGestureHandlerRef = React.createRef<PanGestureHandler>();
  private y = new Animated.Value(0);

  componentDidMount() {
    this.pager?.addGestureHandlerRef(this.panGestureHandlerRef);
  }

  private handlePan = Animated.event([{
    nativeEvent: ({ translationY: y }: { translationY: number }) =>
      Animated.set(this.y, y)
  }]);

  render() {
    return (
      <PagerConsumer>
        {(pager?: PagerContextType) => {
          this.pager = pager;
          return (
            <View style={{ flex: 1 }}>
              <PanGestureHandler
                ref={this.panGestureHandlerRef}
                simultaneousHandlers={[pager!.gestureHandlerRef]}
                waitFor={pager!.gestureHandlerRef}
                onGestureEvent={this.handlePan}
                onHandlerStateChange={this.handlePan}
              />
            </View>
          );
        }}
      </PagerConsumer>
    );
  }
}

Test plan

Additionally exported PagerConsumer, PagerContext, and PagerContextType
Added gestureHandlerRef to Pager.providerVal
@osdnk
Copy link
Collaborator

osdnk commented Jun 4, 2020

Thanks!

@osdnk osdnk merged commit 795f90f into satya164:master Jun 4, 2020
satya164 added a commit that referenced this pull request Jun 4, 2020
@cherryman
Copy link

@satya164 Any reason this was reverted?

@satya164
Copy link
Owner

It's not something we want to expose as public API because there are issues with it, like: not being able to remove a ref once you add it, PagerContext is the wrong name for gesture handler context, PagerConsumer doesn't need to be exported separately.

@immackay
Copy link
Contributor Author

immackay commented Jun 18, 2020

PagerContext is the wrong name for gesture handler context

This was the name that was already in place, I just exported it. Perhaps changing the name would be a better solution?

PagerConsumer doesn't need to be exported separately

Sure, definitely, but the general standard is to export Consumer and Provider components for ease of use. This is still not a reason to revert completely however, you could just remove this export very easily.

Reverting this pull request breaks code that needs to use a PanGestureHandler inside another container that uses GestureHandlers. Can you explain any more specific issues with this PR?

@immackay
Copy link
Contributor Author

not being able to remove a ref once you add it

Is this a common behaviour? Adding this functionality should be approximately three lines of code, if that. Is this a valid reason to revert this PR?

@osdnk
Copy link
Collaborator

osdnk commented Jun 18, 2020

We believe that this change breaks the OCP of the library and we're strongly motivated to not move with having these exports included in the lib.
Can you try maybe come up with some another solution which won't move out of the internals of the library?
Frankly speaking, I have problems with understanding your issue? Can you maybe describe a bit deeper what's your problem?

@immackay
Copy link
Contributor Author

immackay commented Jun 18, 2020

"I have problems with understanding your issue"

Using a PanGestureHandler inside a MaterialTopTabNavigator from react-navigation steals pan gesture input from the Pager used by TabView.

This means that adding extra gesture functionality inside a MaterialTopTabNavigator causes the left/right swiping in the TabNavigator to be broken.

Please run the example I provided within a MaterialTopTabNavigator. With this PR, the code runs just fine, but without it breaks.

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

Successfully merging this pull request may close these issues.

None yet

4 participants