Scroll viewer layoutbug (microsoft#2859)

* Moving picker fix * Add controls to vector and check ShadowNode func needsForceLayout * Clear m_needsForceLayout between calls to DoLayout
kmelmon · Aug 16, 2019 · f42a599 · f42a599
1 parent 922ef9e
commit f42a599
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 23 deletions.
diff --git a/change/react-native-windows-2019-07-31-16-18-01-ScrollViewerLayoutbug.json b/change/react-native-windows-2019-07-31-16-18-01-ScrollViewerLayoutbug.json
@@ -0,0 +1,8 @@
+{
+  "comment": "Moving picker fix",
+  "type": "prerelease",
+  "packageName": "react-native-windows",
+  "email": "decrowle@microsoft.com",
+  "commit": "8455d973a19be1d5e9bfee6329099d5657c64826",
+  "date": "2019-07-31T23:18:01.700Z"
+}
diff --git a/change/react-native-windows-extended-2019-08-02-13-57-41-ScrollViewerLayoutbug.json b/change/react-native-windows-extended-2019-08-02-13-57-41-ScrollViewerLayoutbug.json
@@ -0,0 +1,8 @@
+{
+  "comment": "Clear m_needsForceLayout between calls to DoLayout",
+  "type": "patch",
+  "packageName": "react-native-windows-extended",
+  "email": "decrowle@microsoft.com",
+  "commit": "a783433937d1558283ba587db05b76ab0981a34c",
+  "date": "2019-08-02T20:57:41.153Z"
+}
diff --git a/vnext/ReactUWP/Modules/NativeUIManager.cpp b/vnext/ReactUWP/Modules/NativeUIManager.cpp
@@ -901,6 +901,11 @@ void NativeUIManager::CreateView(
   auto *pViewManager = node.GetViewManager();
 
   if (pViewManager->RequiresYogaNode()) {
+    // Generate list of RN controls that need to have layout rerun on them.
+    if (node.NeedsForceLayout()) {
+      m_extraLayoutNodes.push_back(node.m_tag);
+    }
+
     auto result = m_tagsToYogaNodes.emplace(node.m_tag, make_yoga_node());
     if (result.second == true) {
       YGNodeRef yogaNode = result.first->second.get();
@@ -1011,17 +1016,22 @@ void NativeUIManager::UpdateExtraLayout(int64_t tag) {
   if (shadowNode == nullptr)
     return;
 
-  if (shadowNode->IsExternalLayoutDirty()) {
-    YGNodeRef yogaNode = GetYogaNode(tag);
-    if (yogaNode)
-      shadowNode->DoExtraLayoutPrep(yogaNode);
-  }
-
-  for (int64_t child : shadowNode->m_children)
+  for (int64_t child : shadowNode->m_children) {
     UpdateExtraLayout(child);
+  }
 }
 
 void NativeUIManager::DoLayout() {
+  // Process vector of RN controls needing extra layout here.
+  const auto extraLayoutNodes = m_extraLayoutNodes;
+  for (const int64_t tag : extraLayoutNodes) {
+    ShadowNodeBase &node =
+        static_cast<ShadowNodeBase &>(m_host->GetShadowNodeForTag(tag));
+    auto element = node.GetView().as<winrt::FrameworkElement>();
+    element.UpdateLayout();
+  }
+  // Values need to be cleared from the vector before next call to DoLayout.
+  m_extraLayoutNodes.clear();
   auto &rootTags = m_host->GetAllRootTags();
   for (int64_t rootTag : rootTags) {
     UpdateExtraLayout(rootTag);

diff --git a/vnext/ReactUWP/Modules/NativeUIManager.h b/vnext/ReactUWP/Modules/NativeUIManager.h
@@ -105,6 +105,7 @@ class NativeUIManager : public facebook::react::INativeUIManager {
   std::vector<winrt::Windows::UI::Xaml::FrameworkElement::SizeChanged_revoker>
       m_sizeChangedVector;
   std::vector<std::function<void()>> m_batchCompletedCallbacks;
+  std::vector<int64_t> m_extraLayoutNodes;
 
   std::map<int64_t, std::weak_ptr<IXamlReactControl>> m_tagsToXamlReactControl;
 };

diff --git a/vnext/ReactUWP/Views/PickerViewManager.cpp b/vnext/ReactUWP/Views/PickerViewManager.cpp
@@ -24,10 +24,7 @@ class PickerShadowNode : public ShadowNodeBase {
   PickerShadowNode();
   void createView() override;
   void updateProperties(const folly::dynamic &&props) override;
-  bool IsExternalLayoutDirty() const override {
-    return m_hasNewItems;
-  }
-  void DoExtraLayoutPrep(YGNodeRef yogaNode) override;
+  bool NeedsForceLayout() override;
 
  private:
   void RepopulateItems();
@@ -40,7 +37,6 @@ class PickerShadowNode : public ShadowNodeBase {
 
   folly::dynamic m_items;
   int32_t m_selectedIndex = -1;
-  bool m_hasNewItems = false;
 
   // FUTURE: remove when we can require RS5+
   bool m_isEditableComboboxSupported;
@@ -55,16 +51,6 @@ PickerShadowNode::PickerShadowNode() : Super() {
           L"Windows.UI.Xaml.Controls.ComboBox", L"IsEditableProperty");
 }
 
-void PickerShadowNode::DoExtraLayoutPrep(YGNodeRef yogaNode) {
-  if (!m_hasNewItems)
-    return;
-
-  m_hasNewItems = false;
-
-  auto comboBox = GetView().try_as<winrt::ComboBox>();
-  comboBox.UpdateLayout();
-}
-
 void PickerShadowNode::createView() {
   Super::createView();
   auto combobox = GetView().as<winrt::ComboBox>();
@@ -167,7 +153,6 @@ void PickerShadowNode::RepopulateItems() {
 
       comboBoxItems.Append(comboboxItem);
     }
-    m_hasNewItems = true;
   }
   if (m_selectedIndex < static_cast<int32_t>(m_items.size()))
     combobox.SelectedIndex(m_selectedIndex);
@@ -185,6 +170,10 @@ void PickerShadowNode::RepopulateItems() {
   instance.DispatchEvent(tag, "topChange", std::move(eventData));
 }
 
+bool PickerShadowNode::NeedsForceLayout() {
+  return true;
+}
+
 PickerViewManager::PickerViewManager(
     const std::shared_ptr<IReactInstance> &reactInstance)
     : Super(reactInstance) {}

diff --git a/vnext/ReactUWP/Views/ShadowNodeBase.cpp b/vnext/ReactUWP/Views/ShadowNodeBase.cpp
@@ -31,6 +31,10 @@ void ShadowNodeBase::createView() {
   m_view = GetViewManager()->CreateView(this->m_tag);
 }
 
+bool ShadowNodeBase::NeedsForceLayout() {
+  return false;
+}
+
 void ShadowNodeBase::dispatchCommand(
     int64_t commandId,
     const folly::dynamic &commandArgs) {

diff --git a/vnext/include/ReactUWP/Views/ShadowNodeBase.h b/vnext/include/ReactUWP/Views/ShadowNodeBase.h
@@ -74,6 +74,7 @@ struct REACTWINDOWS_EXPORT ShadowNodeBase : public facebook::react::ShadowNode {
   virtual void AddView(ShadowNode &child, int64_t index) override;
   virtual void RemoveChildAt(int64_t indexToRemove) override;
   virtual void createView() override;
+  virtual bool NeedsForceLayout();
 
   virtual void updateProperties(const folly::dynamic &&props) override;