Coder Social home page Coder Social logo

Comments (32)

Yasser-G avatar Yasser-G commented on May 2, 2024 14

You Can call SetState or any other function inside your component from static navigationOptions using this simple lines

componentDidMount() {
        this.props.navigation.setParams({
            setState: this.setState.bind(this),
            myCustomFunction: myCustomFunction.bind(this),
        })
    }
static navigationOptions = ({ navigation }) => {
        const setState = navigation.getParam('setState', () => { })
        const myCustomFunction = navigation.getParam('myCustomFunction', () => { })
        return {
            headerRight: <TouchableOpacity onPress={() => setState({ buttonPressed: true })} />,
            headerLeft: <TouchableOpacity onPress={() => myCustomFunction()} />

        }
    }

Hope this may help someone.

from rfcs.

dantman avatar dantman commented on May 2, 2024 8

If you need to configure any options based on props and state of the component, or want to update state and props based on some action such as tab press, you need to do it in a hacky way by changing params.

There's nothing hacky about communicating with params, params are basically props/state for the header.

it's way more complicated than it needs to be.

It's not more complicated than it needs to be. navigationOptions are static because some of them need to be read statically. For instance title/drawerLabel/drawerIcon/drawerLockMode are read by the DrawerNavigator to populate the drawer for drawer items corresponding to routes that are not even being rendered. DrawerNavigator only renders the active item, so none of the other instances have an instance you can use this.props.navigation.setOptions({}) or <NavigationOptions> in.

It also breaks when used with HOCs which don't hoist static props, a common source of confusion.

navigationOptions must be static, so we can't move the options to the instance. But perhaps if HOCs are a big issue we could switch the recommendation to an alternative to static navigationOptions = {}; that has less issue with HOCs.

Since HOCs are often applied using a decorator, how about a navigationOptions decorator.

@navigationOptions(({navigation: {params}}) => ({
  title: `${params.user}'s Profile`,
}))
@myHOC
class MyRoute extends PureComponent {
  // ...
}

navigationOptions are still static, they are still kept near the top of the class definition, and all we have to worry about is a "Make sure @navigationOptions is the top decorator in the list" warning instead of worrying about whether any one of the HOCs do not hoist static props.

from rfcs.

grabbou avatar grabbou commented on May 2, 2024 7

I talked to @satya164 earlier today about getting a reference implementation in place and I am thinking I'll give it a go in my free time this week.

from rfcs.

KianooshSoleimani avatar KianooshSoleimani commented on May 2, 2024 4

its really simple you can make stateless component and connect it with redux like this :

   const view = ({ state, color }) => 
   {
   <View>
   <Text>{state.something}</Text>
   </View>
   }

   export default connect(
      (state) => ({
           data: state.data,
   }),
   null
   )(view);

and then use it in static options :

 static navigationOptions = ({ navigation }) => {
    return {
        tabBarIcon: ({ tintColor }) => <view color={tintColor}/>
    }
 };

from rfcs.

satya164 avatar satya164 commented on May 2, 2024 2

There's nothing hacky about communicating with params, params are basically props/state for the header.

Yes, params are props for the route.

It's hacky because of the way we need to use them to achieve basic things.

  1. If you need to trigger a side effect in the screen from an action in the header or vice-versa, for example update scroll position on a button click, you need to add something like a counter in the params which you increment and do stuff when that changes. Side-effects should not be done by doing things like updating a counter in the state.
  2. If you need share state between component and header, for example to show something in the header based on the screen's local state, you need to update params everytime your component state changes, now you have 2 sources of truth.

I have seen people pass instance methods, and sometimes entire component because it's too hard to do basic things with the current API. #145 is a great example of several things people are doing because of the limitations. Take a look at that and tell me it's not hacky.

It probably seems easy for you because you use Redux. Not everyone uses Redux. And even when you use Redux, some use cases still need weird code to be addressed e.g. side-effects and when local state is involved.

It's not more complicated than it needs to be.

I strongly disagree. Literally anyone I talked to who needs to do any of the stuff I listed above has told me otherwise.

navigationOptions are static because some of them need to be read statically. For instance ... DrawerNavigator

Sure, this is an addition to the API. For navigators which need static configuration to work, can enforce them by throwing a descriptive error. The primary goal of this is to make it easier to update the options dynamically. They can still be initialized statically, either with a static navigationOptions or by specifying the options when creating the navigator.

As an unrelated note, it probably makes sense to also render all screens in drawer navigator by default similar to TabNavigator so that the local state of each screen inside the drawer is preserved when switching between them (#2277). Users could still decide if they want to render the actual content or not using the new focus events.

Since HOCs are often applied using a decorator, how about a navigationOptions decorator.

The issue with HOCs is probably the least important one and can be easily fixed. I mentioned is because it's a side-effect the API. A different API using an HOC/decorator is still not ideal and easy to incorrectly use since it relies on the order in which HOC/decorators are applied.

from rfcs.

grabbou avatar grabbou commented on May 2, 2024 2

Yeah, with the above example, we can easily set navigationOptions based on header prop w/o rendering the screen itself ahead of time.

from rfcs.

ravirajn22 avatar ravirajn22 commented on May 2, 2024 2

One year has completed since this RFC opened. Is any progress made on how to handle headerOptions using state directly?. Dynamically configuring NavigationOptions from screen state is one of the major bottleneck of this Library. @brentvatne @satya164 @ericvicenti @grabbou Pls have a fresh look into this.

from rfcs.

grabbou avatar grabbou commented on May 2, 2024 1

I second the API presented in previous comment. I've been partially prototyping this out in my free time recently, thanks to @ericvicenti pointing me towards that direction.

We had this idea of making it easy to just render a header component instead of title, right and other navigation options:

class HomeScreen extends React.Component({
  render() {
    return (
      <Screen header={<Header title="Welcome" />}>
        {...screenContent}
      </Screen>
    );
  }
});

This allows for easy customization of the header. If the header is not set on Screen, it will not be rendered at all. It also makes it easy for people to connect their state with the header which I guess has been one of the most common requests.

It's an interesting idea, but keep in mind that sometimes Header is shared between routes instead of being present inside the Route's card. This is the case on iOS and possibly the case if you are trying to implement Material Design style shared element transitions on Android like you see in Google's apps (try Inbox for example).

As @satya164 mentioned, this can be done with a similar to React's portal pattern - effectively, reusing the existing implementation of the header. However, I've been thinking to what degree we could achieve native-like look&feel of the header by having it separate on each screen and figuring out a new way for shared-element transitions.

from rfcs.

dantman avatar dantman commented on May 2, 2024 1

We'll probably have to store the result of navigationOptions and the setOptions state separately for the instance so we can update the setOptions state and just merge the two into the final options object.

Or we could just dive straight into your StackHeader/TabBarItem/etc component API and provide some backwards compatibility by checking navigationOptions in the navigator when the dynamic option has not been set.

from rfcs.

brentvatne avatar brentvatne commented on May 2, 2024 1

we are not working on this right now. we don't need a reminder of how much time has passed since this discussion started, it's here so that people can comment on it and provide their insights / ideas, and when someone wants to work on it, then it can be turned into a proper rfc and executed on.

from rfcs.

Yasser-G avatar Yasser-G commented on May 2, 2024 1

@toto104 You are Welcome, Use it wherever you like 😃 .

from rfcs.

dantman avatar dantman commented on May 2, 2024

It probably seems easy for you because you use Redux. Not everyone uses Redux. And even when you use Redux, some use cases still need weird code to be addressed e.g. side-effects and when local state is involved.

I used React Navigation before I used Redux, I only use Redux for a subset of things (global state that was already global state before I used Redux), and I have yet to use Redux in any navigationOptions code.

Side topic: I don't even know of a way to access Redux from navigationOptions. One of the issues I haven't solved yet is how I'm going to localize the navigationOptions.title used by both my headers and drawer once I implement Redux backed localization storage. (And setOptions/NavigationOptions won't change this because data needs to be static for the drawer)

