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

Moved to a Single React Root #245

Merged
merged 134 commits into from Jun 10, 2019
Merged

Moved to a Single React Root #245

merged 134 commits into from Jun 10, 2019

Conversation

grahammendick
Copy link
Owner

Fixes #241 Fixes #237

When working on a SearchBarIOS component couldn’t get the search bar to show up in the navigation bar. Each screen was a separate React root. Navigating to new screen first went to native code, which pushed a new UIViewController and rendered the new React root into the screen content. So the screen content was loaded after the new view controller was pushed. Because the search bar wasn’t rendered before the new view controller transitioned it didn’t show up.

@jacobp100 came to the rescue. His NavigatorTest project showed how to have a single React root for all screens, instead of one for each screen. With a single React root the timing problem disappears because the new screen is rendered in React before it’s added on the native side. Native pushes the new view controller and immediately sets its content from the added screen.

Although the search bar was the driver for the change, there are many other benefits.

  1. Don’t get a blank screen during a transition while waiting for the screen to render
  2. Screens share React context because they share a React root
  3. It’s all component and props. For example, there’s a TabBarIOS component for UITabBarController and unmountStyle prop for custom Android animation
  4. Requires no native setup so works on Expo if they can be persuaded to include it
  5. Don’t have to wait for transition to complete before iOS bar buttons appear

Renders all the Scenes in the stack, one for each crumb plus the current state. Copied the getScenes from NavigationMotion in mobile because that's its counterpart. Will need the rest of the scene data when it comes to animating on android anyway - can add props to get the animation instead of adding to the state
This is for react native 57.8 to match with package.json
Can't call suerp insert because then the react native grumbles that the parent react view controller aren't right. It's expecting the parent to be the host view controller but it's actually the navigation controller. Still need to track the subviews because gonna need them when swipe back and navigate I think because need client and server subviews to match (can't call setState when going back)
Need to think about how to handle back on ios. Need to rerender otherwise if go back and then navigate, React won't think adding a scene, just updating an existing one
Unfortunately it doesn't call removeReactSubview so done something wrong because it means that navigating back from react won't work. Probably do need to insertReactSubview to get the remove called
Otherwise the removeReactSubview isn't called. Without the remove then can't pop viewControllers when navigating back in React. Copied the styling from @jacobp100's NavigatorTest (it's a massive help)
Copied from @jacobp100 NavigatorTest again
If the real subview is added then when it's removed during a javascript back navigation the screen goes white when it's popped. Think it's because it's removed from the superview. Adding a placeholder ensures the real superview is retained in teh pop
This works now that inserting a dummy view. It was because the real view was removed from superview that made the screen go blank
When fluently navigated two scenes pushed two scenes but in between the didShow of the first push fired. It looked like a back navigation because the viewControllers < crumbs so it navigated back! Then the second push happened then the setState from the navigateBack which meant it popped the second push!
Applied pushes and pops together after all subview updates which prevents the first push firing the didShow
Changed the name to willAppear to match the ios naming convention. It's a Scene prop now instead of a native event - much better
It's all done through props and components now!
For example, don't want to navigate to invisible tab on ios. Wait until it's pressed
Navigate fluently from A --> B --> C then navigate back 2 to A. The NativeViewHierarchyManager throws layout error because the child Scene is null. Only clean up removed scenes if they were visited, had a child view
Instead of launchMode singleTop, used the default (standard) and set FLAG_ACTIVITY_CLEAR_TOP on up intent. Also means don't need alternate actiivity because default launch mode adds activities on stack.
This is significant because it can be used in expo because there's no setup step for android or ios (provided expo include it)
Fluently navigate from A --> B --> C and navigate back one to B. If sharedElements return non-empty array then it went down the finishAfterTransition path. But postponeEnterTransition hadn't been called on B, because B was fluently navigated over, so it threw and error.
Navigate A --> B --> C with shared element going from A to B. Then press back twice and the shared element didn't show because the onNewItent fired when going back to B which detached SharedElementManger from window. Left the attach listener so it's restored when the view is immediately added back to hierarchy in new intent
Means can remove the attach listener. The removeViewAt handles the clean up anyway
The Android intent to actiivity look up is a nightmare. When navigateUpTo and pass intent it won't find the activity that was created for that intent!!! It finds any activity with a matching type!? A --> B --> C --> D and navigate back 2 will use the activity for C because both have SceneActivtiy type and C is latest. This isn't a problem normally because onNewIntent finds the right SceneView and adds it as a child. The problem happens with shared elements transitions. If there was a shared el transition when going from A to B, then want it to run when going back from B to A. But the actual activity is C (not B) because it's been reused. So if call finishAfterTransition current activity it will go back to B not A!?! craziness.
The only way I could find around it is to ensure all activities have a different type. This guarantees that navigateUpTo always finds the right activity which means shared element navigation works. The catch is that can't there's a finite number of activity types (created 20). So disabled shared elements for back navigation from crumb 21 and beyond.
Now activities are always tied to their crumbs no need for this check
@grahammendick grahammendick merged commit 51fcdaf into master Jun 10, 2019
@grahammendick grahammendick deleted the single-root branch June 10, 2019 10:30
@jacobp100
Copy link

This is awesome! 😀

@grahammendick
Copy link
Owner Author

Giant thank you for your NavigatorTest project and for pushing me in the right direction

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

Successfully merging this pull request may close these issues.

Updating context only works in a single route? Native global React context out of route
2 participants