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

Ensure disconnected ItemsViewHandler doesn't hold onto the items source #24699

Merged
merged 2 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public partial class CarouselViewHandler : ItemsViewHandler<CarouselView>
protected override void ConnectHandler(ListViewBase platformView)
{
ItemsView.Scrolled += CarouselScrolled;
ListViewBase.SizeChanged += OnListViewSizeChanged;
platformView.SizeChanged += OnListViewSizeChanged;

UpdateScrollBarVisibilityForLoop();

Expand All @@ -50,9 +50,9 @@ protected override void DisconnectHandler(ListViewBase platformView)
if (ItemsView != null)
ItemsView.Scrolled -= CarouselScrolled;

if (ListViewBase != null)
if (platformView != null)
{
ListViewBase.SizeChanged -= OnListViewSizeChanged;
platformView.SizeChanged -= OnListViewSizeChanged;
_proxy.Unsubscribe();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ protected override void ConnectHandler(ListViewBase platformView)
protected override void DisconnectHandler(ListViewBase platformView)
{
VirtualView.ScrollToRequested -= ScrollToRequested;
CleanUpCollectionViewSource(platformView);
base.DisconnectHandler(platformView);
}

Expand Down Expand Up @@ -154,6 +155,11 @@ void OnItemsVectorChanged(global::Windows.Foundation.Collections.IObservableVect
protected abstract ListViewBase SelectListViewBase();

protected virtual void CleanUpCollectionViewSource()
{
CleanUpCollectionViewSource(ListViewBase);
}

private void CleanUpCollectionViewSource(ListViewBase platformView)
{
if (CollectionViewSource is not null)
{
Expand All @@ -174,7 +180,7 @@ protected virtual void CleanUpCollectionViewSource()
// Remove all children inside the ItemsSource
if (VirtualView is not null)
{
foreach (var item in ListViewBase.GetChildren<ItemContentControl>())
foreach (var item in platformView.GetChildren<ItemContentControl>())
{
var element = item.GetVisualElement();
VirtualView.RemoveLogicalChild(element);
Expand All @@ -183,7 +189,7 @@ protected virtual void CleanUpCollectionViewSource()

if (VirtualView?.ItemsSource is null)
{
ListViewBase.ItemsSource = null;
platformView.ItemsSource = null;
return;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public abstract partial class ItemsViewHandler<TItemsView> : ViewHandler<TItemsV
protected override void DisconnectHandler(UIView platformView)
{
ItemsView.ScrollToRequested -= ScrollToRequested;
Controller?.DisposeItemsSource();
base.DisconnectHandler(platformView);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,15 @@ public virtual void UpdateItemsSource()
(ItemsView as IView)?.InvalidateMeasure();
}

internal void DisposeItemsSource()
{
_measurementCells?.Clear();
ItemsViewLayout?.ClearCellSizeCache();
ItemsSource?.Dispose();
ItemsSource = new EmptySource();
CollectionView.ReloadData();
}

public virtual void UpdateFlowDirection()
{
CollectionView.UpdateFlowDirection(ItemsView);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.ObjectModel;
using System.Collections.Specialized;
using System.Threading.Tasks;
using Microsoft.Maui.Controls;
using Microsoft.Maui.Controls.Handlers.Items;
Expand Down Expand Up @@ -112,6 +113,45 @@ await CreateHandlerAndAddToWindow<LayoutHandler>(layout, async (handler) =>
Assert.NotNull(handler.PlatformView);
});
}

#if !ANDROID //https://github.com/dotnet/maui/pull/24610
[Fact]
public async void DisconnectedCarouselViewDoesNotHookCollectionViewChanged()
{
SetupBuilder();

CollectionChangedObservableCollection<int> data = new CollectionChangedObservableCollection<int>()
{
1,
2,
};

var template = new DataTemplate(() =>
{
return new Grid()
{
new Label()
};
});

var carouselView = new CarouselView()
{
ItemTemplate = template,
ItemsSource = data
};

await CreateHandlerAndAddToWindow<CarouselViewHandler>(carouselView, async (handler) =>
{
await Task.Delay(100);
Assert.NotNull(handler.PlatformView);
Assert.False(data.IsCollectionChangedEventEmpty);
});

carouselView.Handler?.DisconnectHandler();

Assert.True(data.IsCollectionChangedEventEmpty);
}
#endif
}

internal class CustomDataTemplateSelectorSelector : DataTemplateSelector
Expand All @@ -129,4 +169,17 @@ protected override DataTemplate OnSelectTemplate(object item, BindableObject con
return Template2;
}
}
}

internal class CollectionChangedObservableCollection<T> : ObservableCollection<T>, INotifyCollectionChanged
{
NotifyCollectionChangedEventHandler collectionChanged;

event NotifyCollectionChangedEventHandler INotifyCollectionChanged.CollectionChanged
{
add { collectionChanged += value; base.CollectionChanged += value; }
remove { collectionChanged -= value; base.CollectionChanged -= value; }
}

public bool IsCollectionChangedEventEmpty => collectionChanged is null;
}
}
Loading