Skip to content

Commit

Permalink
Fork NavigationAnimatedView to NavigationTransitioner
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Hedger Wang authored and Facebook Github Bot 7 committed May 18, 2016
1 parent 0e997c6 commit 7db7f78
Show file tree
Hide file tree
Showing 6 changed files with 304 additions and 5 deletions.
13 changes: 13 additions & 0 deletions Libraries/NavigationExperimental/NavigationAnimatedView.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
*/
'use strict';

/**
* WARNING: NavigationAnimatedView will be deprecated soon.
* Use NavigationTransitioner instead.
*/

const Animated = require('Animated');
const NavigationPropTypes = require('NavigationPropTypes');
const NavigationScenesReducer = require('NavigationScenesReducer');
Expand Down Expand Up @@ -40,6 +45,7 @@ type Props = {
type State = {
layout: NavigationLayout,
position: NavigationAnimatedValue,
progress: NavigationAnimatedValue,
scenes: Array<NavigationScene>,
};

Expand Down Expand Up @@ -96,6 +102,9 @@ class NavigationAnimatedView
this.state = {
layout,
position: new Animated.Value(this.props.navigationState.index),
// This `progress` is a adummy placeholder value to meet the values
// as `NavigationSceneRendererProps` requires.
progress: new Animated.Value(1),
scenes: NavigationScenesReducer([], this.props.navigationState),
};
}
Expand Down Expand Up @@ -179,6 +188,7 @@ class NavigationAnimatedView

const {
position,
progress,
scenes,
} = this.state;

Expand All @@ -187,6 +197,7 @@ class NavigationAnimatedView
navigationState,
onNavigate,
position,
progress,
scene,
scenes,
});
Expand All @@ -202,6 +213,7 @@ class NavigationAnimatedView

const {
position,
progress,
scenes,
} = this.state;

Expand All @@ -210,6 +222,7 @@ class NavigationAnimatedView
navigationState,
onNavigate,
position,
progress,
scene: scenes[navigationState.index],
scenes,
});
Expand Down
4 changes: 3 additions & 1 deletion Libraries/NavigationExperimental/NavigationExperimental.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ const NavigationAnimatedView = require('NavigationAnimatedView');
const NavigationCard = require('NavigationCard');
const NavigationCardStack = require('NavigationCardStack');
const NavigationHeader = require('NavigationHeader');
const NavigationPropTypes = require('NavigationPropTypes');
const NavigationReducer = require('NavigationReducer');
const NavigationStateUtils = require('NavigationStateUtils');
const NavigationPropTypes = require('NavigationPropTypes');
const NavigationTransitioner = require('NavigationTransitioner');

