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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 reverts commit 9651e0c.
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
This reverts commit 1a36c37.
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.
This reverts commit 22fe29e.
This reverts commit b1bcfd5.
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
This is awesome! 😀 |
Giant thank you for your NavigatorTest project and for pushing me in the right direction |
This was referenced Jun 12, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 newUIViewController
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.
TabBarIOS
component forUITabBarController
andunmountStyle
prop for custom Android animation