From 204bef3e2e6c9a3eabd3e58cac90cc458423a55f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Du=C5=BCy?= <91994767+alduzy@users.noreply.github.com> Date: Tue, 27 Aug 2024 11:19:16 +0200 Subject: [PATCH] fix(Android): jumping content on fabric (#2313) ## Description This PR applies modifications to a previous fix: https://github.com/software-mansion/react-native-screens/pull/2169 for fabric only, which has stopped working since RN `0.75`. In RN `0.74` the `adopt` in `RNSScreenComponentDescriptior.h` was once called without `stateData` but with children and we could then check if the `ScreenStackHeaderConfig` is present among them and make adjustments based on it. When working on https://github.com/software-mansion/react-native-screens/pull/2292 it became clear that the fix does not work anymore. Now the `adopt` is called either with no children and no `stateData` or with both. The solution is to move the code to `appendChild` in `RNSScreenShadowNode.cpp` so we can perform the adjustments as soon as the children append. ## Changes - moved code from `adopt` in `RNSScreenComponentDescriptior.h` to newly added `appendChild` override in `RNSScreenShadowNode.cpp` ## Screenshots / GIFs ### Before https://github.com/user-attachments/assets/6b76864b-58bb-4c6e-9f5b-a01bb0c88d2a ### After https://github.com/user-attachments/assets/98931e77-3877-4f67-8b28-f49d2e0f42ff ## Test code and steps to reproduce - Use `TestHeader` to test this change ## Checklist - [ ] Included code example that can be used to test this change - [ ] Updated TS types - [ ] Updated documentation: - [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx - [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx - [ ] Ensured that CI passes --------- Co-authored-by: Kacper Kafara --- .../rnscreens/RNSScreenComponentDescriptor.h | 101 +----------------- .../rnscreens/RNSScreenShadowNode.cpp | 101 ++++++++++++++++++ .../rnscreens/RNSScreenShadowNode.h | 2 + 3 files changed, 106 insertions(+), 98 deletions(-) diff --git a/common/cpp/react/renderer/components/rnscreens/RNSScreenComponentDescriptor.h b/common/cpp/react/renderer/components/rnscreens/RNSScreenComponentDescriptor.h index 983a128893..be47334120 100644 --- a/common/cpp/react/renderer/components/rnscreens/RNSScreenComponentDescriptor.h +++ b/common/cpp/react/renderer/components/rnscreens/RNSScreenComponentDescriptor.h @@ -19,9 +19,6 @@ class RNSScreenComponentDescriptor final public: using ConcreteComponentDescriptor::ConcreteComponentDescriptor; - static constexpr const char *kScreenDummyLayoutHelperClass = - "com/swmansion/rnscreens/utils/ScreenDummyLayoutHelper"; - void adopt(ShadowNode &shadowNode) const override { react_native_assert(dynamic_cast(&shadowNode)); auto &screenShadowNode = static_cast(shadowNode); @@ -39,7 +36,8 @@ class RNSScreenComponentDescriptor final #ifdef ANDROID if (stateData.frameSize.width != 0 && stateData.frameSize.height != 0) { // When we receive dimensions from JVM side we can remove padding used for - // correction, and we can stop applying height and offset corrections for the frame. + // correction, and we can stop applying height and offset corrections for + // the frame. // TODO: In future, when we have dynamic header height we might want to // update Y offset correction here. @@ -67,37 +65,10 @@ class RNSScreenComponentDescriptor final screenShadowNode.getFrameCorrectionModes().unset( FrameCorrectionModes::Mode::FrameHeightCorrection); screenShadowNode.getFrameCorrectionModes().unset( - FrameCorrectionModes::Mode::FrameOriginCorrection); + FrameCorrectionModes::Mode::FrameOriginCorrection); layoutableShadowNode.setSize( Size{stateData.frameSize.width, stateData.frameSize.height}); - } else { - // This code path should be executed only on the very first (few) - // layout(s), when we haven't received state update from JVM side yet. - - auto headerConfigChildOpt = findHeaderConfigChild(layoutableShadowNode); - - // During creation of the shadow node children are not attached yet. - // We also do not want to set any padding in case. - if (headerConfigChildOpt) { - const auto &headerConfigChild = headerConfigChildOpt->get(); - const auto &headerProps = - *std::static_pointer_cast( - headerConfigChild->getProps()); - - const auto headerHeight = headerProps.hidden - ? 0.f - : findHeaderHeight( - headerProps.titleFontSize, headerProps.title.empty()) - .value_or(0.f); - - screenShadowNode.setPadding({0, 0, 0, headerHeight}); - screenShadowNode.setHeaderHeight(headerHeight); - screenShadowNode.getFrameCorrectionModes().set( - FrameCorrectionModes::Mode( - FrameCorrectionModes::Mode::FrameHeightCorrection | - FrameCorrectionModes::Mode::FrameOriginCorrection)); - } } #else if (stateData.frameSize.width != 0 && stateData.frameSize.height != 0) { @@ -107,72 +78,6 @@ class RNSScreenComponentDescriptor final #endif // ANDROID ConcreteComponentDescriptor::adopt(shadowNode); } - - std::optional> - findHeaderConfigChild( - const YogaLayoutableShadowNode &screenShadowNode) const { - for (const ShadowNode::Shared &child : screenShadowNode.getChildren()) { - if (std::strcmp( - child->getComponentName(), "RNSScreenStackHeaderConfig") == 0) { - return {std::cref(child)}; - } - } - return {}; - } - -#ifdef ANDROID - std::optional findHeaderHeight( - const int fontSize, - const bool isTitleEmpty) const { - JNIEnv *env = facebook::jni::Environment::current(); - - if (env == nullptr) { - LOG(ERROR) << "[RNScreens] Failed to retrieve env\n"; - return {}; - } - - jclass layoutHelperClass = env->FindClass(kScreenDummyLayoutHelperClass); - - if (layoutHelperClass == nullptr) { - LOG(ERROR) << "[RNScreens] Failed to find class with id " - << kScreenDummyLayoutHelperClass; - return {}; - } - - jmethodID computeDummyLayoutID = - env->GetMethodID(layoutHelperClass, "computeDummyLayout", "(IZ)F"); - - if (computeDummyLayoutID == nullptr) { - LOG(ERROR) - << "[RNScreens] Failed to retrieve computeDummyLayout method ID"; - return {}; - } - - jmethodID getInstanceMethodID = env->GetStaticMethodID( - layoutHelperClass, - "getInstance", - "()Lcom/swmansion/rnscreens/utils/ScreenDummyLayoutHelper;"); - - if (getInstanceMethodID == nullptr) { - LOG(ERROR) << "[RNScreens] Failed to retrieve getInstanceMethodID"; - return {}; - } - - jobject packageInstance = - env->CallStaticObjectMethod(layoutHelperClass, getInstanceMethodID); - - if (packageInstance == nullptr) { - LOG(ERROR) - << "[RNScreens] Failed to retrieve packageInstance or the package instance was null on JVM side"; - return {}; - } - - jfloat headerHeight = env->CallFloatMethod( - packageInstance, computeDummyLayoutID, fontSize, isTitleEmpty); - - return {headerHeight}; - } -#endif // ANDROID }; } // namespace react diff --git a/common/cpp/react/renderer/components/rnscreens/RNSScreenShadowNode.cpp b/common/cpp/react/renderer/components/rnscreens/RNSScreenShadowNode.cpp index e97dc1a7de..506babc08b 100644 --- a/common/cpp/react/renderer/components/rnscreens/RNSScreenShadowNode.cpp +++ b/common/cpp/react/renderer/components/rnscreens/RNSScreenShadowNode.cpp @@ -15,6 +15,107 @@ Point RNSScreenShadowNode::getContentOriginOffset( return {contentOffset.x, contentOffset.y}; } +std::optional> +findHeaderConfigChild(const YogaLayoutableShadowNode &screenShadowNode) { + for (const ShadowNode::Shared &child : screenShadowNode.getChildren()) { + if (std::strcmp(child->getComponentName(), "RNSScreenStackHeaderConfig") == + 0) { + return {std::cref(child)}; + } + } + return {}; +} + +static constexpr const char *kScreenDummyLayoutHelperClass = + "com/swmansion/rnscreens/utils/ScreenDummyLayoutHelper"; + +#ifdef ANDROID +std::optional findHeaderHeight( + const int fontSize, + const bool isTitleEmpty) { + JNIEnv *env = facebook::jni::Environment::current(); + + if (env == nullptr) { + LOG(ERROR) << "[RNScreens] Failed to retrieve env\n"; + return {}; + } + + jclass layoutHelperClass = env->FindClass(kScreenDummyLayoutHelperClass); + + if (layoutHelperClass == nullptr) { + LOG(ERROR) << "[RNScreens] Failed to find class with id " + << kScreenDummyLayoutHelperClass; + return {}; + } + + jmethodID computeDummyLayoutID = + env->GetMethodID(layoutHelperClass, "computeDummyLayout", "(IZ)F"); + + if (computeDummyLayoutID == nullptr) { + LOG(ERROR) << "[RNScreens] Failed to retrieve computeDummyLayout method ID"; + return {}; + } + + jmethodID getInstanceMethodID = env->GetStaticMethodID( + layoutHelperClass, + "getInstance", + "()Lcom/swmansion/rnscreens/utils/ScreenDummyLayoutHelper;"); + + if (getInstanceMethodID == nullptr) { + LOG(ERROR) << "[RNScreens] Failed to retrieve getInstanceMethodID"; + return {}; + } + + jobject packageInstance = + env->CallStaticObjectMethod(layoutHelperClass, getInstanceMethodID); + + if (packageInstance == nullptr) { + LOG(ERROR) + << "[RNScreens] Failed to retrieve packageInstance or the package instance was null on JVM side"; + return {}; + } + + jfloat headerHeight = env->CallFloatMethod( + packageInstance, computeDummyLayoutID, fontSize, isTitleEmpty); + + return {headerHeight}; +} +#endif // ANDROID + +void RNSScreenShadowNode::appendChild(const ShadowNode::Shared &child) { + YogaLayoutableShadowNode::appendChild(child); +#ifdef ANDROID + const auto &stateData = getStateData(); + if (stateData.frameSize.width == 0 || stateData.frameSize.height == 0) { + // This code path should be executed only on the very first (few) + // layout(s), when we haven't received state update from JVM side yet. + auto headerConfigChildOpt = findHeaderConfigChild(*this); + auto &screenShadowNode = static_cast(*this); + + // During creation of the shadow node children are not attached yet. + // We also do not want to set any padding in case. + if (headerConfigChildOpt) { + const auto &headerConfigChild = headerConfigChildOpt->get(); + const auto &headerProps = + *std::static_pointer_cast( + headerConfigChild->getProps()); + + const auto headerHeight = headerProps.hidden + ? 0.f + : findHeaderHeight( + headerProps.titleFontSize, headerProps.title.empty()) + .value_or(0.f); + + screenShadowNode.setPadding({0, 0, 0, headerHeight}); + screenShadowNode.setHeaderHeight(headerHeight); + screenShadowNode.getFrameCorrectionModes().set(FrameCorrectionModes::Mode( + FrameCorrectionModes::Mode::FrameHeightCorrection | + FrameCorrectionModes::Mode::FrameOriginCorrection)); + } + } +#endif // ANDROID +} + void RNSScreenShadowNode::layout(facebook::react::LayoutContext layoutContext) { YogaLayoutableShadowNode::layout(layoutContext); diff --git a/common/cpp/react/renderer/components/rnscreens/RNSScreenShadowNode.h b/common/cpp/react/renderer/components/rnscreens/RNSScreenShadowNode.h index a6c5dc076e..6b4b72ec0e 100644 --- a/common/cpp/react/renderer/components/rnscreens/RNSScreenShadowNode.h +++ b/common/cpp/react/renderer/components/rnscreens/RNSScreenShadowNode.h @@ -28,6 +28,8 @@ class JSI_EXPORT RNSScreenShadowNode final : public ConcreteViewShadowNode< Point getContentOriginOffset(bool includeTransform) const override; + void appendChild(const ShadowNode::Shared &child) override; + void layout(LayoutContext layoutContext) override; #pragma mark - Custom interface