Microsoft-ui-xaml: Changing ItemsRepeater ItemsSource keeps part of the old source data bound (via DataContext).

[MarchingCube]

Describe the bug

Changing ItemsSource on ItemsRepeater to a collection with length smaller than the current one will lead to several items from the item template sticking around (being hidden via -10000 layout offset).

Those items will be still data bound via DataContext and can cause side effects when view model state changes (for instance: focus TextBox when a VM property changes, due to duplicates focus might be taken by offscreen control). This effectively leads to having duplicate items in the ItemsRepeater having the same DataContext.

Steps to reproduce the bug

Steps to reproduce the behavior:

1. Create blank Universal Windows app and add reference to 2.4.0-prerelease.200113001 WinUI nuget package.
2. Simple XAML and code behind setup:

XAML:

<Page
    x:Class="ItemsRepeaterRepro.MainPage"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    xmlns:controls="using:Microsoft.UI.Xaml.Controls"
    mc:Ignorable="d"
    Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">

    <Grid>
        <Grid.RowDefinitions>
            <RowDefinition Height="Auto"/>
            <RowDefinition Height="*"/>
        </Grid.RowDefinitions>
        <Button Content="Change items" Grid.Row="0" Click="ChangeItems"/>
        <controls:ItemsRepeater Grid.Row="1" x:Name="Repeater" >
            <controls:ItemsRepeater.ItemTemplate>
                <DataTemplate>
                    <TextBlock Text="{Binding Text}"/>
                </DataTemplate>
            </controls:ItemsRepeater.ItemTemplate>
        </controls:ItemsRepeater>
    </Grid>
</Page>

C#:

using System.Collections.Generic;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Controls;

namespace ItemsRepeaterRepro
{
    public class TestVm
    {
        public string Text { get; set; }
    }

    public class TestVm2
    {
        public string Text { get; set; }
    }

    public sealed partial class MainPage : Page
    {
        public MainPage()
        {
            InitializeComponent();

            Repeater.ItemsSource = CreateSource(10, false);
        }

        private List<object> CreateSource(int count, bool otherType)
        {
            List<object> source = new List<object>();

            for (int i = 0; i < count; i++)
            {
                if (otherType)
                {
                    source.Add(new TestVm2
                    {
                        Text = i.ToString()
                    });
                }
                else
                {
                    source.Add(new TestVm
                    {
                        Text = i.ToString()
                    });
                }
            }

            return source;
        }

        private void ChangeItems(object sender, RoutedEventArgs e)
        {
            Repeater.ItemsSource = CreateSource(5, true);
        }
    }
}

1. Clicking on Change items button will cause 5 items to be removed, but we can see that in the visual tree we still have 5 items bound to the previous collection. For clarity I am using different object type (5 items of type TestVm and 5 items of TestVm2), this also works with just having one VM class.

Expected behavior

I would expect to not have duplicate data bound elements, especially coming from collections that are no longer used as items source for ItemsRepeater.

Screenshots

Version Info

NuGet package version: 2.4.0-prerelease.200113001

| Windows 10 version | Saw the problem? |
| :--------------------------------- | :-------------------- |
| Insider Build (xxxxx) | |
| November 2019 Update (18363) | Yes |
| May 2019 Update (18362) | Yes |
| October 2018 Update (17763) | |
| April 2018 Update (17134) | |
| Fall Creators Update (16299) | |
| Creators Update (15063) | |

| Device form factor | Saw the problem? |
| :-------------------- | :------------------- |
| Desktop | Yes |
| Mobile | |
| Xbox | |
| Surface Hub | |
| IoT | |

Additional context

[ranjeshj]

@MarchingCube good catch. We do some tricks in ItemsRepeater to improve performance. For example, we do not remove the container immediately (because when scrolling it is likely to get recycled and get reused again). There is some cost associated with removing and adding it back to the visual tree. Hence the large negative offset you see. We just leave it under the repeater and give it back to recycle. If that container comes back for a different index, we can avoid the extra work. However, this looks like a bug to leave the data context still bound.

As a workaround, you could try clearing the datacontext in OnElementClearing event. Thank you for reporting. I've added it to our backlog.

[chingucoding]

@ranjeshj What do you think is the appropriate fix here? Should the elementfactory clear the datacontext of an element when recycling?

[ranjeshj]

Yes. If ItemsRepeater is setting the datacontext, it should clear the datacontext when recycling. We have to be careful to not stomp though (there might be a case where the provided element might already have datacontext set on it)

[chingucoding]

I just tried the sample code you shared in the MUXControlsTestApp and it seems that the DataContext does get cleared correctly already. After changing the datasource, 5 items have a DataContext while the other 5 items have their DataContext set to null. Looks like that this was fixed with #2626 .

[chingucoding]

Also, the changes made in #2626 only affect items that have their datacontext set by the ElementFactory. Items whose DataContext was not set by the repeater are unaffected.

[ranjeshj]

Thanks for verifying @chingucoding. Closing this issue.