fix(Android): jumping content on fabric (#2313)

## Description This PR applies modifications to a previous fix: #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 #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 <kacperkafara@gmail.com>
software-mansion · Aug 27, 2024 · 204bef3 · 204bef3
1 parent ea2d08d
commit 204bef3
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 98 deletions.
diff --git 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<RNSScreenShadowNode *>(&shadowNode));
     auto &screenShadowNode = static_cast<RNSScreenShadowNode &>(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<const RNSScreenStackHeaderConfigProps>(
-                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<std::reference_wrapper<const ShadowNode::Shared>>
-  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<float> 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
@@ -15,6 +15,107 @@ Point RNSScreenShadowNode::getContentOriginOffset(
   return {contentOffset.x, contentOffset.y};
 }
 
+std::optional<std::reference_wrapper<const ShadowNode::Shared>>
+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<float> 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<RNSScreenShadowNode &>(*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<const RNSScreenStackHeaderConfigProps>(
+              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
@@ -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