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

Navigator does not update background scenes on Android #387

Closed
a-s-o opened this issue Dec 15, 2017 · 8 comments
Closed

Navigator does not update background scenes on Android #387

a-s-o opened this issue Dec 15, 2017 · 8 comments
Labels

Comments

@a-s-o
Copy link

a-s-o commented Dec 15, 2017

Hi,

I have run into an issue with the navigator where going back to a previous screen having updated the state does not reflect the changes on the previous screen. I.e. the previous screen is stale and not re-rendered. It works fine on the web and the issue is only on Android (I am not yet setup to test on iOS).

My actual use case is for a page with list of things which when clicked show details of a particular item. Then there is a button on the details page to delete the item which on press triggers a prop method. The prop method emits an event to delete the item and also navigates back to the list as a side effect. I do the state change asynchronously so during the back animation the item detail page does not error out (due to missing item). However, after the navigation back the list screen should then update once the state is changed. This is not happening. Neither setState nor forceUpdate re-render the list screen which is the expected behavior.

I have created an isolated example that is a simple counter to illustrate the problem. Here is the basic flow:

Code for the example is at https://gist.github.com/a-s-o/e6424ba1fc0527daaf35e391ec01b074

Initial load

screenshot_1513349138

Navigate to second page and click emit event button

screenshot_1513349143

Show first page again (but problem: state has not updated!)

screenshot_1513349147

On the web, this works just fine. The problem is only on Android. Additionally, we can be sure that the state updated because if I go back to page 2 it shows the correct counter value of 1

Thanks in advance; any help is very much appreciated.

@a-s-o
Copy link
Author

a-s-o commented Dec 15, 2017

In the example above actually looks like page 1 never updates and always shows a counter value of 0; this is definitely wrong. Do we need a rerenderRoute() type method?

I presume that is not idiomatic React and the re-render should take place in response to setState or forceUpdate

@erictraut erictraut added the bug label Dec 25, 2017
@a-s-o
Copy link
Author

a-s-o commented Jan 4, 2018

Hello, just jumping back to share my current workaround for this issue just in case someone else need it. I have noticed that recreating the route manually will refresh it's state so I am doing that as follows.

I have a wrapper class which wraps the navigator and has a navigateTo() method. In this method, I conditionally refresh() the route each time I do anything other than a push.

The refresh method simply recreates whichever route happens to be on top when it is called.

Here is the code:

class MyWrapper {
  navigateTo(targetRouteId: number) {
    const currentRoutes = this.navigator.getCurrentRoutes()
    const currentIndex = currentRoutes.findIndex(route => route.routeId === targetRouteId)

    if (currentIndex > -1) {
      this.navigator.popToRoute(currentRoutes[currentIndex])
      this.refresh()  // FIXME: Workaround for https://github.com/Microsoft/reactxp/issues/387
    } else {
      this.navigator.push({
        targetRouteId,
        sceneConfigType: Types.NavigatorSceneConfigType.FloatFromRight
      })
    }
  }

  navigateBack() {
    this.navigator.pop()
    this.refresh()
  }

  private refresh() {
    const routes = this.navigator.getCurrentRoutes()
    const index = routes.length - 1
    if (index >= 0) {
      const { routeId, sceneConfigType } = routes[index]
      this.navigator.replaceAtIndex({ routeId, sceneConfigType }, index)
    }
  }
}

Additionally, I also call the refresh() method in a couple of other places, such as componentDidMount and the redux subscribe and the navigateBack method shown above.

@aperdec
Copy link

aperdec commented Apr 11, 2018

Having the same issue. Thanks @a-s-o for the workaround.

@erictraut
Copy link
Contributor

@dryganets, are you aware of this issue? Any thoughts?

@dryganets
Copy link
Contributor

dryganets commented Apr 12, 2018

@erictraut this is a known problem with experimental navigation.
Because of the way how scenes are cached in the framework you should treat the Scene properties as immutable.

I had a fix, but it was colossal perf regression for our app. I don't remember most of the details already. At the moment it was easier to work around rather than fix.

We moved all changing properties to the state for our scenes as a result - it was just a couple of places in our codebase.

I can try to fix it again if you think it is critical.

@thewulf7
Copy link
Member

thewulf7 commented Nov 6, 2018

@erictraut @dryganets is my fix is fine?

@dryganets
Copy link
Contributor

looks good, thank you!

@fbartho
Copy link
Contributor

fbartho commented Aug 9, 2020

Happy to close this ticket since there's a merged fix!

@fbartho fbartho closed this as completed Aug 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants
@dryganets @fbartho @thewulf7 @a-s-o @erictraut @aperdec and others