Permalink
Browse files

NavigationExperimental: Rename `renderOverlay` to `renderHeader`

Summary:
NavigationCardStack is a custom component, and its API should be explicit, not
too generic..

In NavigationCardStack, the prop `renderOverlay` is actually used to render
the NavigationHeader, and we uses absolute position to build the layout for
the header and the body.

One of the problem with using absolute postion and fixed height to build the
layout that contains the header is that the header can't have variant height
easily.

Ideally, if the layout for the header used flex-box, we'd ve able to be more
adaptive to deal with the header that has variant height.

That said, let's rename `renderOverlay` to `renderHeader`, then build the
proper layout that explicitly works better with the header.

If we to need to support overlay in navigation, we may consider add
`renderOverlay` later, if it's really necessary.

Reviewed By: ericvicenti

Differential Revision: D3670224

fbshipit-source-id: ff04acfe9dc995cb57117b3fd9b07d5f97b9c6ee
  • Loading branch information...
1 parent 1d98018 commit ca8531105e29f2204e0c997783e71b214390bedf @hedgerwang hedgerwang committed with Facebook Github Bot 4 Aug 4, 2016
@@ -267,7 +267,7 @@ const YourNavigator = createAppNavigationContainer(class extends Component {
key={'stack_' + tabKey}
onNavigateBack={this._back}
navigationState={scenes}
- renderOverlay={this._renderHeader}
+ renderHeader={this._renderHeader}
renderScene={this._renderScene}
style={styles.navigatorCardStack}
/>
@@ -66,7 +66,7 @@ class UIExplorerApp extends React.Component {
_handleBack: Function;
_handleAction: Function;
_renderCard: Function;
- _renderOverlay: Function;
+ _renderHeader: Function;
_renderScene: Function;
_renderTitleComponent: Function;
state: State;
@@ -78,7 +78,7 @@ class UIExplorerApp extends React.Component {
componentWillMount() {
this._handleAction = this._handleAction.bind(this);
this._handleBack = this._handleAction.bind(this, {type: 'back'});
- this._renderOverlay = this._renderOverlay.bind(this);
+ this._renderHeader = this._renderHeader.bind(this);
this._renderScene = this._renderScene.bind(this);
this._renderTitleComponent = this._renderTitleComponent.bind(this);
}
@@ -137,14 +137,14 @@ class UIExplorerApp extends React.Component {
<NavigationCardStack
navigationState={this.state.stack}
style={styles.container}
- renderOverlay={this._renderOverlay}
+ renderHeader={this._renderHeader}
renderScene={this._renderScene}
/>
);
}
- _renderOverlay(props: NavigationSceneRendererProps): ReactElement<any> {
+ _renderHeader(props: NavigationSceneRendererProps): ReactElement<any> {
return (
<NavigationHeader
{...props}
@@ -60,7 +60,7 @@ type Props = {
direction: NavigationGestureDirection,
navigationState: NavigationState,
onNavigateBack?: Function,
- renderOverlay: ?NavigationSceneRenderer,
+ renderHeader: ?NavigationSceneRenderer,
renderScene: NavigationSceneRenderer,
cardStyle?: any,
style: any,
@@ -93,7 +93,7 @@ class NavigationCardStack extends React.Component<DefaultProps, Props, void> {
direction: PropTypes.oneOf([Directions.HORIZONTAL, Directions.VERTICAL]),
navigationState: NavigationPropTypes.navigationState.isRequired,
onNavigateBack: PropTypes.func,
- renderOverlay: PropTypes.func,
+ renderHeader: PropTypes.func,
renderScene: PropTypes.func.isRequired,
cardStyle: View.propTypes.style,
gestureResponseDistance: PropTypes.number,
@@ -132,10 +132,10 @@ class NavigationCardStack extends React.Component<DefaultProps, Props, void> {
_render(props: NavigationTransitionProps): ReactElement<any> {
const {
- renderOverlay
+ renderHeader
} = this.props;
- const overlay = renderOverlay && renderOverlay(props);
+ const overlay = renderHeader && renderHeader(props);
const scenes = props.scenes.map(
scene => this._renderScene({

5 comments on commit ca85311

@migueloller

renderOverlay made sense for rendering the tabbar in tab views.

@ide
Collaborator
ide commented on ca85311 Aug 9, 2016

renderOverlay made sense for rendering the tabbar in tab views.

this is a good point cc @hedgerwang, @ericvicenti ^

@jmurzy
Contributor
jmurzy commented on ca85311 Aug 9, 2016

@ide I believe the motivation here is two-fold (1)NavigationCardStack is not a part of NavigationExperimental and is designed to behave in a defined way rather than provide a low-level API similar to that of NavigationTratisioner. Limiting its API prevents issues such as this one from being raised up at that level of abstraction. (2) Facebook doesn't use these custom components internally and NavigationExperimental team doesn't want to maintain them.

🍺

@ide
Collaborator
ide commented on ca85311 Aug 9, 2016

Ah OK that makes sense i.e. tab navigation should be built on top of NavigationTransitioner, which is still header-agnostic.

@hedgerwang
Contributor

@jmurzy : you just said exactly what I was about to say. Thanks :)

Please sign in to comment.