ScrollContentView and ScrollView cleanup (#2545)

* add ScrollContentViewManager, which is just a <View> but removes forking from react-native

* cleanup in ScrollViewManager

vnext/ReactUWP/Base/UwpReactInstance.cpp

@@ -27,6 +27,7 @@
 #include <Views/PopupViewManager.h>
 #include <Views/RawTextViewManager.h>
 #include <Views/RootViewManager.h>
+#include <Views/ScrollContentViewManager.h>
 #include <Views/ScrollViewManager.h>
 #include <Views/SwitchViewManager.h>
 #include <Views/TextInputViewManager.h>
@@ -93,6 +94,7 @@ REACTWINDOWS_API_(std::shared_ptr<facebook::react::IUIManager>) CreateUIManager(
   viewManagers.push_back(std::make_unique<PopupViewManager>(instance));
   viewManagers.push_back(std::make_unique<RawTextViewManager>(instance));
   viewManagers.push_back(std::make_unique<RootViewManager>(instance));
+  viewManagers.push_back(std::make_unique<ScrollContentViewManager>(instance));
   viewManagers.push_back(std::make_unique<ScrollViewManager>(instance));
   viewManagers.push_back(std::make_unique<SwitchViewManager>(instance));
   viewManagers.push_back(std::make_unique<TextViewManager>(instance));

vnext/ReactUWP/ReactUWP.vcxproj

@@ -202,6 +202,7 @@
     <ClInclude Include="Views\RawTextViewManager.h" />
     <ClInclude Include="Views\ReactControl.h" />
     <ClInclude Include="Views\RootViewManager.h" />
+    <ClInclude Include="Views\ScrollContentViewManager.h" />
     <ClInclude Include="Views\ScrollViewManager.h" />
     <ClInclude Include="Views\SwitchViewManager.h" />
     <ClInclude Include="Views\TextInputViewManager.h" />
@@ -276,6 +277,7 @@
     <ClCompile Include="Views\ReactControl.cpp" />
     <ClCompile Include="Views\ReactRootView.cpp" />
     <ClCompile Include="Views\RootViewManager.cpp" />
+    <ClCompile Include="Views\ScrollContentViewManager.cpp" />
     <ClCompile Include="Views\ScrollViewManager.cpp" />
     <ClCompile Include="Views\ShadowNodeBase.cpp" />
     <ClCompile Include="Views\SwitchViewManager.cpp" />

vnext/ReactUWP/ReactUWP.vcxproj.filters

@@ -209,12 +209,12 @@
     <ClCompile Include="Views\FlyoutViewManager.cpp">
       <Filter>Views</Filter>
     </ClCompile>
-
     <ClCompile Include="Views\Impl\ScrollViewUWPImplementation.cpp">
       <Filter>Views\Impl</Filter>
     </ClCompile>
-    <ClCompile Include="SnapPointManagingContentControl.cpp">
-      <Filter>Views\Impl</Filter>
+    <ClCompile Include="Views\Impl\SnapPointManagingContentControl.cpp" />
+    <ClCompile Include="Views\ScrollContentViewManager.cpp">
+      <Filter>Views</Filter>
     </ClCompile>
   </ItemGroup>
   <ItemGroup>
@@ -425,12 +425,12 @@
     <ClInclude Include="Views\FlyoutViewManager.h">
       <Filter>Views</Filter>
     </ClInclude>
-
     <ClInclude Include="Views\Impl\ScrollViewUWPImplementation.h">
       <Filter>Views\Impl</Filter>
     </ClInclude>
-    <ClInclude Include="SnapPointManagingContentControl.h">
-      <Filter>Views\Impl</Filter>
+    <ClInclude Include="Views\Impl\SnapPointManagingContentControl.h" />
+    <ClInclude Include="Views\ScrollContentViewManager.h">
+      <Filter>Views</Filter>
     </ClInclude>
   </ItemGroup>
   <ItemGroup>

vnext/ReactUWP/Views/Impl/ScrollViewUWPImplementation.cpp

@@ -12,41 +12,10 @@ ScrollViewUWPImplementation::ScrollViewUWPImplementation(const winrt::ScrollView
   assert(scrollViewContent);
   const auto snapPointManager = scrollViewContent.as<SnapPointManagingContentControl>();
   assert(snapPointManager);
-  assert(snapPointManager->Content().as<winrt::StackPanel>());
 
   m_scrollViewer = winrt::make_weak(scrollViewer);
 }
 
-void ScrollViewUWPImplementation::ConvertScrollViewer(const winrt::ScrollViewer& scrollViewer)
-{
-  if (scrollViewer)
-  {
-    const auto snapPointManager = new SnapPointManagingContentControl();
-    snapPointManager->Content(winrt::StackPanel{});
-
-    scrollViewer.Content(*snapPointManager);
-  }
-}
-
-void ScrollViewUWPImplementation::AddView(const XamlView& child, uint32_t index)
-{
-  const auto contentCollection = ScrollViewerContent().Children();
-  assert(index <= contentCollection.Size());
-  contentCollection.InsertAt(index, child.as<winrt::UIElement>());
-}
-
-void ScrollViewUWPImplementation::RemoveAllChildren()
-{
-  ScrollViewerContent().Children().Clear();
-}
-
-void ScrollViewUWPImplementation::RemoveChildAt(uint32_t index)
-{
-  const auto contentCollection = ScrollViewerContent().Children();
-  assert(index < contentCollection.Size());
-  contentCollection.RemoveAt(index);
-}
-
 void ScrollViewUWPImplementation::SetHorizontal(bool horizontal)
 {
   ScrollViewerSnapPointManager()->SetHorizontal(horizontal);
@@ -62,7 +31,7 @@ void ScrollViewUWPImplementation::SnapToOffsets(const winrt::IVectorView<float>&
   ScrollViewerSnapPointManager()->SnapToOffsets(offsets);
 }
 
-void ScrollViewUWPImplementation::UpdateScrollableSize()
+void ScrollViewUWPImplementation::UpdateScrollableSize() const
 {
   if (const auto scrollViewer = m_scrollViewer.get())
   {
@@ -94,16 +63,16 @@ void ScrollViewUWPImplementation::UpdateScrollableSize()
   }
 }
 
-winrt::ScrollViewer ScrollViewUWPImplementation::ScrollViewer()
+winrt::ScrollViewer ScrollViewUWPImplementation::ScrollViewer() const
 {
   return m_scrollViewer.get();
 }
 
 // privates //
 
-winrt::com_ptr<SnapPointManagingContentControl>ScrollViewUWPImplementation::ScrollViewerSnapPointManager()
+winrt::com_ptr<SnapPointManagingContentControl> ScrollViewUWPImplementation::ScrollViewerSnapPointManager() const
 {
-  if (const auto scrollViewer = m_scrollViewer.get())
+  if (const auto scrollViewer = ScrollViewer())
   {
     if (const auto content = scrollViewer.Content())
     {
@@ -113,16 +82,4 @@ winrt::com_ptr<SnapPointManagingContentControl>ScrollViewUWPImplementation::Scro
   return nullptr;
 }
 
-winrt::StackPanel ScrollViewUWPImplementation::ScrollViewerContent()
-{
-  if (const auto snapPointManagingContentControl = ScrollViewerSnapPointManager())
-  {
-    if (const auto content = snapPointManagingContentControl->Content())
-    {
-      return content.as<winrt::StackPanel>();
-    }
-  }
-  return nullptr;
-}
-
 } }

vnext/ReactUWP/Views/Impl/ScrollViewUWPImplementation.h

@@ -28,24 +28,17 @@ class ScrollViewUWPImplementation
 {
 public:
   ScrollViewUWPImplementation(const winrt::ScrollViewer& scrollViewer);
-  static void ConvertScrollViewer(const winrt::ScrollViewer& scrollViewer);
-
-  void AddView(const XamlView& child, uint32_t index);
-  void RemoveAllChildren();
-  void RemoveChildAt(uint32_t index);
 
   void SetHorizontal(bool isHorizontal);
   void SnapToInterval(float interval);
   void SnapToOffsets(const winrt::IVectorView<float>& offsets);
 
-  void UpdateScrollableSize();
+  void UpdateScrollableSize() const;
 
-  winrt::ScrollViewer ScrollViewer();
-  winrt::com_ptr<SnapPointManagingContentControl> ScrollViewerSnapPointManager();
+  winrt::ScrollViewer ScrollViewer() const;
+  winrt::com_ptr<SnapPointManagingContentControl> ScrollViewerSnapPointManager() const;
 
 private:
-  winrt::StackPanel ScrollViewerContent();
-
   winrt::weak_ref<winrt::ScrollViewer> m_scrollViewer{};
 };
 

vnext/ReactUWP/Views/Impl/SnapPointManagingContentControl.cpp

@@ -10,6 +10,11 @@ SnapPointManagingContentControl::SnapPointManagingContentControl()
 
 }
 
+/*static*/ winrt::com_ptr<SnapPointManagingContentControl> SnapPointManagingContentControl::Create()
+{
+  return winrt::make_self<SnapPointManagingContentControl>();
+}
+
 void SnapPointManagingContentControl::SnapToInterval(float interval)
 {
   if (interval != m_interval)

vnext/ReactUWP/Views/Impl/SnapPointManagingContentControl.h

@@ -15,9 +15,13 @@ namespace uwp {
 
   class SnapPointManagingContentControl : public winrt::ContentControlT<SnapPointManagingContentControl, winrt::IScrollSnapPointsInfo>
   {
-  public:
+  private:
     SnapPointManagingContentControl();
 
+  public:
+    static winrt::com_ptr<SnapPointManagingContentControl> Create();
+    template <typename D, typename... Args> friend auto winrt::make_self(Args&& ... args);
+
     // ScrollView Implementation
     void SnapToInterval(float interval);
     void SnapToOffsets(const winrt::IVectorView<float>& offsets);

vnext/ReactUWP/Views/ScrollContentViewManager.cpp

@@ -0,0 +1,20 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+#include "pch.h"
+
+#include "ScrollContentViewManager.h"
+
+namespace react { namespace uwp {
+
+ScrollContentViewManager::ScrollContentViewManager(const std::shared_ptr<IReactInstance>& reactInstance)
+  : Super(reactInstance)
+{
+}
+
+const char* ScrollContentViewManager::GetName() const
+{
+  return "RCTScrollContentView";
+}
+
+}}

vnext/ReactUWP/Views/ScrollContentViewManager.h

@@ -0,0 +1,19 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+#pragma once
+
+#include <Views/ViewViewManager.h>
+
+namespace react { namespace uwp {
+
+class ScrollContentViewManager : public ViewViewManager
+{
+  using Super = ViewViewManager;
+public:
+  ScrollContentViewManager(const std::shared_ptr<IReactInstance>& reactInstance);
+
+  const char* GetName() const override;
+};
+
+} }

vnext/ReactUWP/Views/ScrollViewManager.cpp

@@ -83,14 +83,15 @@ void ScrollViewShadowNode::createView()
   Super::createView();
 
   const auto scrollViewer = GetView().as<winrt::ScrollViewer>();
-  const auto scrollViewUWPImplementation = new ScrollViewUWPImplementation(scrollViewer);
+  const auto scrollViewUWPImplementation = ScrollViewUWPImplementation(scrollViewer);
+  scrollViewUWPImplementation.ScrollViewerSnapPointManager();
 
   AddHandlers(scrollViewer);
 
   m_scrollViewerSizeChangedRevoker = scrollViewer.SizeChanged(winrt::auto_revoke,
     [scrollViewUWPImplementation](const auto&, const auto&)
     {
-      scrollViewUWPImplementation->UpdateScrollableSize();
+      scrollViewUWPImplementation.UpdateScrollableSize();
     });
 
   m_scrollViewerViewChangedRevoker = scrollViewer.ViewChanged(winrt::auto_revoke,
@@ -101,14 +102,14 @@ void ScrollViewShadowNode::createView()
       if (m_zoomFactor != zoomFactor)
       {
         m_zoomFactor = zoomFactor;
-        scrollViewUWPImplementation->UpdateScrollableSize();
+        scrollViewUWPImplementation.UpdateScrollableSize();
       }
     });
 
-  m_contentSizeChangedRevoker = scrollViewUWPImplementation->ScrollViewerSnapPointManager()->SizeChanged(winrt::auto_revoke,
+  m_contentSizeChangedRevoker = scrollViewUWPImplementation.ScrollViewerSnapPointManager()->SizeChanged(winrt::auto_revoke,
     [this, scrollViewUWPImplementation](const auto&, const auto&)
     {
-      scrollViewUWPImplementation->UpdateScrollableSize();
+      scrollViewUWPImplementation.UpdateScrollableSize();
     });
 }
 
@@ -472,59 +473,40 @@ folly::dynamic ScrollViewManager::GetExportedCustomDirectEventTypeConstants() co
 
 XamlView ScrollViewManager::CreateViewCore(int64_t tag)
 {
-  const auto scrollViewer = [this]()
-  {
-    const auto scrollViewer = winrt::ScrollViewer{};
+  const auto scrollViewer = winrt::ScrollViewer{};
 
-    scrollViewer.HorizontalScrollBarVisibility(winrt::ScrollBarVisibility::Visible);
-    scrollViewer.VerticalScrollBarVisibility(winrt::ScrollBarVisibility::Visible);
-    scrollViewer.VerticalSnapPointsAlignment(winrt::SnapPointsAlignment::Near);
-    scrollViewer.VerticalSnapPointsType(winrt::SnapPointsType::Mandatory);
-    scrollViewer.HorizontalSnapPointsType(winrt::SnapPointsType::Mandatory);
+  scrollViewer.HorizontalScrollBarVisibility(winrt::ScrollBarVisibility::Visible);
+  scrollViewer.VerticalScrollBarVisibility(winrt::ScrollBarVisibility::Visible);
+  scrollViewer.VerticalSnapPointsAlignment(winrt::SnapPointsAlignment::Near);
+  scrollViewer.VerticalSnapPointsType(winrt::SnapPointsType::Mandatory);
+  scrollViewer.HorizontalSnapPointsType(winrt::SnapPointsType::Mandatory);
 
-    return scrollViewer;
-  }();
+  const auto snapPointManager = SnapPointManagingContentControl::Create();
+  scrollViewer.Content(*snapPointManager);
 
-  const auto scrollViewUWPImplementation = [scrollViewer]()
-  {
-    ScrollViewUWPImplementation::ConvertScrollViewer(scrollViewer);
-    return new ScrollViewUWPImplementation(scrollViewer);
-  }();
-
   return scrollViewer;
 }
 
 void ScrollViewManager::AddView(XamlView parent, XamlView child, int64_t index)
 {
-  if (parent && child && index >= 0)
-  {
-    if (const auto scrollViewer = parent.as<winrt::ScrollViewer>())
-    {
-      ScrollViewUWPImplementation(scrollViewer).AddView(child, static_cast<uint32_t>(index));
-    }
-  }
+  assert(index == 0);
+
+  auto scrollViewer = parent.as<winrt::ScrollViewer>();
+  auto snapPointManager = scrollViewer.Content().as<SnapPointManagingContentControl>();
+  snapPointManager->Content(child);
 }
 
 void ScrollViewManager::RemoveAllChildren(XamlView parent)
 {
-  if (parent)
-  {
-    if (const auto scrollViewer = parent.as<winrt::ScrollViewer>())
-    {
-      ScrollViewUWPImplementation(scrollViewer).RemoveAllChildren();
-    }
-  }
+  auto scrollViewer = parent.as<winrt::ScrollViewer>();
+  auto snapPointManager = scrollViewer.Content().as<SnapPointManagingContentControl>();
+  snapPointManager->Content(nullptr);
 }
 
 void ScrollViewManager::RemoveChildAt(XamlView parent, int64_t index)
 {
-  if (parent && index >= 0)
-  {
-    if (const auto scrollViewer = parent.as<winrt::ScrollViewer>())
-    {
-      ScrollViewUWPImplementation(scrollViewer).RemoveChildAt(static_cast<uint32_t>(index));
-    }
-  }
+  assert(index == 0);
+  RemoveAllChildren(parent);
 }
 
 void ScrollViewManager::SnapToInterval(XamlView parent, float interval)