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

[NavigationExperimental] Migrate NavigationCardStack #8268

Closed
wants to merge 1 commit into from

Conversation

jmurzy
Copy link
Contributor

@jmurzy jmurzy commented Jun 21, 2016

As discussed in #8262.

@ghost
Copy link

ghost commented Jun 21, 2016

By analyzing the blame information on this pull request, we identified @hedgerwang and @ericvicenti to be potential reviewers.

@ghost ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jun 21, 2016
@jmurzy
Copy link
Contributor Author

jmurzy commented Jun 21, 2016

@ericvicenti For NavigationCardStack, do we want to stay opinionated on issues mentioned in #7210? @hedgerwang fixed that in 3a62314. Not sure if we should follow the same path for CardStack though. It would only add additional complexity to the examples and I don't think it's worth it.

@jmurzy jmurzy force-pushed the migrate-cardstack branch from 998d153 to 4c8e8cc Compare June 21, 2016 02:36
@jmurzy jmurzy force-pushed the migrate-cardstack branch from 4c8e8cc to 05cc24c Compare June 21, 2016 03:40
@jmurzy jmurzy changed the title [NavigationExperimental] Migrate NavigationCardStack, Kill NavigationAnimatedView [NavigationExperimental] Migrate NavigationCardStack Jun 21, 2016
if (this.props.renderOverlay) {
const route = navigationState.routes[navigationState.index];

const activeScene = props.scenes.find(

Choose a reason for hiding this comment

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

call of method renderOverlay Function cannot be called on possibly undefined value undefined

Choose a reason for hiding this comment

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

call of method renderOverlay Function cannot be called on possibly null value null

@ericvicenti
Copy link
Contributor

@facebook-github-bot import

@ghost
Copy link

ghost commented Jun 21, 2016

Thanks for importing. If you are an FB employee go to Phabricator to review.

jmurzy referenced this pull request Jun 21, 2016
Summary:
- Fork NavigationAnimatedView to NavigationTransitioner
- NavigationAnimatedView will soon be deprecated and we'd ask people to use NavigationTransitioner instead.

Difference between  NavigationTransitioner and NavigationAnimatedView

- prop `applyAnimation` is removed.
- new prop `configureTransition`, `onTransitionStart`, and `onTransitionEnd` are added.

tl;dr;

In NavigationAnimatedView, we `position` (an Animated.Value object) as a proxy of the
transtion which happens whenever the index of navigation state changes.

Because `position` does not change unless navigation index changes, it won't
be possible to build animations for actions that changes the  navigation state
without changing the index.

Also, we believe that the name `Transitioner` is a better name for this core component
that focuses on transitioning. Note that the actual animation work is done via
`<Animated.View />` returnd from the `renderScene` prop.

Reviewed By: ericvicenti

Differential Revision: D3302688

fbshipit-source-id: 720c3a4d3ccf97eb05b038baa44c9e780aad120b
@ghost ghost closed this in 47a7289 Jun 21, 2016
bubblesunyum pushed a commit to iodine/react-native that referenced this pull request Aug 23, 2016
Summary:
As discussed in facebook#8262.
Closes facebook#8268

Reviewed By: hedgerwang

Differential Revision: D3462059

Pulled By: ericvicenti

fbshipit-source-id: 1f48db3a15668e5a1122b7dcb244bd4352acf9a9
mpretty-cyro pushed a commit to HomePass/react-native that referenced this pull request Aug 25, 2016
Summary:
As discussed in facebook#8262.
Closes facebook#8268

Reviewed By: hedgerwang

Differential Revision: D3462059

Pulled By: ericvicenti

fbshipit-source-id: 1f48db3a15668e5a1122b7dcb244bd4352acf9a9
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants