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

mergeOptions does not update title component in Android #5377

Closed
tautvilas opened this issue Aug 14, 2019 · 9 comments · Fixed by #6066
Closed

mergeOptions does not update title component in Android #5377

tautvilas opened this issue Aug 14, 2019 · 9 comments · Fixed by #6066

Comments

@tautvilas
Copy link

Issue Description

If I set custom title component in android and later try to change it to another component with mergeOptions that does not work. On IOS it works fine.

Code example

// Screen methods
componentDidMount() {
    Navigation.mergeOptions(this.props.componentId, {
      topBar: {
        title: {
          component: {
            name: 'component.Comp1',
          }
        }
      }
    });
  }

onInteractiveAction() {
    Navigation.mergeOptions(this.props.componentId, {
      topBar: {
        title: {
          component: {
            name: 'component.Comp2',
          }
        }
      }
    });
  }

If onInteractiveAction is triggered by some touch action in screen Comp2 will become visible in IOS navbar but not in Android


Environment

  • React Native Navigation version: 3.0.0-alpha.7
  • React Native version: 0.60.4
  • Platform(s) (iOS, Android, or both?): Android
  • Device info: Pixel 2 android

Thanks for developing this amazing project :) RNN is the number one navigation solution for react native apps!

@tautvilas
Copy link
Author

tautvilas commented Aug 20, 2019

I have identified the root cause of the problem. This if condition always evaluates to true:

https://github.com/wix/react-native-navigation/blob/master/lib/android/app/src/main/java/com/reactnativenavigation/presentation/StackPresenter.java#L418

The View component does not change even if the options arrive with new RN component. That means the top bar component is never updated. The same problem should also happen for backgroundComponent

If you want I can do a PR for the fix, but need guidance how to solve it. In my case it would be best to remove this check completely because I don't want some hidden caching of RN components when I explicitly specify that I want to change top bar component with mergeOptions.

@guyca

@guyca
Copy link
Collaborator

guyca commented Aug 20, 2019

@tautvilas Any chance you could PR a failing Detox test? I'll pick it up from there

@tautvilas
Copy link
Author

I will try

@tautvilas
Copy link
Author

Hey, was this resolved?

@gp3gp3gp3
Copy link

Any update on this? This is still an issue as of version 3.2.0

@tautvilas
Copy link
Author

tautvilas commented Sep 16, 2019

This is a patch for StackPresenter.java that fixes the problem

418,420c418,420
<             if (titleControllers.containsKey(component)) {
<                 topBar.setTitleComponent(titleControllers.get(component).getView());
<             } else {
---
>             //if (titleControllers.containsKey(component)) {
>             //    topBar.setTitleComponent(titleControllers.get(component).getView());
>             //} else {
426c426
<             }
---
>             //}

micheleb added a commit to urbi-mobility/react-native-navigation that referenced this issue Nov 18, 2019
micheleb added a commit to urbi-mobility/react-native-navigation that referenced this issue Nov 18, 2019
@vodanh109
Copy link

Hi, any update for this issue?

@juddey
Copy link

juddey commented Jan 22, 2020

@vodanh109 Well, the patch still works 😉

guyca added a commit that referenced this issue Mar 22, 2020
This commit fixes a bug related to setting a new react component as title via mergeOptions. When a title component was set, and the user tried to replace the existing title with a new title via mergeOptions, the new title was not created and instead the current title was reapplied.
Fixes #5377
yogevbd pushed a commit that referenced this issue Mar 23, 2020
This commit fixes a bug related to setting a new react component as title via mergeOptions. When a title component was set, and the user tried to replace the existing title with a new title via mergeOptions, the new title was not created and instead the current title was reapplied.
Fixes #5377
@guyca
Copy link
Collaborator

guyca commented Apr 5, 2020

Hey guys, the fix is available in 6.4.0, sorry for taking so long to address this.

I understand this was mostly used to replace a component with the same component, only with different props. If all you need is to update props then you should be using Navigation.updateProps. This way the component isn't recreated and the existing component's props are updated.

stachu2k pushed a commit to stachu2k/react-native-navigation that referenced this issue Apr 8, 2020
This commit fixes a bug related to setting a new react component as title via mergeOptions. When a title component was set, and the user tried to replace the existing title with a new title via mergeOptions, the new title was not created and instead the current title was reapplied.
Fixes wix#5377
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants