Fix issue with single select and unrealized children on TreeView (#2547)

* Add test page * Add test * Fix the issue * Clean up tests * Remove empty line
microsoft · Jun 3, 2020 · eb6d418 · eb6d418
1 parent 3e33555
commit eb6d418
Show file tree

Hide file tree

Showing 8 changed files with 193 additions and 3 deletions.
diff --git a/dev/TreeView/InteractionTests/TreeViewTests.cs b/dev/TreeView/InteractionTests/TreeViewTests.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 Common;
@@ -2868,6 +2868,34 @@ public void SelectedItemBindingsWork()
             }
         }
 
+        [TestMethod]
+        [TestProperty("TestSuite", "B")]
+        public void SingleSelectWithUnrealizedChildrenDoesNotMoveSelection()
+        {
+            using (var setup = new TestSetupHelper(new[] { "TreeView Tests", "TreeViewUnrealizedChildrenTestPage" }))
+            {
+                TapOnTreeViewAt(50, 12, "GetSelectedItemName");
+
+                Log.Comment("Selecting item");
+                ClickButton("GetSelectedItemName");
+                Wait.ForIdle();
+
+                Log.Comment("Verifying current selection");
+                var textBlock = new TextBlock(FindElement.ByName("SelectedItemName"));
+                Verify.AreEqual("Item: 0; layer: 3", textBlock.GetText());
+
+                Log.Comment("Expanding selected item");
+                TapOnTreeViewAt(12, 12, "GetSelectedItemName");
+                Wait.ForIdle();
+
+                Log.Comment("Verifying selection again");
+                ClickButton("GetSelectedItemName");
+                Wait.ForIdle();
+                textBlock = new TextBlock(FindElement.ByName("SelectedItemName"));
+                Verify.AreEqual("Item: 0; layer: 3", textBlock.GetText());
+            }
+        }
+
         private void ClickButton(string buttonName)
         {
             var button = new Button(FindElement.ByName(buttonName));

diff --git a/dev/TreeView/TestUI/TreeViewPage.xaml b/dev/TreeView/TestUI/TreeViewPage.xaml
@@ -71,6 +71,7 @@
                     <Button x:Name="SelectLastRootNode" AutomationProperties.Name="SelectLastRootNode" Content="SelectLastRootNode" HorizontalAlignment="Stretch" Margin="1" Click="SelectLastRootNode_Click"/>
                     <Button x:Name="TreeViewLateDataInitTestPage" AutomationProperties.Name="TreeViewLateDataInitTestPage" Content="TreeViewLateDataInitTestPage" HorizontalAlignment="Stretch" Margin="1" Click="TreeViewLateDataInitTestPage_Click"/>
                     <Button x:Name="TreeViewNodeInMarkupTestPage" AutomationProperties.Name="TreeViewNodeInMarkupTestPage" Content="TreeViewNodeInMarkupTestPage" HorizontalAlignment="Stretch" Margin="1" Click="TreeViewNodeInMarkupTestPage_Click"/>
+                    <Button x:Name="TreeViewUnrealizedChildrenTestPage" AutomationProperties.Name="TreeViewUnrealizedChildrenTestPage" Content="TreeViewUnrealizedChildrenTestPage" HorizontalAlignment="Stretch" Margin="1" Click="TreeViewUnrealizedChildrenTestPage_Click"/>
                     <Button x:Name="ResetItemsSource" AutomationProperties.Name="ResetItemsSource" Content="ResetItemsSource" HorizontalAlignment="Stretch" Margin="1" Click="ResetItemsSource_Click"/>
                     <Button x:Name="ResetItemsSourceAsync" AutomationProperties.Name="ResetItemsSourceAsync" Content="ResetItemsSourceAsync" HorizontalAlignment="Stretch" Margin="1" Click="ResetItemsSourceAsync_Click"/>
                     <Button x:Name="ExpandRootNode" AutomationProperties.Name="ExpandRootNode" Content="ExpandRootNode" HorizontalAlignment="Stretch" Margin="1" Click="ExpandRootNode_Click"/>

diff --git a/dev/TreeView/TestUI/TreeViewPage.xaml.cs b/dev/TreeView/TestUI/TreeViewPage.xaml.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 System;
@@ -978,6 +978,11 @@ private void TreeViewNodeInMarkupTestPage_Click(object sender, RoutedEventArgs e
             Frame.NavigateWithoutAnimation(typeof(TreeViewNodeInMarkupTestPage));
         }
 
+        private void TreeViewUnrealizedChildrenTestPage_Click(object sender, RoutedEventArgs e)
+        {
+            Frame.NavigateWithoutAnimation(typeof(TreeViewUnrealizedChildrenTestPage));
+        }
+
         private void ClearException_Click(object sender, RoutedEventArgs e)
         {
             ExceptionMessage.Text = string.Empty;

diff --git a/dev/TreeView/TestUI/TreeViewUnrealizedChildrenTestPage.xaml b/dev/TreeView/TestUI/TreeViewUnrealizedChildrenTestPage.xaml
@@ -0,0 +1,23 @@
+<local:TestPage
+    x:Class="MUXControlsTestApp.TreeViewUnrealizedChildrenTestPage"
+    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"
+    xmlns:muxc="using:Microsoft.UI.Xaml.Controls"
+    Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
+
+    <StackPanel>
+        <TextBlock Text="Unrealized children selection problem"/>
+        <TextBlock x:Name="SelectedItemName" AutomationProperties.Name="SelectedItemName"/>
+        <Button x:Name="GetSelectedItemName" AutomationProperties.Name="GetSelectedItemName" Content="Get selected item" Click="GetSelectedItemName_Click"/>
+        <muxc:TreeView
+            x:Name="UnrealizedTreeViewSelection"
+            Expanding="UnrealizedTreeViewSelection_Expanding"
+            Collapsed="UnrealizedTreeViewSelection_Collapsed">
+
+        </muxc:TreeView>
+    </StackPanel>
+</local:TestPage>
diff --git a/dev/TreeView/TestUI/TreeViewUnrealizedChildrenTestPage.xaml.cs b/dev/TreeView/TestUI/TreeViewUnrealizedChildrenTestPage.xaml.cs
@@ -0,0 +1,52 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Runtime.InteropServices.WindowsRuntime;
+using Windows.Foundation;
+using Windows.Foundation.Collections;
+using Windows.UI.Xaml;
+using Microsoft.UI.Xaml.Controls;
+using Windows.UI.Xaml.Controls.Primitives;
+using Windows.UI.Xaml.Data;
+using Windows.UI.Xaml.Input;
+using Windows.UI.Xaml.Media;
+using Windows.UI.Xaml.Navigation;
+
+// The Blank Page item template is documented at https://go.microsoft.com/fwlink/?LinkId=234238
+
+namespace MUXControlsTestApp
+{
+    /// <summary>
+    /// An empty page that can be used on its own or navigated to within a Frame.
+    /// </summary>
+    public sealed partial class TreeViewUnrealizedChildrenTestPage : TestPage
+    {
+
+        public TreeViewNode VirtualizingTestRootNode;
+        public CustomContent CustomContentRootNode;
+
+        public TreeViewUnrealizedChildrenTestPage()
+        {
+
+            this.InitializeComponent();
+            CustomContentRootNode = new CustomContent(3);
+            VirtualizingTestRootNode = CustomContentRootNode.GetTreeViewNode();
+            UnrealizedTreeViewSelection.RootNodes.Add(VirtualizingTestRootNode);
+        }
+
+        private void GetSelectedItemName_Click(object sender, RoutedEventArgs e)
+        {
+            SelectedItemName.Text = ((UnrealizedTreeViewSelection.SelectedItem as TreeViewNode).Content as CustomContent).ToString();
+        }
+
+        private void UnrealizedTreeViewSelection_Expanding(Microsoft.UI.Xaml.Controls.TreeView sender, Microsoft.UI.Xaml.Controls.TreeViewExpandingEventArgs args)
+        {
+            VirtualizingDataSource.FillTreeNode(args.Node);
+        }
+        private void UnrealizedTreeViewSelection_Collapsed(Microsoft.UI.Xaml.Controls.TreeView sender, Microsoft.UI.Xaml.Controls.TreeViewCollapsedEventArgs args)
+        {
+            VirtualizingDataSource.EmptyTreeNode(args.Node);
+        }
+    }
+}
diff --git a/dev/TreeView/TestUI/TreeView_TestUI.projitems b/dev/TreeView/TestUI/TreeView_TestUI.projitems
@@ -24,6 +24,10 @@
       <DependentUpon>TreeViewPage.xaml</DependentUpon>
     </Compile>
     <Compile Include="$(MSBuildThisFileDirectory)TreeViewItemSource.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)TreeViewUnrealizedChildrenTestPage.xaml.cs">
+      <DependentUpon>TreeViewUnrealizedChildrenTestPage.xaml</DependentUpon>
+    </Compile>
+    <Compile Include="$(MSBuildThisFileDirectory)VirtualizingDataSource.cs" />
   </ItemGroup>
   <ItemGroup>
     <Page Include="$(MSBuildThisFileDirectory)TreeViewItemTemplateSelectorTestPage.xaml">
@@ -38,6 +42,10 @@
       <SubType>Designer</SubType>
       <Generator>MSBuild:Compile</Generator>
     </Page>
+    <Page Include="$(MSBuildThisFileDirectory)TreeViewUnrealizedChildrenTestPage.xaml">
+      <SubType>Designer</SubType>
+      <Generator>MSBuild:Compile</Generator>
+    </Page>
     <Page Include="$(MSBuildThisFileDirectory)\TreeViewPage.xaml">
       <SubType>Designer</SubType>
       <Generator>MSBuild:Compile</Generator>

diff --git a/dev/TreeView/TestUI/VirtualizingDataSource.cs b/dev/TreeView/TestUI/VirtualizingDataSource.cs
@@ -0,0 +1,68 @@
+using Microsoft.UI.Xaml.Controls;
+using System;
+using System.Collections.Generic;
+using System.Text;
+using Windows.ApplicationModel.Chat;
+
+namespace MUXControlsTestApp
+{
+    public class CustomContent
+    {
+        public List<CustomContent> Children = new List<CustomContent>();
+
+        private int nestingLevel;
+        private int index;
+
+        public CustomContent(int nestingLevel = 3,int index=0)
+        {
+            this.nestingLevel = nestingLevel;
+            this.index = index;
+            if(nestingLevel <= 0)
+            {
+                return;
+            }
+            for (int i = 0; i < 4; i++)
+            {
+                Children.Add(new CustomContent(nestingLevel - 1,i));
+            }
+        }
+
+        public override string ToString()
+        {
+            return $"Item: {this.index}; layer: {this.nestingLevel}";
+        }
+
+        public TreeViewNode GetTreeViewNode()
+        {
+            var node = new TreeViewNode();
+            node.Content = this;
+            node.HasUnrealizedChildren = this.Children.Count != 0;
+            return node;
+        }
+    }
+
+    public class VirtualizingDataSource
+    {
+        public static void FillTreeNode(TreeViewNode node)
+        {
+            var customContent = (CustomContent)node.Content;
+            if(customContent != null)
+            {
+                if(node.HasUnrealizedChildren)
+                {
+                    foreach(var child in customContent.Children)
+                    {
+                        node.Children.Add(child.GetTreeViewNode());
+                    }
+                }
+                node.HasUnrealizedChildren = false;
+            }
+        }
+
+        public static void EmptyTreeNode(TreeViewNode node)
+        {
+            node.Children.Clear();
+            node.HasUnrealizedChildren = true;
+        }
+    }
+}
diff --git a/dev/TreeView/ViewModel.cpp b/dev/TreeView/ViewModel.cpp
@@ -1040,7 +1040,12 @@ void ViewModel::SelectedNodeChildrenChanged(winrt::TreeViewNode const& sender, w
     case (winrt::CollectionChange::ItemInserted):
     {
         auto newNode = changingChildrenNode.Children().GetAt(index);
-        UpdateNodeSelection(newNode, NodeSelectionState(changingChildrenNode));
+
+        // If we are in multi select, we want the new child items to be also selected.
+        if (!IsInSingleSelectionMode())
+        {
+            UpdateNodeSelection(newNode, NodeSelectionState(changingChildrenNode));
+        }
         break;
     }