Fix bug with empty DataTemplate (#1358)

* Fix bug with empty DataTemplate * Apply changes from code review * Update ItemsRepeater.h * Update ItemsRepeater.cpp
microsoft · Sep 24, 2019 · 7d9a12e · 7d9a12e
1 parent 0a1fc4c
commit 7d9a12e
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 6 deletions.
diff --git a/dev/Repeater/APITests/RepeaterTests.cs b/dev/Repeater/APITests/RepeaterTests.cs
@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.
+// Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See LICENSE in the project root for license information.
 
 using MUXControlsTestApp.Utilities;
@@ -145,6 +145,32 @@ public void ValidateGetSetItemsSource()
             });
         }
 
+        [TestMethod]
+        public void ValidateNoSizeWhenEmptyDataTemplate()
+        {
+
+            ItemsRepeater repeater = null;
+            RunOnUIThread.Execute(() =>
+            {
+                var elementFactory = new RecyclingElementFactory();
+                elementFactory.RecyclePool = new RecyclePool();
+                elementFactory.Templates["Item"] = (DataTemplate)XamlReader.Load(
+                    @"<DataTemplate xmlns='http://schemas.microsoft.com/winfx/2006/xaml/presentation' />");
+
+                repeater = new ItemsRepeater() {
+                    ItemsSource = Enumerable.Range(0, 10).Select(i => string.Format("Item #{0}", i)),
+                    ItemTemplate = elementFactory,
+                    // Default is StackLayout, so do not have to explicitly set.
+                    // Layout = new StackLayout(),
+                };
+                repeater.UpdateLayout();
+
+                // Asserting render size is zero
+                Verify.IsLessThan(repeater.RenderSize.Width , 0.0001);
+                Verify.IsLessThan(repeater.RenderSize.Height , 0.0001);
+            });
+        }
+
         [TestMethod]
         public void ValidateGetSetBackground()
         {

diff --git a/dev/Repeater/ItemsRepeater.cpp b/dev/Repeater/ItemsRepeater.cpp
@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.
+// Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See LICENSE in the project root for license information.
 
 #include <pch.h>
@@ -118,8 +118,15 @@ winrt::Size ItemsRepeater::MeasureOverride(winrt::Size const& availableSize)
         virtualContext->Indent(Indent());
 #endif
 
-        desiredSize = layout.Measure(layoutContext, availableSize);
-        extent = winrt::Rect{ m_layoutOrigin.X, m_layoutOrigin.Y, desiredSize.Width, desiredSize.Height };
+        // Checking if we have an data template and it is empty
+        if (m_isItemTemplateEmpty) {
+            // Has no content, so we will draw nothing and request zero space
+            extent = winrt::Rect{ m_layoutOrigin.X, m_layoutOrigin.Y, 0, 0 };
+        }
+        else {
+            desiredSize = layout.Measure(layoutContext, availableSize);
+            extent = winrt::Rect{ m_layoutOrigin.X, m_layoutOrigin.Y, desiredSize.Width, desiredSize.Height };
+        }
 
         // Clear auto recycle candidate elements that have not been kept alive by layout - i.e layout did not
         // call GetElementAt(index).
@@ -589,7 +596,8 @@ void ItemsRepeater::OnItemTemplateChanged(const winrt::IElementFactory& oldValue
         // UIAffinityQueue cleanup. To avoid that bug, take a strong ref
         m_itemTemplate = newValue;
     }
-
+    // Clear flag for bug #776
+    m_isItemTemplateEmpty = false;
     m_itemTemplateWrapper = newValue.try_as<winrt::IElementFactoryShim>();
     if (!m_itemTemplateWrapper)
     {
@@ -598,6 +606,10 @@ void ItemsRepeater::OnItemTemplateChanged(const winrt::IElementFactory& oldValue
         if (auto dataTemplate = newValue.try_as<winrt::DataTemplate>())
         {
             m_itemTemplateWrapper = winrt::make<ItemTemplateWrapper>(dataTemplate);
+            if (!dataTemplate.LoadContent().as<winrt::FrameworkElement>()) {
+                // We have a DataTemplate which is empty, so we need to set it to true
+                m_isItemTemplateEmpty = true;
+            }
         }
         else if (auto selector = newValue.try_as<winrt::DataTemplateSelector>())
         {

diff --git a/dev/Repeater/ItemsRepeater.h b/dev/Repeater/ItemsRepeater.h
@@ -175,4 +175,9 @@ class ItemsRepeater :
     winrt::IElementFactory m_itemTemplate{ nullptr };
     winrt::Layout m_layout{ nullptr };
     winrt::ElementAnimator m_animator{ nullptr };
+
+    // Bug where DataTemplate with no content causes a crash.
+    // See: https://github.com/microsoft/microsoft-ui-xaml/issues/776
+    // Solution: Have flag that is only true when DataTemplate exists but it is empty.
+    bool m_isItemTemplateEmpty{ false };
 };
diff --git a/dev/Repeater/TestUI/Samples/Defaults.xaml b/dev/Repeater/TestUI/Samples/Defaults.xaml
@@ -1,4 +1,4 @@
-<!-- Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. See LICENSE in the project root for license information. -->
+<!-- Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. See LICENSE in the project root for license information. -->
 <Page
     x:Class="MUXControlsTestApp.Samples.Defaults"
     xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
@@ -7,6 +7,7 @@
 
     <Grid Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
         <Grid.RowDefinitions>
+            <RowDefinition Height="auto" />
             <RowDefinition Height="auto" />
             <RowDefinition Height="*" />
         </Grid.RowDefinitions>
@@ -20,5 +21,10 @@
                 <controls:ItemsRepeater ItemsSource="{x:Bind Data}" />
             </ScrollViewer>
         </controls:ItemsRepeaterScrollHost>
+        <controls:ItemsRepeater x:Name="emptyDataTemplateRepeater" ItemsSource="{x:Bind Data}">
+            <controls:ItemsRepeater.ItemTemplate>
+                <DataTemplate />
+            </controls:ItemsRepeater.ItemTemplate>
+        </controls:ItemsRepeater>
     </Grid>
 </Page>