fix: Make NativeViewHierarchyManager nullable
#2643
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
software-mansion#2638) ## Description ### Important context * [Android SDK 35 behaviour changes](https://developer.android.com/about/versions/15/behavior-changes-15#ux) * [status bar color API deprecation note](https://developer.android.com/reference/android/view/Window#setStatusBarColor(int)) * [navigation bar color API deprecation note](https://developer.android.com/reference/android/view/Window#setNavigationBarColor(int)) * [edge-to-edge on Android](https://developer.android.com/develop/ui/views/layout/edge-to-edge) For all apps targeting Android SDK 35 the edge-to-edge mode is enabled by default (it is effectively enforced, opting out takes some effort) and everything indicates that it will be enforced in future SDKs. For SDKs below 35 the status / navigation bar APIs affected by this PR are deprecated or deprecated & disabled (see above links ☝🏻 ). Currently minimal target sdk level for google app store is 34 ([source](https://median.co/blog/google-plays-target-api-level-requirement-for-android-apps)). We can't really foresee when SDK level 35 will be required, however it can be clearly seen that edge-to-edge will be the promoted and supported way of designing UI on Android. We want to get rid of these props for one more reason. These are Android-specific props and translucency is implemented by (not) consuming appropriate window insets, which leads to conflicts with other libs *and we can't set right defaults*, since it is not really possible to detect whether the app is in edge to edge or not leading to some buggy behaviour (initial content jump etc.). ## Changes - **Deprecate navigationBarColor** - **Deprecate navigationBarTranslucent** - **Deprecate statusBarColor** - **Deprecate statusBarTranslucent** - **Deprecate topInsetEnabled** ## Test code and steps to reproduce No runtime changes. ## Checklist - [ ] Ensured that CI passes
…g header subviews (software-mansion#2623) ## Description In software-mansion#2466 I've introduced `RCTMountingTransactionObserving` for `RNSScreenStackHeaderConfig` component. Now we can use this to reduce amount of calls to `updateViewControllerIfNeeded` (causing layout passes) when adding subviews. I'm not changing the `unmount` path of the code as we have some more additional logic there which requires some more careful consideration. This also aligns us with Paper implementation. Note that it has been only implemented this way, because at the implementation time there was no way to run this code on transaction completion. ## Changes `- [RNSScreenStackHeaderConfig updateViewControllerIfNeeded]` is now called only once per transaction when adding subviews to header config. ## Test code and steps to reproduce Test432, TestHeaderTitle, Test2552 - see that there are no regressions. > [!note] During testing I've found that there is a bug with subview layout when modifying subviews after first render. This is not a regression however. Notice the wrong position of header right on video below 👇🏻 (Test432) https://github.com/user-attachments/assets/7f8653e8-a7d9-4fb8-a875-182e7deb0495 ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
…oftware-mansion#2639) ## Description Closes software-mansion#2633 ## Changes Enhanced description of `formSheet` presentation style with information that nested stack rendering is not yet supported on Android. ## Checklist - [ ] Ensured that CI passes
… scenarios (software-mansion#2641) ## Description Fixes software-mansion#2631 I've given extensive description of both the issue ~and potential solution~ in software-mansion#2631 issue discussion * software-mansion#2631 In particular important parts are: * software-mansion#2631 (comment) * software-mansion#2631 (comment) I settled down on zeroing origin of the `FullWindowOverlay` frame in HostTree & setting `ShadowNodeTraits::RootNodeKind` for the custom shadow node of `FullWindowOverlay` component in the ShadowTree. This is much cleaner than managing the state & content offset manually. ## Changes `FullWindowOverlay` has now custom component descriptor, shadow node & shadow node state (its empty). The shadow node has `ShadowNodeTraits::RootNodeKind` set allowing it to be the reference point when computing position of any descendant view in shadow tree - this is fine, because we always expect `FullWindowOverlay` to have origin at `(0, 0)` in window coordinate space. In the HostTree we now ensure that `FWO` origin is at `(0, 0)` by overriding frame received during mounting stage. ## Test code and steps to reproduce Test2631 should now work not as in recording from issue description but correctly. ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
|
(this is a requirement for RN 78, otherwise the build fails.) |
|
Thanks! I'll merge it to aggregate PR for 0.78. |
kkafar
merged commit 12adf96
into
software-mansion:@kkafar/support-0.78
2 of 4 checks passed
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.
Description
The
NativeViewHierarchyManagerargument in.addUIBlockis nullable. (SeeUIBlock::execute(...))This PR fixes that definition and makes it nullable, and also checks for null. In our case, we likely want to exit out of that and just do nothing. (but lmk, I can also change to a throw)
Changes
NativeViewHierarchyManagerbeing nullableTest code and steps to reproduce
Checklist