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

ItemTemplateSelector is broken in all Collection Views (ListView, GridView, TreeView...) that has their default containers specified in the DataTemplate #9189

Closed
ReneeGA2020 opened this issue Dec 27, 2023 · 3 comments
Labels
area-Lists ListView, GridView, ListBox, etc bug Something isn't working team-Controls Issue for the Controls team

Comments

@ReneeGA2020
Copy link

ReneeGA2020 commented Dec 27, 2023

Describe the bug

ItemTemplateSelector is in a broken state for collection view controls (ListView, GridView, TreeView, etc) in WinUI3.

When the user scrolls through the views and triggers recycling of containers, the control will constantly choose an incompatible container for the new items that are brought into view. Which will often lead to broken views, binding failing and/or crashing of applications.

It is surprising that Microsoft have this glancing bug to live for so long since WinUI's release.

Workarounds and analysis

There is a Workarounds that can help to you avoid such a crashing: Don't use the default containers in the DataTemplate. 'Default Containers' here means ListViewItem for ListView, GridViewItem for GridViewView.

For Example, don't do

<DataTemplate x:Key="ListViewItemTemplate1" x:DataType="viewmodel:TestVM1">
    <ListViewItem>
        <TextBlock Text="{x:Bind Content1}" />
    </ListViewItem>
</DataTemplate>

Instead, do

<DataTemplate x:Key="ListViewItemTemplate1" x:DataType="viewmodel:TestVM1">
    <TextBlock Text="{x:Bind Content1}" />
</DataTemplate>

My analysis of how this workarounds works: If you directly provide a container corresponding to the view as the root frame of the DataTemplate, the ListViewBase will directly use it as the container of the new items. But if there is no valid container in the DataTemplate, the ListViewBase will create a new container and put your templated stuff into its Content field. In this case, since the Containers will always be the same, the view controls just replaced its contents, so that the container will always be compatible with each other.

Unfortunately, this workarounds cannot work for TreeViews. Because without a TreeViewItem in the DataTemplate, you basically have no way to specify ItemsSource for your nodes. This rendered the TreeViews unusable, as it is basically became a simple ListView.

I had to write my own TreeView control from scratch because of this bug... I really hope Microsoft can have this bug fixed.

Steps to reproduce the bug

WinUIItemTemplateSelectorBug.zip

Attached is a test app for this bug. I added a ListView, a GridView and a TreeView to demonstrate this bug. They use a very simple selector based on some very simple data.

  1. Build and Run this App
  2. You are expected to see a ListView, a GridView, a TreeView in the window, they are displaying the same bunch of data with the help of DataTemplate
  3. Quickly scrolls through any of the views using the scroll bar to trigger container recycling.
  4. ->Application Crash. Message = "Source object type is not a projected type and does not inherit from a projected type. (Parameter 'value')"

Expected behavior

No crash, no broken view.

Screenshots

image

NuGet package version

WinUI 3 - Windows App SDK 1.4.3: 1.4.231115000

Windows version

Windows Insider Build (xxxxx), Windows 11 (22H2): Build 22621, Windows 11 (21H2): Build 22000, Windows 10 (21H2): Build 19044, Windows 10 (21H1): Build 19043

Additional context

This bug has been there since the first release of WinUI3.

