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

Support react-router-relay via applyRouterMiddleware. #23

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

joonhocho
Copy link

@jmurzy,
I finally got it working with react-router-relay.
Please take a look and let me know what you think.

@joonhocho
Copy link
Author

Related #21.

@jmurzy
Copy link
Owner

jmurzy commented Jun 18, 2016

Thanks. At first sight it looks good. Do you have an example repo with react-router-relay I can use to run a few tests?

I'm currently working on #17 which will require refactoring a few things internally. Once I land a fix for that, this will be much easier to merge. Please bare with me as I catch up on my backlog.

🍺

@jmurzy jmurzy added the WIP label Jun 18, 2016
@joonhocho
Copy link
Author

joonhocho commented Jun 18, 2016

I will definitely add examples tomorrow.

@joonhocho
Copy link
Author

joonhocho commented Jun 19, 2016

@jmurzy, Just created todo example with relay:
https://github.com/joonhocho/react-native-router-relay-todo

@jmurzy
Copy link
Owner

jmurzy commented Jun 20, 2016

Awesome! Thanks.

🍺

@joonhocho
Copy link
Author

@jmurzy,
There are currently few bugs I can spot with the todo app.
First, relay initially fetches data for /complete and /any. Not sure, why it fetches /complete.
Second, when you navigate to either complete tab, you will see two list all and complete on the screen. The same also happens for active tab.

@joonhocho
Copy link
Author

joonhocho commented Jun 22, 2016

I've been looking into why it was rendering two lists after switching tabs, and I've found that it's because I am using Route as a parent route instead of StackRoute or TabsRoute.

RouteView does not supply configureTransition and applyAnimation to NavigationAnimatedView unlike StackRouteView or TabsRouteView.

As result, NavigationAnimatedView._onProgressChange does not get called which is responsible for removing stale scenes.

Also, Relay initially fetching /complete and /any turned out to be not a bug. /complete is loaded by TodoListFooter.

I've noticed that NavigationAnimatedView is being killed. Will it fix the above RouteView issue of not having any transition animation?

@joonhocho
Copy link
Author

joonhocho commented Jun 23, 2016

More investigation on the bug.
Not having the applyAnimation on RouteView was not the cause of the issue.
The bug was caused by NavigationAnimatedView.componentDidUpdate.
Removing the if statement fixed it.

@joonhocho
Copy link
Author

facebook/react-native#7422 may be related.

@joonhocho
Copy link
Author

Hopefully new NavigationTransitioner fixes the issue.

@jmurzy
Copy link
Owner

jmurzy commented Jun 23, 2016

Yeah. Unfortunately though, patches that address that specific issue, facebook/react-native@c57bac4 and facebook/react-native@b4d15d3, did not make it into 0.29-rc. I'm trying to get those cherry picked into 0.29, facebook/react-native#8333.

#17 and #3 are also related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants