Allow again for injecting custom root view via ReactActivityDelegate #26495
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
facebook-github-bot
approved these changes
Sep 19, 2019
There was a problem hiding this comment.
@mdvacca is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
This pull request was successfully merged by @kmagiera in 9f0dede. When will my fix make it into a release? | Upcoming Releases |
grabbou
pushed a commit
that referenced
this pull request
Sep 25, 2019
…26495) Summary: This change restores the possibility of injecting custom root views via ReactAcitivtyDelegate. It has been used by react-native-gesture-handler library in order to replace default root view with a one that'd route touch events to gesture-handler internal pipeline. The regression happened in d0792d4 where new `ReactDelegate` was introduced to provide support for rendering react native views in both Android fragments and activities. As a part of that change the logic responsible for creating root view has been moved from `ReactActivityDelegate` to `ReactDelegate` rendering `ReactActivityDelegate.createRootView` unused – that is there is no code path that leads to this method being called. Instead `ReactDelegate.createRootView` method has been added which now plays the same role. The custom root view injection was relying on overwriting that method and hence the method is no longer referenced overwriting it has no effect. Following the logic migration out of `ReactActivityDelegate` into `ReactDelegate` we could potentially now start overwriting methods of `ReactDelegate`. However when working with Android's activities in React Native we can only provide an instance of `ReactActivityDelegate` and in my opinion it does not make too much sense to expose also a way to provide own instance of `ReactDelegate`. The proposed fix was to route `ReactDelegate.createRootView` to call `ReactActivityDelegate.createRootView` and this way regaining control over root view creation to `ReactActivityDelgate`. The change of the behavior has been implemented by subclassing `ReactDelegate` directly from `ReactActivityDelegate` and hooking the aforementioned methods together. Thanks to this approach, the change has no effect on `ReactDelegate` being used directly from fragments or in other scenarios where it is not being instantiated from `ReactActivityDelegate`. This fixes an issue reported in software-mansion/react-native-gesture-handler#745 and discussed on 0.61 release thread: react-native-community/releases#140 (comment) ## Changelog [Internal] [Fixed] - Allow for custom root view to be injected via ReactActivityDelegate Pull Request resolved: #26495 Test Plan: 1. Run RNTester, take layout snapshot, see the react root view being on the top of view hierarchy. 2. Run gesture-handler Example app (or some other app that overwrites ReactActivityDelegate.createRootView method), on layout snapshot see custom root view being used. Differential Revision: D17482966 Pulled By: mdvacca fbshipit-source-id: 866f551b8b077bafe1eb9e34e5dccb1240fa935e
|
This commit will cause Blank screen for Android after just |
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.
Summary
This change restores the possibility of injecting custom root views via ReactAcitivtyDelegate. It has been used by react-native-gesture-handler library in order to replace default root view with a one that'd route touch events to gesture-handler internal pipeline.
The regression happened in d0792d4 where new
ReactDelegatewas introduced to provide support for rendering react native views in both Android fragments and activities. As a part of that change the logic responsible for creating root view has been moved fromReactActivityDelegatetoReactDelegaterenderingReactActivityDelegate.createRootViewunused – that is there is no code path that leads to this method being called. InsteadReactDelegate.createRootViewmethod has been added which now plays the same role. The custom root view injection was relying on overwriting that method and hence the method is no longer referenced overwriting it has no effect. Following the logic migration out ofReactActivityDelegateintoReactDelegatewe could potentially now start overwriting methods ofReactDelegate. However when working with Android's activities in React Native we can only provide an instance ofReactActivityDelegateand in my opinion it does not make too much sense to expose also a way to provide own instance ofReactDelegate.The proposed fix was to route
ReactDelegate.createRootViewto callReactActivityDelegate.createRootViewand this way regaining control over root view creation toReactActivityDelgate. The change of the behavior has been implemented by subclassingReactDelegatedirectly fromReactActivityDelegateand hooking the aforementioned methods together. Thanks to this approach, the change has no effect onReactDelegatebeing used directly from fragments or in other scenarios where it is not being instantiated fromReactActivityDelegate.This fixes an issue reported in software-mansion/react-native-gesture-handler#745 and discussed on 0.61 release thread: react-native-community/releases#140 (comment)
Changelog
[Internal] [Fixed] - Allow for custom root view to be injected via ReactActivityDelegate
Test Plan