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

[V2] Fix iOS pop gesture when topBar is hidden #4568

Merged
merged 8 commits into from
Mar 11, 2019

Conversation

rawrmaan
Copy link
Contributor

Fixes #4455.

The broken-ness of this feature stems from an iOS bug. I adapted the fix from this StackOverflow answer: https://stackoverflow.com/questions/24710258/no-swipe-back-when-hiding-navigation-bar-in-uinavigationcontroller/41895151#41895151 It seemed like the most well-thought-out and bulletproof fix out of all the answers.

I did thoroughly test this to make sure popGesture works with the nav bar hidden and visible, but I did not add any e2e tests. I would like some guidance on how to test something like this (a swipe gesture) if anyone could help, as I'm not very familiar with the XCTest framework.

Thanks for an amazing library! There really is nothing close out there!

MasoudAlali
MasoudAlali previously approved these changes Jan 18, 2019
@MasoudAlali
Copy link

I tested these changes and it works
I'm waiting for next release and I hope this problem will be fixed
thanks

@Rigellute
Copy link

Any updates on when this will be merged?

kasterlod
kasterlod previously approved these changes Jan 28, 2019
aqweider
aqweider previously approved these changes Feb 4, 2019
nzliu
nzliu previously approved these changes Feb 5, 2019
mackiedrew
mackiedrew previously approved these changes Feb 5, 2019
Copy link

@mackiedrew mackiedrew left a comment

Choose a reason for hiding this comment

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

I have been using this in production for weeks.

leeandher
leeandher previously approved these changes Feb 5, 2019
yashafromrussia
yashafromrussia previously approved these changes Feb 5, 2019
@LRNZ09
Copy link

LRNZ09 commented Feb 7, 2019

This would be really useful

@tunegov
Copy link

tunegov commented Feb 8, 2019

Merge this, please!!!

tunegov
tunegov previously approved these changes Feb 8, 2019
@yashafromrussia
Copy link

yashafromrussia commented Feb 8, 2019

@guyca hey, other than the conflicts, what are the requirements to have this merged? do we need tests for this? i dont see any rn, i can help out

@kyle-ssg
Copy link
Contributor

kyle-ssg commented Feb 11, 2019

This probably fixes two of my open issues, any reason why this is not merged? I don’t want to have to keep a big fork just to have fixes like this in

LRNZ09
LRNZ09 previously approved these changes Feb 11, 2019
fly486
fly486 previously approved these changes Feb 18, 2019
@Rigellute
Copy link

@rawrmaan thank you so much for doing this.

Do you have an idea for when this PR could be merged?

@mackiedrew
Copy link

@guyca What can we do to help the merge along?

@Rigellute
Copy link

Another way of fixing this would be to revert this change 11a174b

The bug that this change was fixing Fix popGesture freezes the app can be worked around by doing this in your root component

componentDidAppear() {
    Navigation.mergeOptions(this.props.componentId, {
        popGesture: false,
    });
}

componentDidDisappear() {
    Navigation.mergeOptions(this.props.componentId, {
        popGesture: true,
    });
}

yogevbd
yogevbd previously approved these changes Mar 11, 2019
@yogevbd yogevbd merged commit 81d8b69 into wix:master Mar 11, 2019
@moonou
Copy link

moonou commented Mar 20, 2019

I like this PR, I upgrade version to 2.14.0-snapshot.246 for the pop gesture, but when I use gesture to pop page, I found registerCommandListener wasn't called. Does anybody have some idea for this?

@guyca
Copy link
Collaborator

guyca commented Mar 20, 2019

@moonou the command listener is invoked only when Navigation.pop command is invoked.

@dangkhoa2708
Copy link

Navigation.push(this.currentScreen, { component: { id: screenID, name: screenID, passProps: params, options: { popGesture: true, bottomTabs: { visible: false, drawBehind: true }, statusBar: { backgroundColor: 'white' }, animations: { push: { content: { x: { from: 1000, to: 0, duration: 250 } } }, pop: { content: { x: { from: 0, to: 1000, duration: 250 } } } } } } })

First of all, thank you for this awesome library, I ran into the pop gesture issue when the top bar is hidden. I've been following this issue from when it was not merged until now. But I still can not make it work. Am I doing it wrong ? Very appreciated with any help.

I'm using version 2.16

vshkl pushed a commit to vshkl/react-native-navigation that referenced this pull request Feb 5, 2020
* Fix iOS pop gesture when nav bar is hidden

* Add missing property

* Factor out InteractivePopGestureDelegate into its own file

* Add missing import

* Make sure fix supports hidden and visible nav bars

* Minor code style fix
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.

[V2] iOS back gesture doesn't work when topBar or backButton is hidden