const NavigationExperimental = {
// Core
Expand All @@ -26,6 +27,7 @@ const NavigationExperimental = {

// Views
AnimatedView: NavigationAnimatedView,
Transitioner: NavigationTransitioner,

// CustomComponents:
Card: NavigationCard,
Expand Down
2 changes: 2 additions & 0 deletions Libraries/NavigationExperimental/NavigationPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ const SceneRendererProps = {
navigationState: navigationParentState.isRequired,
onNavigate: PropTypes.func.isRequired,
position: animatedValue.isRequired,
progress: animatedValue.isRequired,
scene: scene.isRequired,
scenes: PropTypes.arrayOf(scene).isRequired,
};
Expand Down Expand Up @@ -103,6 +104,7 @@ function extractSceneRendererProps(
navigationState: props.navigationState,
onNavigate: props.onNavigate,
position: props.position,
progress: props.progress,
scene: props.scene,
scenes: props.scenes,
};
Expand Down
266 changes: 266 additions & 0 deletions Libraries/NavigationExperimental/NavigationTransitioner.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,266 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule NavigationTransitioner
* @flow
*/
'use strict';

const Animated = require('Animated');
const Easing = require('Easing');
const NavigationPropTypes = require('NavigationPropTypes');
const NavigationScenesReducer = require('NavigationScenesReducer');
const React = require('React');
const StyleSheet = require('StyleSheet');
const View = require('View');

import type {
NavigationActionCaller,
NavigationAnimatedValue,
NavigationLayout,
NavigationParentState,
NavigationScene,
NavigationSceneRenderer,
NavigationTransitionConfigurator,
} from 'NavigationTypeDefinition';

type Props = {
configureTransition: NavigationTransitionConfigurator,
navigationState: NavigationParentState,
onNavigate: NavigationActionCaller,
onTransitionEnd: () => void,
onTransitionStart: () => void,
renderOverlay: ?NavigationSceneRenderer,
renderScene: NavigationSceneRenderer,
style: any,
};

type State = {
layout: NavigationLayout,
position: NavigationAnimatedValue,
progress: NavigationAnimatedValue,
scenes: Array<NavigationScene>,
};

const {PropTypes} = React;

const DefaultTransitionSpec = {
duration: 250,
easing: Easing.inOut(Easing.ease),
};

function isSceneNotStale(scene: NavigationScene): boolean {
return !scene.isStale;
}

class NavigationTransitioner extends React.Component<any, Props, State> {

_onLayout: (event: any) => void;
_onTransitionEnd: () => void;

props: Props;
state: State;

static propTypes = {
configureTransition: PropTypes.func,
navigationState: NavigationPropTypes.navigationState.isRequired,
onNavigate: PropTypes.func.isRequired,
onTransitionEnd: PropTypes.func,
onTransitionStart: PropTypes.func,
renderOverlay: PropTypes.func,
renderScene: PropTypes.func.isRequired,
};

constructor(props: Props, context: any) {
super(props, context);

// The initial layout isn't measured. Measured layout will be only available
// when the component is mounted.
const layout = {
height: new Animated.Value(0),
initHeight: 0,
initWidth: 0,
isMeasured: false,
width: new Animated.Value(0),
};

this.state = {
layout,
position: new Animated.Value(this.props.navigationState.index),
progress: new Animated.Value(1),
scenes: NavigationScenesReducer([], this.props.navigationState),
};
}

componentWillMount(): void {
this._onLayout = this._onLayout.bind(this);
this._onTransitionEnd = this._onTransitionEnd.bind(this);
}

componentWillReceiveProps(nextProps: Props): void {
const nextScenes = NavigationScenesReducer(
this.state.scenes,
nextProps.navigationState,
this.props.navigationState
);

if (nextScenes === this.state.scenes) {

This comment has been minimized.

Copy link
@jmurzy

jmurzy May 24, 2016

Contributor

@hedgerwang Possible regression here. Given 2 scenes, namely scene A and scene B, where scene B is marked stale and if nextNavigationState tries to recover scene B from stale, the scenes and navigationState go out of sync when render is called.

In that renderScene and renderOverlay are both called with scene = undefined.

i.e.

return renderOverlay({
  layout: this.state.layout,
  navigationState,
  onNavigate,
  position,
  progress,
  scene: scenes[navigationState.index],
  scenes,
});

scenes[navigationState.index] will be undefined because navigationState.index = 1 where scenes only contains the first scene.

Verified that AnimatedView doesn't have the same issue. Cannot determine exactly why but commenting out the following lines fixes the issue:

if (nextScenes === this.state.scenes) {
  return;
}

Update

I think there is a race condition between when render is invoked and _onTransitionEnd is called after animations finish. You are calling setState in _onTransitionEnd after removing stale scenes that fires a rerender but there is no guarantees that happens after the initial render.

/cc @ericvicenti

🍺

This comment has been minimized.

Copy link
@hedgerwang

hedgerwang May 24, 2016

good catch. I'll fix it. Thanks.

This comment has been minimized.

Copy link
@jmurzy

jmurzy Jun 21, 2016

Contributor

@hedgerwang I'm still seeing this issue.

To reproduce, use #8268:

Using the NavigationCardStack-NavigationHeader-Tabs-example, PUSH > POP > PUSH. This will "throw" the now removed 'no active scene found' error.

invariant(!!activeScene, 'no active scene found');

/cc @ericvicenti

🍺

return;
}

const {
position,
progress,
} = this.state;

// update scenes.
this.setState({
scenes: nextScenes,
});

// get the transition spec.
const transitionUserSpec = nextProps.configureTransition ?
nextProps.configureTransition() :
null;

const transtionSpec = {
...DefaultTransitionSpec,
...transitionUserSpec,
};

progress.setValue(0);

const animations = [
Animated.timing(
progress,
{
...transtionSpec,
toValue: 1,
},
),
];

if (nextProps.navigationState.index !== this.props.navigationState.index) {
animations.push(
Animated.timing(
position,
{
...transtionSpec,
toValue: nextProps.navigationState.index,
},
),
);
}

// play the transition.
nextProps.onTransitionStart && nextProps.onTransitionStart();
Animated.parallel(animations).start(this._onTransitionEnd);

This comment has been minimized.

Copy link
@jmurzy

jmurzy May 23, 2016

Contributor

This looks nice. But I'm seeing a huge degradation in performance compared to AnimatedView impl. Any idea why that would be?

@hedgerwang @ericvicenti

🍺

This comment has been minimized.

Copy link
@hedgerwang

hedgerwang May 23, 2016

We haven't noticed this yet. But I'll definitely look into this. Thanks for the reporting :)

}

render(): ReactElement {
const overlay = this._renderOverlay();
const scenes = this._renderScenes();
return (
<View
onLayout={this._onLayout}
style={this.props.style}>
<View style={styles.scenes} key="scenes">
{scenes}
</View>
{overlay}
</View>
);
}

_renderScenes(): Array<?ReactElement> {
return this.state.scenes.map(this._renderScene, this);
}

_renderScene(scene: NavigationScene): ?ReactElement {
const {
navigationState,
onNavigate,
renderScene,
} = this.props;

const {
position,
progress,
scenes,
} = this.state;

return renderScene({
layout: this.state.layout,
navigationState,
onNavigate,
position,
progress,
scene,
scenes,
});
}

_renderOverlay(): ?ReactElement {
if (this.props.renderOverlay) {
const {
navigationState,
onNavigate,
renderOverlay,
} = this.props;

const {
position,
progress,
scenes,
} = this.state;

return renderOverlay({
layout: this.state.layout,
navigationState,
onNavigate,
position,
progress,
scene: scenes[navigationState.index],
scenes,
});
}
return null;
}

_onLayout(event: any): void {
const {height, width} = event.nativeEvent.layout;

const layout = {
...this.state.layout,
initHeight: height,
initWidth: width,
isMeasured: true,
};

layout.height.setValue(height);
layout.width.setValue(width);

this.setState({ layout });
}

_onTransitionEnd(): void {
const scenes = this.state.scenes.filter(isSceneNotStale);
if (scenes.length !== this.state.scenes.length) {
this.setState({ scenes });

This comment has been minimized.

Copy link
@jmurzy

jmurzy May 24, 2016

Contributor

@hedgerwang

Performance issues I mentioned earlier ^ are caused by this line. I think this is also the cause of the undefined scene issue I mentioned ^. Wrapping this in runAfterInteractions to batch removing of stale scenes seems to help a lot but still nothing near the performance of NavigationAnimated.

InteractionManager.runAfterInteractions(() => {
  const scenes = this.state.scenes.filter(isSceneNotStale);
  if (scenes.length !== this.state.scenes.length) {
    this.setState({ scenes });
  }
});

/cc @ericvicenti

🍺

}
this.props.onTransitionEnd && this.props.onTransitionEnd();
}
}

const styles = StyleSheet.create({
scenes: {
flex: 1,
},
});

module.exports = NavigationTransitioner;
Loading

0 comments on commit 7db7f78

Please sign in to comment.