from rfcs.

dantman avatar dantman commented on May 2, 2024

I'll agree that the idea of being able to set some 100% active route only options from the instance would look nice, namely:

this.props.navigation.setOptions({
  headerRight: // ... my buttons
});

However I do believe that there are solutions to the issue of needing to use params.onSubmit issue other than having the instance completely override the options defined by navigationOptions. Like some sort of message passing or possibly the eventEmitter that has already been partially implemented in React Navigation.

Sure, this is an addition to the API. For navigators which need static configuration to work, can enforce them by throwing a descriptive error. The primary goal of this is to make it easier to update the options dynamically. They can still be initialized statically, either with a static navigationOptions or by specifying the options when creating the navigator.

I don't like how this can result in some code do being non-DRY.

i.e. If I want to make my title vary based on state but I still need a static title because my component is used in a drawer then the "adding dynamic setOptions but keeping static navigationOptions" way of handling this is to have to repeat the title.

// Static navigationOptions
navigationOptions = {
  title: 'My Profile',
};

// Instance
this.props.navigation.setOptions({
  title: this.state.isEditing ? 'Editing My Profile' : 'My Profile',
});

In this example I can just move isEditing to params.isEditing and go back to using params.isEditing ? 'Editing My Profile' : 'My Profile' in navigationOptions instead of using setOptions. However if I run into any other situation where a param is both needed statically by a component but I need it to communicate with the instance when it's not static, I have to duplicate the option because setOptions can't be accessed by the static navigator and navigationOptions doesn't have an options<->instance communication method.

If you need share state between component and header, for example to show something in the header based on the screen's local state, you need to update params everytime your component state changes, now you have 2 sources of truth.

params are not header state, they are route params/state. You should not need 2 sources of truth. If you have a piece of state that is relevant to the header, say counter, then you should not be mirroring counter from state.counter into params.counter, you should be using params.counter in both the header and your route.

