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

Add augmentScene and augmentNavigationBar methods #2

Merged
merged 1 commit into from
Oct 7, 2015

Conversation

jesseruder
Copy link
Contributor

Call _subscribeToFocusEvents with navigator instead of this. Otherwise navigator.navigationContext crashes.

Add augmentScene and augmentNavigationBar which can be overridden to add custom components to every scene/navigation bar.

@jesseruder
Copy link
Contributor Author

Also deleted duplicate get navigationContext()

@jadsonlourenco
Copy link

I think this is the problem that I get here #3
I will test this fork, thanks!

<Navigator.NavigationBar
routeMapper={this._routeRenderer.navigationBarRouteMapper}
style={[ExNavigatorStyles.bar, this.props.navigationBarStyle]}
/>
);

return this.augmentNavigationBar(navigationBar);
Copy link
Member

Choose a reason for hiding this comment

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

Looking at this again I think a prop would be preferable:

let navigationBar = this.props.renderNavigationBar({
  routeMapper: this._routeRenderer.navigationBarRouteMapper,
  style: [ExNavigatorStyles.bar, this.props.navigationBarStyle],
});

And use defaultProps to render a basic Navigator.NavigationBar by default.

For the inheritance case I think we could override defaultProps:

class CustomNavigator extends ExNavigator {
  static defaultProps = {
    ...ExNavigator.defaultProps,
    renderNavigationBar: props => <CustomNavigationBar {...props} />,
  };
}

@jesseruder
Copy link
Contributor Author

@jadsonlourenco this fixes the problem in #3 so you can use this branch for now. I'm going to work on some of the other things I changed in this pull so I won't merge it quite yet.

}

let scene = this._routeRenderer.renderScene(route, this);
scene = this.augmentScene(scene, route);
Copy link
Member

Choose a reason for hiding this comment

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

Now I'm thinking this actually could be a prop (this.props.augmentScene). Then the owner of the ExNavigator (instead of its subclass) could write:

render() {
  return <ExNavigator augmentScene={this._augmentScene} />;
}
@autobind
_augmentScene(scene, route) {
 ...
}

@jesseruder jesseruder force-pushed the augment_scene_and_navigation_bar branch from 452dd4d to 96db52d Compare October 7, 2015 07:14
@ide
Copy link
Member

ide commented Oct 7, 2015

Thanks Jesse!

ide pushed a commit that referenced this pull request Oct 7, 2015
Add augmentScene and augmentNavigationBar methods
@ide ide merged commit d2f7520 into master Oct 7, 2015
@ide ide deleted the augment_scene_and_navigation_bar branch October 23, 2015 08:59
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.

None yet

3 participants