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

[NavigatorIOS] few improvements [custom titleView, navigationItem and padding] #1024

Closed
grabbou opened this issue Apr 26, 2015 · 6 comments
Closed
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@grabbou
Copy link
Contributor

grabbou commented Apr 26, 2015

Hey,

I am trying to scaffold a proof of concept of the current native iOS application we've done recently. Although everything is going flawlessly I think that NavigatorIOS can be slightly improved when it comes down to pushing new view controllers and setting custom title views. Otherwise - it's just easier to stick to native UINavigationController and expose pure ViewControllers with React with no navigation (in that particular case).

For now, AFAIK, we can only set a title on a component being pushed to a navigator when calling

this.props.navigator.push({
   title: 'A title'
});

It would be handy to make that component more iOS-like so it allows including custom view (e.g. an image) as a titleView inside UINavigationBar and gets navigationItem properties directly from a component so it's way cleaner and easier to maintain those properties in the future, when number of components grows.

NavigatorIOS could then look for navigationItem method inside a newly created component and if it's present, get those properties and update its navigationBar. Initially I was thinking about creating static methods but then, it would be harder to set e.g. title based on props passed to a component.

Replace whole navigationItem with a different component:

var MainViewComponent = new React.createClass({

    navigationItem: function() {
        return {
            title: <Image source={require('./myLogo.jpg')} />
        };
    }

});

or just overwrite a title

var MainViewComponent = new React.createClass({    

    navigationItem: function() {
       return {
            title: 'A book with title ' + this.props.book.title
       };
    }

});

Same thing goes for back/forward buttons (similar to TabBar.item)

var MainViewComponent = new React.createClass({    

    navigationItem: function() {
       return {
            backButton: <NavigatorIOSBar.Item systemIcon='bookmarks' />,
            rightButton: <NavigatorIOSBar.Item systemIcon='send' />
       };
    }

});

This will obviously require a new component - NavigatorIOSBar.Item to be created and optionally a NavigatorIOSBar (although it can be left empty and added in the future). Also, this could've been done without touching the current behaviour so no breaking changes in the next releases (they can be marked as deprecated with a special warning printed as when using similar methods with react.js)

Another thing is, when I've built my first View with a NavigatorIOS the View inside it (initialRoute) was hidden beneath the navigation bar. Shouldn't that padding be applied automatically on a NavigatorIOS as being done by the native implementation?

What do you think? Looks like a big PR to be done but I am quite happy to take it.

@grabbou grabbou changed the title NavigatorIOS - custom titleView for UINavigationBar NavigatorIOS - few improvements [custom titleView, navigationItem and padding] Apr 26, 2015
@lelandrichardson
Copy link
Contributor

+1

I've been working on porting an existing app as well and this is one of the biggest holes that React Native didn't handle very well by default.

Some other people have tried to solve similar problems and their (quite different) implementations may provide some inspiration:

https://github.com/t4t5/react-native-router

https://github.com/Kureev/react-native-navbar

I'm currently trying to tweak the "react-native-router" implementation to do something similar. Here's a gif of the "react-native-router" in action: (Edit: To be clear, the gif below is NOT my own work, it's the work of @t4t5)

The main things that I want to be able to do:

  • update the navigation title from the current route component (ie, when something updates), or alternatively, have the title be overridden by an arbitrary component and subscribe to updates (eg, flux stores)
  • have arbitrary "right" and "left" bar area view components that could override the default forward/back buttons, including extra buttons if desired (for example, the "New Tweet" button in the example above)

I believe this is all possible by creating a custom wrapper around the component, but it would be nice if we could reduce the friction a little bit.

@jaygarcia
Copy link
Contributor

^-- Holy hell that looks awesome.

@lelandrichardson
Copy link
Contributor

LoroTashi,

I'm not sure if this is the issue or not.... but how do you know the return
value is not being returned? just because you don't see it?

If this is the case, one thing you might want to check is to make sure that
your view isn't being rendered underneath the navigation bar, which can
often be the case...

for instance, try adding a marginTop: 64 to the styles.wrapper and see
if it shows up?

Apologies if you've already tried this.

  • Leland

On Tue, Apr 28, 2015 at 7:53 PM, LoroTashi notifications@github.com wrote:

Hi, I think improvement on the navigation is really a brilliant idea. I
hope it would not be offensive that I ask a silly question here. When I
implement the demo test about navigation,
the MyView is triggered and the console.log function works. However, the
return value is NOT return.
in fact, I find a post with the same issue at
http://stackoverflow.com/questions/29367270/component-wont-render-within-navigatorios-react-native
but none of the solution helps in my case.

[image: 2015-04-29 10 42 08]
https://cloud.githubusercontent.com/assets/912960/7384075/aae6ba0e-ee5d-11e4-9d8e-d6c16b6fbb52.png


Reply to this email directly or view it on GitHub
#1024 (comment)
.

@watsonwu9
Copy link

@lelandrichardson on thanks. Yes, the overlapping issue might where the problem lies. But I did add a
marginTop value and nothing there.
2015-04-29 12 39 15
2015-04-29 12 39 21

@brentvatne brentvatne changed the title NavigatorIOS - few improvements [custom titleView, navigationItem and padding] [NavigatorIOS] few improvements [custom titleView, navigationItem and padding] May 30, 2015
@marcshilling
Copy link

update the navigation title from the current route component (ie, when something updates)

I definitely have a need for this. And not just title - also the ability to dynamically update the left/right buttons from the current route component

@grabbou
Copy link
Contributor Author

grabbou commented Jun 4, 2015

Currently I've just put all the stuff into a separate routing factory that has all the possible components defined upfront so displaying a new route is as easy as calling this.navigateTo('route'), but I am happy to work on few improvements on NavigatorIOS to bring more UINavigationController goodies. The issue seems to be somewhere at the bottom of the priority list, so let's talk about this during react conf and try to figure out what we can do about that ;)
CC: @vjeux

@grabbou grabbou closed this as completed Jul 21, 2015
@facebook facebook locked as resolved and limited conversation to collaborators Jul 22, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

6 participants