From 7a01afe03b7f3425ac286fc7d4a6ee7b62e5aec6 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Mon, 30 Mar 2020 04:00:43 -0700 Subject: [PATCH] Remove LayoutInspectingPolicy.includeScrollViewContentOffset Summary: `LayoutInspectingPolicy` has two flags, `includeTransform` and `includeScrollViewContentOffset`. `includeScrollViewContentOffset` seems to be redundant for two reasons. # 1st From looking at callers, they have always the same value. I looked at all call sites, and they are either always both set to true or both set to false. # 2nd The way we include scroll view content offset, is through transformation, so setting `includeTransform` to true and `includeScrollViewContentOffset` to false will include content offset anyway. In order to make both flags work, we would need to introduce further changes to `getRelativeLayoutMetrics`. But since the flag isn't used anyway, I think it is better to get rid of it for now. If we need it in the future, we could re-introduce it. Reviewed By: shergin Differential Revision: D20622256 fbshipit-source-id: fb6156c66b752319ea928239fa723ff90688b0a0 --- .../fabric/core/layout/LayoutableShadowNode.cpp | 2 +- .../fabric/core/layout/LayoutableShadowNode.h | 1 - ReactCommon/fabric/uimanager/UIManagerBinding.cpp | 12 ++++-------- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp index a23a6de045c..2108fd38263 100644 --- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp +++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp @@ -57,7 +57,7 @@ static LayoutMetrics calculateOffsetForLayoutMetrics( auto origin = layoutableCurrentShadowNode->getLayoutMetrics().frame.origin; - if (policy.includeTransform || policy.includeScrollViewContentOffset) { + if (policy.includeTransform) { // The check for ScrollView will be implemented after we have // a dedicated trait (part of `ShadowNodeTraits`) for that. origin = origin * layoutableCurrentShadowNode->getTransform(); diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h index 6c64dad40b4..53513285223 100644 --- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h +++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h @@ -46,7 +46,6 @@ class LayoutableShadowNode : public ShadowNode { class LayoutInspectingPolicy final { public: bool includeTransform{true}; - bool includeScrollViewContentOffset{true}; }; using UnsharedList = better:: diff --git a/ReactCommon/fabric/uimanager/UIManagerBinding.cpp b/ReactCommon/fabric/uimanager/UIManagerBinding.cpp index c300bcc828b..981a165bc66 100644 --- a/ReactCommon/fabric/uimanager/UIManagerBinding.cpp +++ b/ReactCommon/fabric/uimanager/UIManagerBinding.cpp @@ -454,8 +454,7 @@ jsi::Value UIManagerBinding::get( auto layoutMetrics = uiManager->getRelativeLayoutMetrics( *shadowNodeFromValue(runtime, arguments[0]), shadowNodeFromValue(runtime, arguments[1]).get(), - {/* .includeTransform = */ true, - /* .includeScrollViewContentOffset = */ true}); + {/* .includeTransform = */ true}); auto frame = layoutMetrics.frame; auto result = jsi::Object(runtime); result.setProperty(runtime, "left", frame.origin.x); @@ -499,8 +498,7 @@ jsi::Value UIManagerBinding::get( auto layoutMetrics = uiManager->getRelativeLayoutMetrics( *shadowNodeFromValue(runtime, arguments[0]), shadowNodeFromValue(runtime, arguments[1]).get(), - {/* .includeTransform = */ false, - /* .includeScrollViewContentOffset = */ false}); + {/* .includeTransform = */ false}); if (layoutMetrics == EmptyLayoutMetrics) { auto onFailFunction = @@ -536,8 +534,7 @@ jsi::Value UIManagerBinding::get( auto layoutMetrics = uiManager->getRelativeLayoutMetrics( *shadowNodeFromValue(runtime, arguments[0]), nullptr, - {/* .includeTransform = */ true, - /* .includeScrollViewContentOffset = */ true}); + {/* .includeTransform = */ true}); auto frame = layoutMetrics.frame; auto onSuccessFunction = arguments[1].getObject(runtime).getFunction(runtime); @@ -567,8 +564,7 @@ jsi::Value UIManagerBinding::get( auto layoutMetrics = uiManager->getRelativeLayoutMetrics( *shadowNodeFromValue(runtime, arguments[0]), nullptr, - {/* .includeTransform = */ true, - /* .includeScrollViewContentOffset = */ true}); + {/* .includeTransform = */ true}); auto onSuccessFunction = arguments[1].getObject(runtime).getFunction(runtime);