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

When the TreeListView expands too much content, scrolling to the top will shrink the content #3640

Open
KashimuraBaka opened this issue Jul 31, 2024 · 2 comments
Labels
bug evaluation required Items is pending review or evaluation by the team

Comments

@KashimuraBaka
Copy link

Bug explanation

When using the TreeViewList control, if too much data is expanded, scrolling to the top will automatically collapse all content, and it cannot maintain the expanded state.
I thought it was my own problem, the same issue occurred during the demo attempt.
1

Version

5.1.0

@KashimuraBaka KashimuraBaka added bug evaluation required Items is pending review or evaluation by the team labels Jul 31, 2024
@corvinsz
Copy link
Contributor

corvinsz commented Aug 7, 2024

This bug can be reproduced a little bit easier by following these steps:

  1. Add alot of items to the TreeListView
  2. Expand any item (preferably one at the top)
  3. Scroll far away from the expanded item
  4. Scroll back to the previous expanded item

MultiTreeViewBug

My theory

My theory is that the PrepareTreeListViewItem method is getting called twice, and the actual setting of the IsExpanded property is done asynchronusly via a Dispatcher. The first method call sets the IsExpanded to True, which is correct. The second method call sets it to False because the Dispatcher hasn't finished executing yet.
When uncommenting the Dispatcher it works like expected, however I am not sure if that causes other side effects.

internal void PrepareTreeListViewItem(object? item, TreeListView treeListView, int level, bool isExpanded)
{
    Level = level;
    TreeListView = treeListView;

    //NB: This can occur as part of TreeListView.PrepareContainerForItemOverride
    //Because this can trigger additional collection changes we enqueue the operation
    //to occur after the current operation has completed.
    //Dispatcher.BeginInvoke(() =>
    //{
    if (GetTemplate() is HierarchicalDataTemplate { ItemsSource: { } itemsSourceBinding })
    {
        SetBinding(ChildrenProperty, itemsSourceBinding);
    }
    IsExpanded = isExpanded;
    //});

    DataTemplate? GetTemplate()
    {
        return ContentTemplate ??
               ContentTemplateSelector?.SelectTemplate(item, this) ??
               ContentPresenter?.Template;
    }
}

MultiTreeViewBug_Fixed

@Keboo
Copy link
Member

Keboo commented Aug 7, 2024

This is "by design" but there is a compelling argument that the design may be flawed (or at least confusing).

First a little background. A big reason for creating this control was to have a tree view that supports UI virtualization (the default WPF tree view does not). By default, this tree view has UI virtualization enabled specifically for that reason. As you scroll the UI elements are being recycled to keep it nice and responsive. The problem comes because the IsExpanded property on the TreeListViewItem is not bound to anything in the item's view model, so once the UI element is recycled its previous state is lost.

The dispatcher change above is interesting however, it does introduce so issue with data binding from multiple threads specifically if the "children" are an observable collection that is being updated dynamically when the parent is expanded.

There are a couple solutions here:
First, if virtualization is not desirable, then just switch back to the regular WPF TreeView. This one adds a lot of complexity to get the virtualization so only use it if you need it.

Second, bind the IsExpanded state to something in your item's view model. This allows your view model to maintain the state of the item, even when its UI container is recycled.
Assuming the property on your view model is named IsExpanded then the XAML for this would look like:

<materialDesign:TreeListView >
  ...
  <materialDesign:TreeListView.ItemContainerStyle>
    <Style TargetType="materialDesign:TreeListViewItem" BasedOn="{StaticResource {x:Type materialDesign:TreeListViewItem}}">
      <Setter Property="IsExpanded" Value="{Binding IsExpanded}" />
    </Style>
  </materialDesign:TreeListView.ItemContainerStyle>
 ...
</materialDesign:TreeListView>

I would also be really interested in hearing thoughts on how this could be better documented or made more obvious for people.
I am going to check in a sample of this into the repo as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug evaluation required Items is pending review or evaluation by the team
Projects
None yet
Development

No branches or pull requests

3 participants