facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
119.26k stars 24.35k forks source link

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

Closed grabbou closed 9 years ago

grabbou commented 9 years ago

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.

lelandrichardson commented 9 years ago

+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:

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 commented 9 years ago

^-- Holy hell that looks awesome.

lelandrichardson commented 9 years ago

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.

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 https://github.com/facebook/react-native/issues/1024#issuecomment-97289632 .

watsonwu9 commented 9 years ago

@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

marcshilling commented 9 years ago

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 commented 9 years ago

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