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 dismiss helper, made possible by also adding carefullyGetParent #3669

Merged
merged 4 commits into from Mar 12, 2018

Conversation

Projects
None yet
5 participants
@brentvatne
Member

brentvatne commented Mar 6, 2018

Motivation

Sometimes you want to be able to dismiss a nested stack, for example if you have a root stack that goes from your main app to a modal stack, you might want to go back to the main app when you're n screens deep in the modal stack. People came up with a pretty good solution for this in a previous discussion on the issues here: #686 (comment) - I just adapted this to addNavigationHelpers.

As for navigation.carefullyGetParent, we dicussed it on react-navigation/rfcs#27.

Test plan

Run tests, try it in playground.

@react-navigation-ci

This comment has been minimized.

@react-navigation-ci

This comment has been minimized.

1 similar comment
@react-navigation-ci

This comment has been minimized.

@brentvatne brentvatne changed the title from Add dismiss action, made possible by also adding getParentState to Add dismiss helper, made possible by also adding getParentState Mar 6, 2018

@react-navigation-ci

This comment has been minimized.

@codecov-io

This comment has been minimized.

codecov-io commented Mar 6, 2018

Codecov Report

Merging #3669 into master will decrease coverage by 0.02%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3669      +/-   ##
==========================================
- Coverage   70.16%   70.13%   -0.03%     
==========================================
  Files          57       57              
  Lines        1753     1758       +5     
==========================================
+ Hits         1230     1233       +3     
- Misses        523      525       +2
Impacted Files Coverage Δ
src/navigators/createNavigator.js 65.38% <0%> (-2.62%) ⬇️
src/addNavigationHelpers.js 53.33% <75%> (+3.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 052d228...c4cacbe. Read the comment docs.

@react-navigation-ci

This comment has been minimized.

@ericvicenti

this is looking good except I think we should move to navigation.dangerouslyGetParent, as discussed, instead of getParentState

Next question, dismiss is a stack-only concept, yeah?

@brentvatne

This comment has been minimized.

Member

brentvatne commented Mar 6, 2018

@ericvicenti - agreed, will update this PR for something like dangerouslyGetParent. dismiss only makes sense in the contact of a stack I believe, but it's a bit trickier than that -- it only makes sense when the grandparent navigator is a stack: stack -> stack/tab/whatever -> screen

@brentvatne brentvatne changed the title from Add dismiss helper, made possible by also adding getParentState to Add dismiss helper, made possible by also adding carefullyGetParent Mar 12, 2018

@brentvatne brentvatne force-pushed the @brent/dismiss-helper branch from ac309e3 to c4cacbe Mar 12, 2018

@react-navigation-ci

This comment has been minimized.

1 similar comment
@react-navigation-ci

This comment has been minimized.

@brentvatne brentvatne merged commit 9a6e0bb into master Mar 12, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@ScreamZ

This comment has been minimized.

Contributor

ScreamZ commented Mar 23, 2018

Is that for 2.0 only ?

What do you think of #686 (comment) ?

@brentvatne

This comment has been minimized.

Member

brentvatne commented Apr 3, 2018

2.0 only yes. your solution there looks like a good one for now! :)

@brentvatne brentvatne deleted the @brent/dismiss-helper branch May 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment