Updated TitleBar and ActionBar size in native on Fabric #619
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
Based this off of react native screens. Copied the RNSScreen hand written state/descriptor/shadownode at https://github.com/software-mansion/react-native-screens/tree/main/common/cpp/rnscreens. Need these to use c++ state. Updated podspec to include the cpp in the source files. Also set interfaceOnly in the native spec otherwise it generates some of these classes and so get a clash. Now hand writing descriptor, for example, so don't want react native codegen'ing it
So removing the change bounds workaround. Tested using issue #515
Saw that RCTModalHostViewComponentView.mm in React Native doesn't use the facebook::react:: namespace qualifier
Used c++ state - copied approach just added to title bar
This is more complicated than ios because can't work out how to get the hand-written files included with the generated ones. For now, manually copied them to the generated folder android/build/generated/source/codegen/jni/react/renderer/components/navigation-react-native - but they'll probably be overwritten so think might have to have a custom Android.mk? Also had to store the latest > 0 left position otherwise the title bar loses its left position when the screen is rotated. Must be a fabric bug that the left gets reset when updating node size?!
Want the include for the custom ComponentDescriptors to match the generated include #include <react/renderer/components/navigation-react-native/NVTitleBarComponentDescriptor.h> #include <react/renderer/components/navigation-react-native/NVSearchBarComponentDescriptor.h>
Haven't tested but copied over from title bar - haven't copied over the left field, will test to see if it's needed
It doesn't work reliably. The problem happens on orientation change with the title bar going back to left 0. Without this left change the problem always happens - prefer that for now. Should raise a react native bug because this is a change from old architecture. Might be because new arch doesn't call needsCustomLayoutForChildren?!
Releasing a wip so can make a repo to demonstrate regression caused by Fabric not calling needsCustomLayoutForChildren
Action bar and Title bar update node size on native
Needed now it's interfaceOnly where the component descriptor is hand-written (not codegen'ed)
Search bar and Title bar update node size on native
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.
Would’ve done this in #608 but it didn’t work because the new React Native architecture was missing the needsCustomLayoutForChildren check on Android. Worked around it by setting the size in React but this caused a flicker. The PR that adds the needsCustomLayoutForChildren check has just been merged into React Native so it’s safe to update the size in native (it’s in native on the old architecture).