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

Add drawerLockMode to DrawerView #793

Closed

Conversation

martnu
Copy link

@martnu martnu commented Mar 23, 2017

This allows us to disable the gesture to open the drawer. Related to #390.

@zoontek
Copy link

zoontek commented Mar 23, 2017

This PR will simplify a LOT our navigation. Thanks! :D

@ferrannp
Copy link

ferrannp commented Mar 24, 2017

Did you test that locked-open ? Because I tried doing this PR and I couldn't make the option to work and I didn't spend more time on it. I understand that if I choose locked-open, the drawer should be opened by default right? And that is controlled by the navigation state so probably we need to add some logic ? cc @satya164

@satya164
Copy link
Member

cc @DanielMSchmidt

@DanielMSchmidt
Copy link

DanielMSchmidt commented Mar 24, 2017

@ferrannp AFAIK lock-open is implemented both in Android as well as in the react-native-drawer-layout-polyfill as "When the drawer is open, don't let it close". This does not include opening it as the property gets set. If you experience a different behaviour on Android, please let me know.

@martnu PR looks nice, thank you for your effort!

@ferrannp
Copy link

@DanielMSchmidt right mmm, it would be nice thought to provide a prop as initial state of the drawer (closed or open) and then I can use it together with locked-open to be able for example to develop for tablets.

@satya164
Copy link
Member

satya164 commented Mar 24, 2017

Probably better to just use a plain view on tablets. You don't need any of drawer's behaviour.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MUCH NEEDED!

@DanielMSchmidt
Copy link

@satya164 @martnu How can I help getting this merged ASAP?

@martnu
Copy link
Author

martnu commented Mar 29, 2017

@DanielMSchmidt I just updated the branch, but after CI runs I'm just waiting for a merge.

@satya164
Copy link
Member

I think it makes more sense in navigationOptions. You cannot change this option dynamically, so it doesn't really do anything. A drawer navigator with always locked drawer? not very useful.

@martnu
Copy link
Author

martnu commented Mar 29, 2017

@satya164 It is useful since it allows us to only open/close the drawer programmatically and not via touch gestures since that currently conflicts with the default swipe-back behavior in a stack. I do agree that it would be nice to have it configured for each screen but I'd have to look into the code a bit more.

@satya164
Copy link
Member

How can you change this programmatically?

@martnu
Copy link
Author

martnu commented Mar 29, 2017

@satya164 We can't change the behavior for the drawer, but I can open the Drawer programmatically via navigate('DrawerOpen').

@satya164
Copy link
Member

I see.

Anyway, we should add it to navigationOptions since we will need that, and it will not be very nice to have the same config in 2 places.

@martnu martnu changed the title Add drawerLockMode to DrawerNavigator Add drawerLockMode to DrawerView Mar 29, 2017
@satya164
Copy link
Member

CI is failing. You probably need to fix the flow types.

const screenNavigation = addNavigationHelpers({
state: this._screenNavigationProp.state,
dispatch: this._screenNavigationProp.dispatch,
});

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

@ericvicenti ericvicenti mentioned this pull request Apr 6, 2017
@j-mendez
Copy link

j-mendez commented Apr 24, 2017

How to update the lock-mode dynamically? Currently setting these options only work when you do it in the DrawerNavigator options and cannot be updated. When I try to use static navigationOptions for the drawer nothing happens. I declare it on the contentComponent. Even using a global for the value for lockMode will not cause it to update when needed.
.

@nishiltamboli
Copy link

Yes this is much needed feature. Will solve a lot of problems.

@maximromanyuk
Copy link

@mmerickel Are you able to fix it?

@matzero
Copy link

matzero commented Aug 25, 2017

If you fork the library and patched it with the PR code, it works but
Why you no merge!

@mukurpuri
Copy link

Hello!! Is anyone there? This PR is critically important, I will clone this repo and work with you for this.

@williamdes
Copy link

Exactly, do it :)

@vonovak
Copy link
Member

vonovak commented Aug 29, 2017

seems like #1377 is a better PR than this one since it allows to specify this within navigationOptions

@Jacse Jacse mentioned this pull request Aug 31, 2017
@sospedra
Copy link

sospedra commented Sep 7, 2017

Hmmm 🤔 So, what's happening? Why a highly-requested 6 months-ago PR is not merged? Is there any reason? I mean, it's not even a feature. We're just passing down a prop to another lib which is doing the work behind. No big deal, no?

@sibelius
Copy link

sibelius commented Sep 9, 2017

can we get this or #1377 merge in?

This solves some real app use cases

@maximromanyuk
Copy link

@sibelius there is some issues someone need to fix

@kelset
Copy link

kelset commented Sep 12, 2017

There are conflicts in src/TypeDefinition.js - can you fix them?

@antofa
Copy link

antofa commented Sep 15, 2017

any update?

@guilhermedecampo
Copy link

@spencercarli I read on a blog post that you'll work here on react-navigation to take a look on documentation and issues can you give a heads up about this one here?

We are building an app that this is primordial. There is any timeline or another PR, issue etc that people are working for this feature? Definitely we can help in any means.
Best,

@spencercarli
Copy link
Member

@guilhermedecampo going to try to get this merged asap. I'm not super familiar with drawers so I'm going to have someone else look it over. Looks good to me though

Copy link

@GantMan GantMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks/feels great

Not sure what CI is complaining about, I forever get a spinner on the message, but the code looks great to me @spencercarli

@GantMan
Copy link

GantMan commented Sep 21, 2017

besides this, it looks great.
no

@vonovak
Copy link
Member

vonovak commented Sep 21, 2017

@spencercarli please note this PR does not allow to specify drawer lock mode in navigation options and several people already expressed they need that. There is another pr that I linked a couple of posts higher which should be able to do that, it might be good to take a look at it.

@GantMan
Copy link

GantMan commented Sep 21, 2017

@spencercarli - I agree with @vonovak

After reviewing both PRs, I find #1377 better.

We might need to make drawer lock mode optional in the type and implementation but that would be a small patch, I'm sure everyone is up for.

@janzal
Copy link

janzal commented Sep 21, 2017

Agree with @vonovak. +1

@spencercarli
Copy link
Member

spencercarli commented Sep 21, 2017

Ok we'll go with #1337 instead (thanks for pointing that out again @vonovak). It just needs a bit of work to get tests passing then we can ship it :shipit:

@spencercarli
Copy link
Member

spencercarli commented Sep 21, 2017

Just merged #1377 in! Thanks for everyone's work and input on this. Will be in the next release 😄

@t407289575
Copy link

Hold coming soon !!! @spencercarli

@jacktan165
Copy link

Wait it's being fixed!? I actually lost hope, only checking on this status once per month! Thanks guys!

@giantss
Copy link

giantss commented May 23, 2018

const navigationOptions = ({navigation}) => ({
  //Other configuration items are omitted
    drawerLockMode:'locked-closed' //here
});


const MainStackNavigator = StackNavigator({
    Chat:{
        screen: Chat,
        navigationOptions:{   
             drawerLockMode:'unlocked',  //here
        }
    }
  //Other configuration items are omitted
},{
    navigationOptions: navigationOptions,
    initialRouteName: 'Chat', //home page
});

@react-navigation react-navigation locked as resolved and limited conversation to collaborators May 23, 2018
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