Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion dev/TreeView/InteractionTests/TreeViewTests.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -2843,6 +2843,34 @@ public void ItemsSourceSwitchForthAndBackTest()
}
}

[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));
Expand Down
1 change: 1 addition & 0 deletions dev/TreeView/TestUI/TreeViewPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>
Expand Down
7 changes: 6 additions & 1 deletion dev/TreeView/TestUI/TreeViewPage.xaml.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
Expand Down
23 changes: 23 additions & 0 deletions dev/TreeView/TestUI/TreeViewUnrealizedChildrenTestPage.xaml
Original file line number Diff line number Diff line change
@@ -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>
52 changes: 52 additions & 0 deletions dev/TreeView/TestUI/TreeViewUnrealizedChildrenTestPage.xaml.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
8 changes: 8 additions & 0 deletions dev/TreeView/TestUI/TreeView_TestUI.projitems
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand All @@ -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>
Expand Down
68 changes: 68 additions & 0 deletions dev/TreeView/TestUI/VirtualizingDataSource.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
7 changes: 6 additions & 1 deletion dev/TreeView/ViewModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// If we are in multi select, we want the new child items to be also selected. [](start = 8, length = 78)

I assume that the desired behavior is not dependent on the 'HasUnrealizedChildren' flag. That is, when all the children are realized and we expand a node we would behave the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chingucoding do you know if this is the case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know this method would handle both cases regardless of the HasUnrealizedChildren flag. This listener just acts when the children of the selected node have changed. However in the case of HasUnrealizedChildren, those children got added to the selected item, raising this listener. Since in SingleSelect, selecting an item unselects the previous item, this resulted in the faulty behavior.

if (!IsInSingleSelectionMode())
{
UpdateNodeSelection(newNode, NodeSelectionState(changingChildrenNode));
}
break;
}

Expand Down