Fix bug with calling SwipeItems Clear function would crash (#1383)

* Add checks to fix bug with clearing swipeitems * Add test page * Add unit test for calling Clear on SwipeItems objects * Add vertical scroll items * Fix faulty unit test for SwipeItems crashing when calling clear
microsoft · Oct 2, 2019 · 18ef85a · 18ef85a
1 parent 2c8f540
commit 18ef85a
Show file tree

Hide file tree

Showing 7 changed files with 227 additions and 4 deletions.
diff --git a/dev/SwipeControl/SwipeControl.cpp b/dev/SwipeControl/SwipeControl.cpp
@@ -1440,7 +1440,10 @@ void SwipeControl::OnLeftItemsChanged(const winrt::IObservableVector<winrt::Swip
     SWIPECONTROL_TRACE_INFO(*this, TRACE_MSG_METH, METH_NAME, this);
 
     ThrowIfHasVerticalAndHorizontalContent();
-    m_interactionTracker.get().Properties().InsertBoolean(s_hasLeftContentPropertyName, sender.Size() > 0);
+    if (m_interactionTracker)
+    {
+        m_interactionTracker.get().Properties().InsertBoolean(s_hasLeftContentPropertyName, sender.Size() > 0);
+    }
 
     if (m_createdContent == CreatedContent::Left)
     {
@@ -1453,7 +1456,11 @@ void SwipeControl::OnRightItemsChanged(const winrt::IObservableVector<winrt::Swi
     SWIPECONTROL_TRACE_INFO(*this, TRACE_MSG_METH, METH_NAME, this);
 
     ThrowIfHasVerticalAndHorizontalContent();
-    m_interactionTracker.get().Properties().InsertBoolean(s_hasRightContentPropertyName, sender.Size() > 0);
+
+    if (m_interactionTracker)
+    {
+        m_interactionTracker.get().Properties().InsertBoolean(s_hasRightContentPropertyName, sender.Size() > 0);
+    }
 
     if (m_createdContent == CreatedContent::Right)
     {
@@ -1466,7 +1473,10 @@ void SwipeControl::OnTopItemsChanged(const winrt::IObservableVector<winrt::Swipe
     SWIPECONTROL_TRACE_INFO(*this, TRACE_MSG_METH, METH_NAME, this);
 
     ThrowIfHasVerticalAndHorizontalContent();
-    m_interactionTracker.get().Properties().InsertBoolean(s_hasTopContentPropertyName, sender.Size() > 0);
+    if (m_interactionTracker)
+    {
+        m_interactionTracker.get().Properties().InsertBoolean(s_hasTopContentPropertyName, sender.Size() > 0);
+    }
 
     if (m_createdContent == CreatedContent::Top)
     {
@@ -1479,7 +1489,10 @@ void SwipeControl::OnBottomItemsChanged(const winrt::IObservableVector<winrt::Sw
     SWIPECONTROL_TRACE_INFO(*this, TRACE_MSG_METH, METH_NAME, this);
 
     ThrowIfHasVerticalAndHorizontalContent();
-    m_interactionTracker.get().Properties().InsertBoolean(s_hasBottomContentPropertyName, sender.Size() > 0);
+    if (m_interactionTracker)
+    {
+        m_interactionTracker.get().Properties().InsertBoolean(s_hasBottomContentPropertyName, sender.Size() > 0);
+    }
 
     if (m_createdContent == CreatedContent::Bottom)
     {

diff --git a/dev/SwipeControl/SwipeControl_InteractionTests/SwipeControlTests.cs b/dev/SwipeControl/SwipeControl_InteractionTests/SwipeControlTests.cs
@@ -149,6 +149,63 @@ public void CanSwipeToExecuteVertical()
             }
         }
 
+        [TestMethod]
+        public void CanClearItemsWithoutCrashing()
+        {
+            if (PlatformConfiguration.IsOSVersionLessThan(OSVersion.Redstone5))
+            {
+                Log.Warning("This test relies on touch input, the injection of which is only supported in RS5 and up. Test is disabled.");
+                return;
+            }
+
+            using (var setup = new TestSetupHelper("SwipeControl Tests"))
+            {
+                if (!PlatformConfiguration.IsOsVersionGreaterThanOrEqual(OSVersion.Redstone2))
+                {
+                    Log.Warning("Test is disabled because RS1 doesn't have the right interaction tracker APIs.");
+                    return;
+                }
+
+
+                Log.Comment("Navigating to clear items test page");
+                UIObject navigateToClearPageObject = FindElement.ByName("navigateToClear");
+                Verify.IsNotNull(navigateToClearPageObject, "Verifying that navigateToClear Button was found");
+
+                Button navigateToClearPageButton = new Button(navigateToClearPageObject);
+                navigateToClearPageButton.Invoke();
+                Wait.ForIdle();
+
+                Log.Comment("Find FindItemsSum textblock");
+                TextBlock sumOfSwipeItemsCount = new TextBlock(FindElement.ByName("SwipeItemsChildSum"));
+                Verify.IsNotNull(sumOfSwipeItemsCount);
+                Verify.AreEqual("2", sumOfSwipeItemsCount.GetText());
+
+                Log.Comment("Find clear SwipeItems button");
+                Button clearItemsButton = new Button(FindElement.ByName("ClearItemsButton"));
+                Verify.IsNotNull(clearItemsButton);
+                clearItemsButton.Invoke();
+                Wait.ForIdle();
+                Verify.AreEqual("0", sumOfSwipeItemsCount.GetText());
+
+
+                Log.Comment("Find add SwipeItem button");
+                Button addItemsButton = new Button(FindElement.ByName("AddItemsButton"));
+                Verify.IsNotNull(addItemsButton);
+                addItemsButton.Invoke();
+                Wait.ForIdle();
+                // Only adds horizontal items, see test app for explanation
+                Verify.AreEqual("1", sumOfSwipeItemsCount.GetText());
+
+                Log.Comment("clearing items again");
+                clearItemsButton.Invoke();
+                Wait.ForIdle();
+                Verify.AreEqual("0", sumOfSwipeItemsCount.GetText());
+
+
+            }
+        }
+
+
         [TestMethod]
         public void CanSwipeAndTapFirstRevealedItemHorizontal()
         {

diff --git a/dev/SwipeControl/SwipeControl_TestUI/SwipeControlClearPage.xaml b/dev/SwipeControl/SwipeControl_TestUI/SwipeControlClearPage.xaml
@@ -0,0 +1,94 @@
+<!-- Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. See LICENSE in the project root for license information. -->
+<local:TestPage
+    xmlns:muxc="using:Microsoft.UI.Xaml.Controls"
+    x:Class="MUXControlsTestApp.SwipeControlClearPage"
+    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
+    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
+    xmlns:local="using:MUXControlsTestApp"
+    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
+    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
+    mc:Ignorable="d"
+    Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
+
+
+    <Grid>
+        <Grid.Resources>
+            <muxc:SwipeItems x:Name="DefaultSwipeItemsHorizontal" >
+                <muxc:SwipeItem Background="Orange" BehaviorOnInvoked="RemainOpen"></muxc:SwipeItem>
+            </muxc:SwipeItems>
+            <muxc:SwipeItem x:Name="DefaultSwipeItemHorizontal" Background="Orange" BehaviorOnInvoked="RemainOpen"></muxc:SwipeItem>
+            <muxc:SwipeItems x:Name="DefaultSwipeItemsVertical">
+                <muxc:SwipeItem Background="Orange" BehaviorOnInvoked="RemainOpen"></muxc:SwipeItem>
+            </muxc:SwipeItems>
+            <muxc:SwipeItem x:Name="DefaultSwipeItemVertical" Background="Orange" BehaviorOnInvoked="RemainOpen"></muxc:SwipeItem>
+        </Grid.Resources>
+        <Grid.RowDefinitions>
+            <RowDefinition Height="Auto"></RowDefinition>
+        </Grid.RowDefinitions>
+
+        <Grid.ColumnDefinitions>
+            <ColumnDefinition Width="Auto" ></ColumnDefinition>
+            <ColumnDefinition Width="Auto" ></ColumnDefinition>
+        </Grid.ColumnDefinitions>
+
+        <Grid Grid.Row="0" Grid.Column="0" Margin="20">
+            <Grid.RowDefinitions>
+                <RowDefinition Height="100"></RowDefinition>
+                <RowDefinition Height="20"></RowDefinition>
+                <RowDefinition Height="100"></RowDefinition>
+            </Grid.RowDefinitions>
+
+            <Grid.ColumnDefinitions>
+                <ColumnDefinition Width="120"></ColumnDefinition>
+                <ColumnDefinition Width="0"></ColumnDefinition>
+                <ColumnDefinition Width="120"></ColumnDefinition>
+            </Grid.ColumnDefinitions>
+            <ListView  Grid.Row="0" Grid.Column="0" x:Name="leftSwipe">
+                <ListView.ItemTemplate>
+                    <DataTemplate>
+                        <muxc:SwipeControl LeftItems="{StaticResource DefaultSwipeItemsHorizontal}">
+                            <Grid Background="Red" Width="100" Height="100" />
+                        </muxc:SwipeControl>
+                    </DataTemplate>
+                </ListView.ItemTemplate>
+            </ListView>
+            <ListView Grid.Row="0" Grid.Column="2" x:Name="topSwipe">
+                <ListView.ItemTemplate>
+                    <DataTemplate>
+                        <muxc:SwipeControl TopItems="{StaticResource DefaultSwipeItemsVertical}">
+                            <Grid Background="Green" Width="100" Height="100" />
+                        </muxc:SwipeControl>
+                    </DataTemplate>
+                </ListView.ItemTemplate>
+            </ListView>
+            <ListView Grid.Row="2" Grid.Column="0" x:Name="rightSwipe">
+                <ListView.ItemTemplate>
+                    <DataTemplate>
+                        <muxc:SwipeControl RightItems="{StaticResource DefaultSwipeItemsHorizontal}">
+                            <Grid Background="Blue" Width="100" Height="100" />
+                        </muxc:SwipeControl>
+                    </DataTemplate>
+                </ListView.ItemTemplate>
+            </ListView>
+            <ListView Grid.Row="2" Grid.Column="2" x:Name="bottomSwipe">
+                <ListView.ItemTemplate>
+                    <DataTemplate>
+                        <muxc:SwipeControl BottomItems="{StaticResource DefaultSwipeItemsVertical}">
+                            <Grid Background="Yellow" Width="100" Height="100"/>
+                        </muxc:SwipeControl>
+                    </DataTemplate>
+                </ListView.ItemTemplate>
+            </ListView>
+
+
+
+        </Grid>
+
+        <StackPanel Grid.Row="0" Grid.Column="1">
+            <Button Click="AddSwipeItemsButton_Click" x:Name="AddItemsButton" AutomationProperties.Name="AddItemsButton">Add SwipeItems</Button>
+            <Button Click="ClearSwipeItemsButton_Click" x:Name="ClearItemsButton" AutomationProperties.Name="ClearItemsButton">Clear SwipeItems</Button>
+            <TextBlock x:Name="SwipeItemsChildSum" AutomationProperties.Name="SwipeItemsChildSum" ></TextBlock>
+        </StackPanel>
+
+    </Grid>
+</local:TestPage>
diff --git a/dev/SwipeControl/SwipeControl_TestUI/SwipeControlClearPage.xaml.cs b/dev/SwipeControl/SwipeControl_TestUI/SwipeControlClearPage.xaml.cs
@@ -0,0 +1,50 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License. See LICENSE in the project root for license information.
+
+using System;
+using Windows.UI.Xaml;
+
+namespace MUXControlsTestApp
+{
+    /// <summary>
+    /// Test page used for clearing existing SwipeControls
+    /// </summary>
+    public sealed partial class SwipeControlClearPage : TestPage
+    {
+        private string[] items = new string[] { "some text" };
+
+        public SwipeControlClearPage()
+        {
+            this.InitializeComponent();
+            SwipeItemsChildSum.Text = (DefaultSwipeItemsHorizontal.Count + DefaultSwipeItemsVertical.Count).ToString();
+
+            leftSwipe.ItemsSource = items;
+            topSwipe.ItemsSource = items;
+            rightSwipe.ItemsSource = items;
+            bottomSwipe.ItemsSource = items;
+        }
+
+        public void AddSwipeItemsButton_Click(object sender, RoutedEventArgs e)
+        {
+            DefaultSwipeItemsHorizontal.Clear();
+            DefaultSwipeItemsVertical.Clear();
+
+            DefaultSwipeItemsHorizontal.Mode = Microsoft.UI.Xaml.Controls.SwipeMode.Reveal;
+            DefaultSwipeItemsHorizontal.Add(DefaultSwipeItemHorizontal);
+
+            // Using swipecontrol inside datatemplate prevents us from setting that:
+            // Swipecontrol is in horizontal mode, can not add vertical swipe items...
+            //DefaultSwipeItemsVertical.Mode = Microsoft.UI.Xaml.Controls.SwipeMode.Reveal;
+            //DefaultSwipeItemsVertical.Add(DefaultSwipeItemVertical);
+
+            SwipeItemsChildSum.Text = (DefaultSwipeItemsHorizontal.Count + DefaultSwipeItemsVertical.Count).ToString();
+        }
+        public void ClearSwipeItemsButton_Click(object sender, RoutedEventArgs e)
+        {
+            DefaultSwipeItemsHorizontal.Clear();
+            DefaultSwipeItemsVertical.Clear();
+            SwipeItemsChildSum.Text = (DefaultSwipeItemsHorizontal.Count + DefaultSwipeItemsVertical.Count).ToString();
+        }
+
+    }
+}
diff --git a/dev/SwipeControl/SwipeControl_TestUI/SwipeControl_TestUI.projitems b/dev/SwipeControl/SwipeControl_TestUI/SwipeControl_TestUI.projitems
@@ -10,6 +10,9 @@
     <Import_RootNamespace>SwipeControl_TestUI</Import_RootNamespace>
   </PropertyGroup>
   <ItemGroup>
+    <Compile Include="$(MSBuildThisFileDirectory)SwipeControlClearPage.xaml.cs" >
+      <DependentUpon>SwipeControlClearPage.xaml</DependentUpon>
+    </Compile>
     <Compile Include="$(MSBuildThisFileDirectory)SwipeControlPage2.xaml.cs">
       <DependentUpon>SwipeControlPage2.xaml</DependentUpon>
     </Compile>
@@ -22,6 +25,10 @@
     </Compile>
   </ItemGroup>
   <ItemGroup>
+    <Page Include="$(MSBuildThisFileDirectory)SwipeControlClearPage.xaml">
+      <SubType>Designer</SubType>
+      <Generator>MSBuild:Compile</Generator>
+    </Page>
     <Page Include="$(MSBuildThisFileDirectory)SwipeControlPage.xaml">
       <SubType>Designer</SubType>
       <Generator>MSBuild:Compile</Generator>

diff --git a/dev/SwipeControl/SwipeControl_TestUI/SwipePage.xaml b/dev/SwipeControl/SwipeControl_TestUI/SwipePage.xaml
@@ -18,6 +18,7 @@
         <StackPanel Margin="2">
             <Button x:Name="navigateToSimpleContents" AutomationProperties.Name="navigateToSimpleContents" Margin="2" HorizontalAlignment="Stretch">List Items with simple swipe items.</Button>
             <Button x:Name="navigateToDynamic" AutomationProperties.Name="navigateToDynamic" Margin="2" HorizontalAlignment="Stretch">Exercise Swipe API</Button>
+            <Button x:Name="navigateToClear" AutomationProperties.Name="navigateToClear" Margin="2" HorizontalAlignment="Stretch">Clear items page</Button>
         </StackPanel>
 
         <StackPanel Grid.Column="1" Margin="2">

diff --git a/dev/SwipeControl/SwipeControl_TestUI/SwipePage.xaml.cs b/dev/SwipeControl/SwipeControl_TestUI/SwipePage.xaml.cs
@@ -30,6 +30,7 @@ public SwipePage()
 
             navigateToSimpleContents.Click += delegate { Frame.NavigateWithoutAnimation(typeof(SwipeControlPage), 0); };
             navigateToDynamic.Click += delegate { Frame.NavigateWithoutAnimation(typeof(SwipeControlPage2), 0); };
+            navigateToClear.Click += delegate { Frame.NavigateWithoutAnimation(typeof(SwipeControlClearPage), 0); };
         }
 
         private void CmbSwipeControlOutputDebugStringLevel_SelectionChanged(object sender, SelectionChangedEventArgs e)