From 283512cc42aee833edabe90a8bd13c9d0e23db92 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Fri, 5 Mar 2021 10:23:42 -0800 Subject: [PATCH] Fix Yoga's right to left offset in horizontal scroll view Summary: Changelog: [internal] Yoga offsets content view of scrollview in RTL environment. React Native Classis deals with it by using a separate component [ScrollContentView](https://github.com/facebook/react-native/blob/6e6443afd04a847ef23fb6254a84e48c70b45896/React/Views/ScrollView/RCTScrollContentShadowView.m#L18-L25 ) and making the adjustment there. In New React Native Renderer, it can be handled inside `ScrollViewShadowNode`. Reviewed By: JoshuaGross Differential Revision: D26817121 fbshipit-source-id: ad48374ef19b802d25e919ac0aae05c5890762f2 --- .../scrollview/ScrollViewShadowNode.cpp | 22 +++++++++++++++++++ .../scrollview/ScrollViewShadowNode.h | 1 + .../components/view/tests/ViewTest.cpp | 4 ++++ .../tests/StateReconciliationTest.cpp | 13 ++++++----- 4 files changed, 35 insertions(+), 5 deletions(-) diff --git a/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp b/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp index a421f8c9de..96c0d5b556 100644 --- a/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp +++ b/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.cpp @@ -7,6 +7,7 @@ #include "ScrollViewShadowNode.h" +#include #include namespace facebook { @@ -30,10 +31,31 @@ void ScrollViewShadowNode::updateStateIfNeeded() { } } +void ScrollViewShadowNode::updateScrollContentOffsetIfNeeded() { +#ifndef ANDROID + react_native_assert( + children_->size() == 1 && "ScrollView only has single child"); + if (getLayoutMetrics().layoutDirection == LayoutDirection::RightToLeft) { + // Yoga place `contentView` on the right side of `scrollView` when RTL + // layout is enforced. To correct for this, in RTL setting, correct the + // frame's origin. React Native Classic does this as well in + // `RCTScrollContentShadowView.m`. + for (auto layoutableNode : getLayoutableChildNodes()) { + auto layoutMetrics = layoutableNode->getLayoutMetrics(); + if (layoutMetrics.frame.origin.x != 0) { + layoutMetrics.frame.origin.x = 0; + layoutableNode->setLayoutMetrics(layoutMetrics); + } + } + } +#endif +} + #pragma mark - LayoutableShadowNode void ScrollViewShadowNode::layout(LayoutContext layoutContext) { ConcreteViewShadowNode::layout(layoutContext); + updateScrollContentOffsetIfNeeded(); updateStateIfNeeded(); } diff --git a/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.h b/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.h index 19172a9e74..f4d4a5f964 100644 --- a/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.h +++ b/ReactCommon/react/renderer/components/scrollview/ScrollViewShadowNode.h @@ -36,6 +36,7 @@ class ScrollViewShadowNode final : public ConcreteViewShadowNode< private: void updateStateIfNeeded(); + void updateScrollContentOffsetIfNeeded(); }; } // namespace react diff --git a/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp b/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp index 0c5adbdba7..ab2d603ac9 100644 --- a/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp +++ b/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp @@ -62,6 +62,10 @@ class YogaDirtyFlagTest : public ::testing::Test { Element() .reference(scrollViewShadowNode_) .tag(7) + .children({ + Element() + .tag(8) + }) }) }); // clang-format on diff --git a/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp b/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp index 2b6ccfe7f1..c8f80e5b0f 100644 --- a/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp +++ b/ReactCommon/react/renderer/mounting/tests/StateReconciliationTest.cpp @@ -77,11 +77,14 @@ TEST(StateReconciliationTest, testStateReconciliation) { .reference(shadowNodeAB) .children({ Element() - .reference(shadowNodeABA), - Element() - .reference(shadowNodeABB), - Element() - .reference(shadowNodeABC) + .children({ + Element() + .reference(shadowNodeABA), + Element() + .reference(shadowNodeABB), + Element() + .reference(shadowNodeABC) + }) }) }); // clang-format on