For any one that seek to make your templated TreeView to work without having to write your own treeview control from scratch - there are an extra workarounds things you can try, but it is not granted that it suit your case:

  1. Create a new TreeView class based on the official TreeViewEx control (Let's call it TreeViewEx)
  2. Create a new TreeViewList class based on the official TreeViewList. (Let's call it TreeViewExList)
  3. Create your own Styles for TreeViewEx, using TreeViewExList to replace the old TreeViewList in the Control Template.
  4. Add a override for IsItemItsOwnContainerOverride() function, and make it always return false. This is so that the control sees nothing as their default containers, and a new TreeViewItem will always be created by the control to contain your templated stuff. This is the key step to avoid the crash.
  5. Create a TreeViewExItem class based on ContentControl. This step is important, that you are not creating a replacement for the TreeViewItem, you are creating a fake hook that is used to provide necessary information for the real TreeViewItem to work.
  6. Create Dependency Properties for the fake TreeViewExItem: ItemsSource, IsExpanded, GlyphBrush, CollapsedGlyph, ExpandedGlyph, GlyphOpacity, GlyphSize, HasUnrealizedChildren, with the same property type of the real TreeViewItem. (ItemsSource is the only one that is nessary to make your TreeView work, and perhaps IsExpanded)
  7. Override OnApplyTemplate() function in the TreeViewExItem, find through visual tree a TreeViewItem in its ascendants
  8. Hook the changed events of all the Dependency Properties in your TreeViewExItem, and feed the corresponding values to for the real TreeViewItem. Please note that when ItemsSource changes, you may also want to update the value of IsExpaneded.
  9. Change your XAML to use the new TreeViewEx and TreeViewExItem and see if it works for you.
@ReneeGA2020 ReneeGA2020 added the bug Something isn't working label Dec 27, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Dec 27, 2023
@DHancock
Copy link

An alternative work around is to comment out the following override in the TestTemplateSelector class:

    protected override DataTemplate SelectTemplateCore(object item)
    {
        return GetTemplate(item) ?? base.SelectTemplateCore(item);
    }

The remarks section of SelectTemplateCore suggests that only the second override should be provided when using virtualising panels

@ReneeGA2020
Copy link
Author

ReneeGA2020 commented Dec 28, 2023

An alternative work around is to comment out the following override in the TestTemplateSelector class:

    protected override DataTemplate SelectTemplateCore(object item)
    {
        return GetTemplate(item) ?? base.SelectTemplateCore(item);
    }

The remarks section of SelectTemplateCore suggests that only the second override should be provided when using virtualising panels

That's very good information. I haven't noticed that part yet.

But still, you get the result of a broken view, as it is the same as putting your Template into an internally created Container. Which leads to your gridview and listview to have doubled layer (if you already put a Container in the DataTemplate), and treeview still have no way to set a ItemsSource.

Which means we will still have to do similar things to set the ItemsSource value for the hidden real TreeViewItem.

@bpulliam bpulliam added area-Lists ListView, GridView, ListBox, etc team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Jan 22, 2024
@RBrid
Copy link
Contributor

RBrid commented Feb 7, 2024

Hello. Thanks for your report. This turns out to be by design.

In a nutshell, taking a ListView as an example, when the application uses a ListView.ItemTemplateSelector instead of a ListView.ItemTemplate, and that selector returns DataTemplate instances with a ListViewItem as its root element, that application must listen and handle the ListView.ChoosingItemContainer and ListView.ContainerContentChanging events.

The ContainerContentChanging handler is in charge of building lists of recyclable containers (for each kind of item template returned by the ItemTemplateSelector).

The ChoosingItemContainer is in charge of returning an appropriate container based on the provided item, if and only if the suggested container is inapplicable to the item. The returned container may be picked from those recyclable container lists or not.

Adding such handlers to your repro application fixes the crashes.

Which changes are required in the repro app, just for the ListView case?

  • in MainWindow.xaml:
    name the Grid: <Grid x:Name="grid" Height="500" ...

    add the two handlers:

    <ListView
      Header="ListView"
      ItemTemplateSelector="{StaticResource ListViewSelector}"
      ItemsSource="{x:Bind CollectionViewModel}"
      ChoosingItemContainer="ListView_ChoosingItemContainer"
      ContainerContentChanging="ListView_ContainerContentChanging"/>
  • in MainWindow.xaml.cs's MainWindow class, add:
    private List<SelectorItem> recyclableVM1Containers = new List<SelectorItem>();
    private List<SelectorItem> recyclableVM2Containers = new List<SelectorItem>();

    private void ListView_ChoosingItemContainer(object sender, ChoosingItemContainerEventArgs e)
    {
        if (e.ItemContainer != null)
        {
            bool itemIsVM1 = e.Item is TestVM1;

            if (itemIsVM1 && e.ItemContainer.ContentTemplateRoot is TextBox)
            {
                if (recyclableVM1Containers.Count > 0)
                {
                    e.ItemContainer = recyclableVM1Containers[0];
                    recyclableVM1Containers.RemoveAt(0);
                }
                else
                {
                    DataTemplate dataTemplate = grid.Resources["ListViewItemTemplate1"] as DataTemplate;
                    e.ItemContainer = dataTemplate.LoadContent() as SelectorItem;
                }
            }
            else if (!itemIsVM1 && e.ItemContainer.ContentTemplateRoot is TextBlock)
            {
                if (recyclableVM2Containers.Count > 0)
                {
                    e.ItemContainer = recyclableVM2Containers[0];
                    recyclableVM2Containers.RemoveAt(0);
                }
                else
                {
                    DataTemplate dataTemplate = grid.Resources["ListViewItemTemplate2"] as DataTemplate;
                    e.ItemContainer = dataTemplate.LoadContent() as SelectorItem;
                }
            }
            else
            {
                // Suggested container is usable for e.Item. Remove it from the recyclable list if present.
                if (itemIsVM1 && recyclableVM1Containers.Contains(e.ItemContainer))
                {
                    recyclableVM1Containers.Remove(e.ItemContainer);
                }
                else if (!itemIsVM1 && recyclableVM2Containers.Contains(e.ItemContainer))
                {
                    recyclableVM2Containers.Remove(e.ItemContainer);
                }
            }
        }
    }

    private void ListView_ContainerContentChanging(ListViewBase sender, ContainerContentChangingEventArgs e)
    {
        if (e.InRecycleQueue && e.ItemContainer != null)
        {
            if (e.Item is TestVM1)
                recyclableVM1Containers.Add(e.ItemContainer);
            else
                recyclableVM2Containers.Add(e.ItemContainer);
            e.Handled = true;
        }
    }

We have a couple of potential ideas for improving the developer experience though:

  • provide a more guiding error message when those ListViewBase events are not hooked up and such a binding fails.
  • add the ability to set e.ItemContainer to null in the ChoosingItemContainer handler to tell the Xaml framework that it needs to create a brand-new container.
    These potential improvements are being considered for a future Xaml release.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Lists ListView, GridView, ListBox, etc bug Something isn't working team-Controls Issue for the Controls team
Projects
None yet
Development

No branches or pull requests

5 participants