wix / react-native-navigation

A complete native navigation solution for React Native
https://wix.github.io/react-native-navigation/
MIT License
13.04k stars 2.67k forks source link

Is popGesture working in v 2.16.0 ? #4918

Closed zabojad closed 5 years ago

zabojad commented 5 years ago

Issue Description

I've specified popGesture: true in my default options but it doesn't seem to have any effect. I cannot use a swipe right gesture to pop the current page. What am I missing?

Here are my options:

    {
    layout: {
        componentBackgroundColor: 'white',
        backgroundColor: 'white',
        orientation: [NavigationUtils.getLayout()],
    },
    popGesture: true,
    topBar: {
        drawBehind: false,
        backButton: {
            visible: false,
            showTitle: false,
        },
        buttonColor: "black",
        hideOnScroll: false,
        title: {
            component: {
                name: MyAppHeaderToolbar,
                alignment:'center',
            }
        },
    },
    sideMenu: {
        left: {
            width: 250, // doesn't have any effect on Android !?
        },
        openGestureMode:'bezel',
    },
}

Environment

zabojad commented 5 years ago

Or can't we have both a swipable left side menu and the popGesture at the same time maybe?

zabojad commented 5 years ago

OK, it actually work but something like 1 time out of 4... Why is that ? If I test the pop gesture in the ios settings app for example, it seems to work much better...

zabojad commented 5 years ago

It really depends on where we touch the screen. If we could enlarge the touch zone where the pop gesture starts, it would be good enough I think...

zabojad commented 5 years ago

Hey @yogevbd ! Sorry to ask you directly but do you know if enlarging the "swipe" zone for the pop gesture is supported in native iOS?

itsam commented 5 years ago

Might be relevant to #3651 which was fixed (?) months ago but it actually still exists...

itsam commented 5 years ago

For me the following worked...

    if (options.sideMenu.left.enabled.hasValue) {
        [sideMenuController side:MMDrawerSideLeft enabled:options.sideMenu.left.enabled.get];

        if(options.sideMenu.left.enabled.get){ 
            [sideMenuController setOpenDrawerGestureModeMask:[[options.sideMenu.openGestureMode getWithDefaultValue:@(MMOpenDrawerGestureModeAll)] integerValue]];
        }
        else {
            [sideMenuController setOpenDrawerGestureModeMask:[[options.sideMenu.openGestureMode getWithDefaultValue:@(MMOpenDrawerGestureModeNone)] integerValue]];
        }
        [options.sideMenu.left.enabled consume];
    }

    if (options.sideMenu.right.enabled.hasValue) {
        [sideMenuController side:MMDrawerSideRight enabled:options.sideMenu.right.enabled.get];

        if(options.sideMenu.right.enabled.get){ 
            [sideMenuController setOpenDrawerGestureModeMask:[[options.sideMenu.openGestureMode getWithDefaultValue:@(MMOpenDrawerGestureModeAll)] integerValue]];
        }
        else {
            [sideMenuController setOpenDrawerGestureModeMask:[[options.sideMenu.openGestureMode getWithDefaultValue:@(MMOpenDrawerGestureModeNone)] integerValue]];
        }
        [options.sideMenu.right.enabled consume];
    }

I add this at /node-modules/react-native-navigation/lib/ios/RNNSideMenuPresenter.m at the end of function - (void)applyOptions:(RNNNavigationOptions *)options {

Should also be put on functions applyOptionsOnInit and mergeOptions respectively

Please, someone who knows objective C to fine tune the above code and create a PR or verify the above and inform me to create a PR myself.

The main idea from @rgoldiez

https://github.com/wix/react-native-navigation/blob/86046eaa5f83c922308591b4213c8d347efd11c8/lib/ios/RNNSideMenuSideOptions.m

zabojad commented 5 years ago

wow... I'll try it and let you know the results...

Anyone from the wix team to merge it? @guyca @yogevbd?

mokoshi commented 5 years ago

I'm having the same problem on v2.20.0. iOS popGesture doesn't work perfectly on components with sideMenus. (About 1 out of 3~4 gestures just works.) Applying the patch from @itsam https://github.com/wix/react-native-navigation/issues/4918#issuecomment-489321039 seems to be fixing the issue.

svenlombaert commented 5 years ago

Can confirm that the above fix works. So that part is added in the applyOptions method. To be complete the same checks need to be added in the mergeOptions as well.

I'll check to open a PR and reference it here. The only problem I see is that, when setting the openDrawerGestureMode, it's for both left and right :/

latusdenis commented 5 years ago

Can confirm too, it works! @itsam Thanks a lot! Terrific job!

svenlombaert commented 5 years ago

The package used is MMDrawerController (for ios), and it seems this one got the latest update 4 years ago.

There's this open issue which asks to set the openDrawerGestureMode separately for the left or right drawer: https://github.com/mutualmobile/MMDrawerController/issues/462

Apparently, it's not possible out of the box. My obj-c skills are quite rust so not sure if we can work around it.

The fix of @itsam will work for a lot of cases, but not when you have two sideMenu's (left and right) and only want to disable one side.

Also, there's a small mistake in the code, the else block should look like this:

[sideMenuController setOpenDrawerGestureModeMask:[@(MMOpenDrawerGestureModeNone) integerValue]];

Because, when disabling the menu, we don't want to get the option the user passed, we always want to set gesture mode to none.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe the issue is still relevant, please test on the latest Detox and report back. Thank you for your contributions.

stale[bot] commented 5 years ago

The issue has been closed for inactivity.

itsam commented 4 years ago

for v3 https://gist.github.com/itsam/84acaf9afac99eceeb31e779f7a707f3

trantus commented 4 years ago

for v3 https://gist.github.com/itsam/84acaf9afac99eceeb31e779f7a707f3

did not help for v3... v2 solution works! but gestures work just after first open any screen

itsam commented 4 years ago

In v3 to be honest the experience originally was far better than v2. In v2 I had to swipe 3-4 times before I actually managed to swipe the sidemenu. On the contrary, in v3 I thought the issue was fixed until I realise (after using the app a lot) that sometimes the swiped needed a second (or third) try... Applying the above gist though it seems (for my case) that swiping is now constantly works as expected. Note: I use no button at all and I use the left side only.

For your case I assume that either in your sidebar (or other screen) maybe you set some static options that might merge a conflicting option? Just a guess...

GioLogist commented 4 years ago

Having a hard time w/v3 myself. It seems as though it only works when swiping from the very edge.

Furthermore, the leftMenu (when enabled) gives me a far better experience. I can open it from swiping in the very middle of the screen. With the popGesture, however, it only works from the edge.