As an unrelated note, it probably makes sense to also render all screens in drawer navigator by default similar to TabNavigator so that the local state of each screen inside the drawer is preserved when switching between them (#2277). Users could still decide if they want to render the actual content or not using the new focus events.

I was going to say I don't like the fact that this means all my drawer routes that do async initialization on mount would now have to register focus handlers and implement complex "Is this the first focus, or am I already initialized" handling just to avoid every inactive drawer route from slowing down app-startup and fetching data that will be out of data if the user ever switches. But I suppose that the fact the lazy option was removed from TabNavigator when the focus events were added means this was the plan anyways.


I see 3 possible options/solutions to our problems:

  1. Keep the static navigationOptions and add setOptions/NavigationOptions.
    I don't like this option because of the DRY issue.
  2. Make DrawerNavigator render all routes and make all navigationOptions dynamic. i.e. The drawerNavigator will use the dynamic title instead of needing the static navigationOptions title.
    I'm ok with this idea. However requires a pretty significant change to how React Navigation works and gets options in order for this to work.
  3. Keep navigationOptions static as it is. But implement better methods for navigationOptions and the instance to communicate with each other to solve the params.onSubmit issue.
    I'm also ok with this.

from rfcs.

satya164 avatar satya164 commented on May 2, 2024

the instance completely override the options defined by navigationOption

It won't override, it will shallow merge them, similar to .setState.

Like some sort of message passing

ex-navigation has it, I don't remember exactly, but it can't be the same event API. there's no way to manage a subscription inside navigationOptions. You would only be able to use it inside components.

duplicate the option

No reason you can't use a constant for those rare occasions.

It's not much different from initializing state in constructor and then updating it later with .setState.

params are not header state, they are route params/state.

params are not state. They are closer props. A component controls it's own state, however a component cannot initialize it's params, they must come from parent. This doesn't make it easy to use params as a local state container. I'm not even sure it should hold component state. Navigation state is similar to a URL to me, if it doesn't make sense to be in the URL, it doesn't make sense to be in params.

focus handlers and implement complex "Is this the first focus, or am I already initialized" handling

It's just a boolean check, nothing complex. And it can be abstracted out to an HOC to reduce boilerplate.

Anyway, we can probably introduce a bunch of new API and new patterns to solve the problem we encounter. But we are writing React apps and the more we make the library play with React, the easier we make it to learn and use.

from rfcs.

satya164 avatar satya164 commented on May 2, 2024

Came of with another API when talking to @brentvatne about it

function App() {
  return (
    <Screen
      options={{
        title: 'Hello'
      }}
      render={({ navigation }) => {
        return <Text>Hello world</Text>
      }}
    />
  );
}

With this, static navigationOptions are not needed anymore! Probably unlikely to happen, but just putting it out there 😂

from rfcs.

dantman avatar dantman commented on May 2, 2024

@satya164 How do you intend to communicate with the header? Do you just mean that the header component is inside the <Screen> component and that component is used by the routes? Or that the <Screen> component handles communication the way <NavitationOptions> would have?

It's an interesting idea, but keep in mind that sometimes Header is shared between routes instead of being present inside the Route's card. This is the case on iOS and possibly the case if you are trying to implement Material Design style shared element transitions on Android like you see in Google's apps (try Inbox for example).

We also still need something for DrawerNavigator and TabNavigator where options are used in shared elements (drawer sidebar and tab labels) outside of the route.

^_^ This kind of thing might be easier if there was some way React's Portals would let you create a Portal inside React instead of just a Portal to a dom node.

from rfcs.

satya164 avatar satya164 commented on May 2, 2024

Header can be anywhere, doesn't have to be inside Screen. The options object could be communicated with the header via React's context. Is similar to React's Portal pattern.

from rfcs.

dantman avatar dantman commented on May 2, 2024

Ok. Though I don't see how it's different from <NavigationOptions>, aside from the page content being in a render prop instead of being a sibling.

from rfcs.

satya164 avatar satya164 commented on May 2, 2024

This component can be rendered all the time, but the screen content doesn't have to be.

from rfcs.

brentvatne avatar brentvatne commented on May 2, 2024

issue that would be solved easily by this: react-navigation/react-navigation#940

from rfcs.

ericvicenti avatar ericvicenti commented on May 2, 2024

Yeah, I think its a good idea to move the library in this direction. We need a nicer way of dynamic option configuration, and the convention here is rendering. But we should take an incremental approach, so the only thing that makes sense as the first step is navigation.setOptions . Maybe that would make for a good RFC?

@satya164, I'm curious how a component with static navigationOptions would behave if it also has a <NavigationOptions /> component inside of it. These options could conflict, no? I suppose we shouldn't support that usage at all.

Also I'd like to suggest that we have separate option config components and prop types for each navigator. That way we can implement cute things like this:

const HomeScreen = props =>
  <React.Fragment>
    <StackHeader
      title="Welcome"
      rightComponent={<MyButton />}
    />
    <HomeScreenContent />
    <TabBarItem icon={Icons.home} label="Home"/>
  <React.Fragment>

from rfcs.

dantman avatar dantman commented on May 2, 2024

@ericvicenti So the majority of navigator options that affect the UI (including the titles for headers, tabs, and drawers) get moved into render components and disappear from static navigationOptions?

I suppose we could even kill title itself. That would also solve my problem which is I have the same react-navigation/react-navigation#940 but for doing redux based i18n, except I want to be able to apply that to the navigationOptions.title used by the drawer in addition to the header, which hasn't been solved by most of the other suggestions that keep navigationOptions while adding dynamic options.

title probably isn't completely necessary if all the title handling is in render() where it's trivial to just use a title variable.

Just need to contemplate whether this may work with the iOS back title behaviour.

from rfcs.

satya164 avatar satya164 commented on May 2, 2024

I'm curious how a component with static navigationOptions would behave if it also has a component inside of it. These options could conflict, no?

My idea was to shallow merge the stuff from <NavigationOptions /> over static navigationOptions so that the config inside the component overrides the one defined statically. But not allowing makes sense if we wanna move away from the static configuration entirely.

except I want to be able to apply that to the navigationOptions.title used by the drawer in addition to the header, which hasn't been solved by most of the other suggestions that keep navigationOptions while adding dynamic options

This one can support all use cases without a static navigationOptions.

const HomeScreen = props =>
  <Screen
    header={<Header title="Welcome" />}
    render={() => {
      return <Text>Hello world</Text>
    }}
  />

from rfcs.

dantman avatar dantman commented on May 2, 2024

My idea was to shallow merge the stuff from over static navigationOptions so that the config inside the component overrides the one defined statically. But not allowing makes sense if we wanna move away from the static configuration entirely.

Don't forget that navigationOptions config isn't exactly defined statically if it's a function. You're not merging setOptions with a single unchanging object. You're merging it with the result of a function that is re-executed whenever params change.

from rfcs.

ericvicenti avatar ericvicenti commented on May 2, 2024

Don't forget that navigationOptions config isn't exactly defined statically if it's a function. You're not merging setOptions with a single unchanging object. You're merging it with the result of a function that is re-executed whenever params change.

Yeah, this is what has me confused. Seems like we will might momentarily show incorrect options on every props change because we get them from the static function and also in the options component

from rfcs.

dantman avatar dantman commented on May 2, 2024

@satya164 Is suggesting that this fixes the issue of context being inaccessible in screenProps, which leaves #5 open to removing screenProps.

Is this an accepted conclusion? If so then we are going to have to make sure that whatever solution we choose allows static properties such as title, drawerLabel, etc... access to context, because those properties can require things in context/screenProps as well (for example localizing drawer labels and the fallback title used for drawers, headers, back buttons, etc...).

from rfcs.

satya164 avatar satya164 commented on May 2, 2024

@dantman The solution we proposed in #24 (comment) should not require any static options since it allows to defer rendering of the screen to when necessary. It also works well with context and other patterns.

from rfcs.

dantman avatar dantman commented on May 2, 2024

@satya164 Sure, I'm just mentioning it because we have around 3 different proposals, some that still include static options, and don't seem to have consensus on exactly which one we want. And we're still talking about how to merge static navigationOptions with dynamic setOptions.

from rfcs.

lxcid avatar lxcid commented on May 2, 2024

I just add my 2 cents here against navigationOptions and for props & states:

  • As you use React, you often end up using HOCs, with navigationOptions and react-navigation, my HOCs get more complicate because I need to know where to hoist navigationOptions in my HOCs chain.
  • Sometimes, you need the navigation bar to participate with the screen state, there's no way to do this unless you pass the function through setParams, because of this workaround, we hit react-navigation/react-navigation#3867. All I wanted was to pass function but I couldn't do that without setParams(), I can't hoist the state (with withState) and pass down either.

I love react-navigation and its way better than native navigation but I agree with @satya164 that this is pattern is more hacky…

EDITED:

To work around this issue, redux actually come to the rescue, by hoisting the state all the way to redux, I can use the navigation.dispatch as an escape patch to communicate with the application as a whole.

from rfcs.

xXFracXx avatar xXFracXx commented on May 2, 2024

Any update?

from rfcs.

satya164 avatar satya164 commented on May 2, 2024

https://github.com/react-navigation/navigation-ex

from rfcs.

tomasf10 avatar tomasf10 commented on May 2, 2024

You Can call SetState or any other function inside your component from static navigationOptions using this simple lines

componentDidMount() {
        this.props.navigation.setParams({
            setState: this.setState.bind(this),
            myCustomFunction: myCustomFunction.bind(this),
        })
    }
static navigationOptions = ({ navigation }) => {
        const setState = navigation.getParam('setState', () => { })
        const myCustomFunction = navigation.getParam('myCustomFunction', () => { })
        return {
            headerRight: <TouchableOpacity onPress={() => setState({ buttonPressed: true })} />,
            headerLeft: <TouchableOpacity onPress={() => myCustomFunction()} />

        }
    }

Hope this may help someone.

Brilliant! Thanks! This help me to change the state of the modal visibility

from rfcs.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.