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

TreeView doesn't preserve the value of IsExpanded when containers in "content mode" are recycled and reused #9549

Closed
ramezgerges opened this issue Apr 15, 2024 · 2 comments
Labels
area-TreeView bug Something isn't working closed-Fixed Described behavior has been fixed. fix-released The fix has been in a release (experimental, preview, stable, or servicing). team-Controls Issue for the Controls team
Milestone

Comments

@ramezgerges
Copy link

ramezgerges commented Apr 15, 2024

Describe the bug

When you expand and collapse nodes in a TreeView with an ItemsSource set (i.e. in "content mode"), the value of TreeViewItem.IsExpanded is not preserved. If you scroll through a large TreeView such that an expanded node is recycled and then you scroll back up, you'll find the node most likely collapsed. Likewise, if you scroll down, you'll find a random node expanded on its own.

void TreeViewList::PrepareContainerForItemOverride(winrt::DependencyObject const& element, winrt::IInspectable const& item)
{
auto itemNode = NodeFromContainer(element);
TreeViewNode* itemNodeImplNoRef = winrt::get_self<TreeViewNode>(itemNode);
winrt::TreeViewItem itemContainer = element.as<winrt::TreeViewItem>();
const auto selectionState = itemNodeImplNoRef->SelectionState();
//Set the expanded property to match that of the Node, and enable Drop by default
itemContainer.AllowDrop(true);
if (IsContentMode())
{
const bool hasChildren = itemContainer.HasUnrealizedChildren() || itemNode.HasChildren();
itemContainer.GlyphOpacity(hasChildren ? 1.0 : 0.0);
if (itemContainer.IsExpanded() != itemNode.IsExpanded()) {
DispatcherQueue().TryEnqueue(winrt::DispatcherQueueHandler(
[itemNode, itemContainer]()
{
itemNode.IsExpanded(itemContainer.IsExpanded());
}));
}
}

This is due to how TreeViewList.PrepareContainerForItemOverride is written, where it considers the container the "source of truth" for IsExpanded, instead of the TreeViewNode that the container is wrapping. This makes sense only when the ItemTemplate of the TreeView binds the container's IsExpanded to some property in the view model. Otherwise, the TreeViewNode (which still remembers the IsExpanded state the last time it was in view) should be the "source of truth".

Instead, it could be something like

DispatcherQueue().TryEnqueue(winrt::DispatcherQueueHandler( 
     [itemNode, itemContainer]() 
 { 
     if (itemContainer.GetBindingExpression(MetadataAPI::GetDependencyPropertyByIndex(KnownPropertyIndex::TreeViewItem_IsExpanded))
     {
        itemNode.IsExpanded(itemContainer.IsExpanded()); 
     }
     else
     {
        itemContainer.IsExpanded(itemNode.IsExpanded()); 
     }
 })); 

Steps to reproduce the bug

  <StackPanel>
    <Button Click="Button_Click_1">Expand the second root node</Button>
    <Button Click="Button_Click_2">Scroll to the end, you'll find root node 16 expanded for no reason</Button>
    <Button Click="Button_Click_3">Scroll back up, you'll find that the second root node that was expanded is now collapsed</Button>
    <TextBlock>If you scroll around after that, you might find random root nodes expanded and others that were expanded now collapsed.</TextBlock>
    <TextBlock>The first root node will always behave well because it's never recycled, so when investigating, play with any node except the very first one.</TextBlock>
    <TreeView x:Name="MyTreeView" ItemsSource="{x:Bind DataSource}" Height="100">
      <TreeView.ItemTemplate>
        <DataTemplate>
          <TreeViewItem ItemsSource="{Binding Children}" Content="{Binding Name}"/>
        </DataTemplate>
      </TreeView.ItemTemplate>
    </TreeView>
  </StackPanel>
public sealed partial class MainPage : Page
{
    public class Item
    {
        public string Name { get; set; }
        public List<Item> Children { get; set; }
    }

    public List<Item> DataSource { get; set; }

    public MainPage()
    {
        this.InitializeComponent();
        DataSource = Enumerable.Range(0, 20).Select(i => 
            new Item
            {
                Name = $"Root{i}",
                Children = new List<Item>
                {
                    new Item { Name = $"Root{i} Child 1", Children = new List<Item>
                        {
                            new Item { Name = $"Root{i} Grandchild 1" },
                            new Item { Name = $"Root{i} Grandchild 2" }
                        }
                    },
                    new Item { Name = $"Root{i} Child 2" }
                }
            }
        ).ToList();
    }

    private void Button_Click_1(object sender, RoutedEventArgs e)
    {
        MyTreeView.RootNodes[1].IsExpanded = true;
    }

    private void Button_Click_2(object sender, RoutedEventArgs e)
    {
        var sv = EnumerateDescendants(MyTreeView).OfType<ScrollViewer>().First();
        sv.ScrollToVerticalOffset(9999);
    }

    private void Button_Click_3(object sender, RoutedEventArgs e)
    {
        var sv = EnumerateDescendants(MyTreeView).OfType<ScrollViewer>().First();
        sv.ScrollToVerticalOffset(0);
    }

    private static IEnumerable<UIElement> EnumerateDescendants(UIElement? reference)
    {
        var children = Enumerable
            .Range(0, VisualTreeHelper.GetChildrenCount(reference))
            .Select(x => VisualTreeHelper.GetChild(reference, x))
            .OfType<UIElement>();
        foreach (var child in children)
        {
            yield return child;
            foreach (var grandchild in EnumerateDescendants(child))
            {
                yield return grandchild;
            }
        }
    }
}

Click on the buttons from top to bottom.

Expected behavior

Expanded nodes should stay expanded, collapsed nodes should stay collapsed.

Screenshots

No response

NuGet package version

WinUI 3 - Windows App SDK 1.5.2: 1.5.240404000

Windows version

Windows 10 (21H2): Build 19044

Additional context

@ramezgerges ramezgerges added the bug Something isn't working label Apr 15, 2024
Copy link

Hi I'm an AI powered bot that finds similar issues based off the issue title.

Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one. Thank you!

Open similar issues:

Closed similar issues:

Note: You can give me feedback by thumbs upping or thumbs downing this comment.

@Youssef1313
Copy link
Contributor

Related PR: #1924

@bpulliam bpulliam added area-TreeView team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Apr 18, 2024
@llongley llongley added this to the WinAppSDK 1.6 milestone Jul 23, 2024
@llongley llongley added the closed-Fixed Described behavior has been fixed. label Jul 23, 2024
@codendone codendone added the fix-released The fix has been in a release (experimental, preview, stable, or servicing). label Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-TreeView bug Something isn't working closed-Fixed Described behavior has been fixed. fix-released The fix has been in a release (experimental, preview, stable, or servicing). team-Controls Issue for the Controls team
Projects
None yet
Development

No branches or pull requests

5 participants