BindableLayout should disconnect handlers

Author: albyrock87

src/Controls/src/Core/BindableLayout/BindableLayout.cs

@@ -2,6 +2,7 @@
 using System;
 using System.Collections;
 using System.Collections.Specialized;
+using System.Linq;
 using Microsoft.Maui.Controls.Internals;
 
 namespace Microsoft.Maui.Controls
@@ -371,6 +372,7 @@ void CreateChildren()
 			if (childrenCount == 1 && layoutChildren[0] == _currentEmptyView)
 			{
 				layout.RemoveAt(0);
+				_currentEmptyView.DisconnectHandlers();
 				childrenCount = 0;
 			}
 
@@ -394,11 +396,15 @@ void CreateChildren()
 			// Remove exceeding items
 			while (index <= --childrenCount)
 			{
-				var child = (BindableObject)layoutChildren[childrenCount]!;
+				IView child = (IView)layoutChildren[childrenCount]!;
 				layout.RemoveAt(childrenCount);
+
+				// Disconnect platform view so when we clear binding context it doesn't run mappers
+				child.DisconnectHandlers();
+
 				// It's our responsibility to clear the BindingContext for the children
 				// Given that we've set them manually in CreateItemView
-				child.BindingContext = null;
+				ClearBindingContext(child);
 			}
 		}
 
@@ -413,7 +419,7 @@ bool TryAddEmptyView(IBindableLayout layout, out IEnumerator enumerator)
 				// We may have a single child that is either the old empty view or a generated item
 				if (layoutChildren.Count == 1)
 				{
-					var maybeEmptyView = (View)layoutChildren[0]!;
+					var maybeEmptyView = (IView)layoutChildren[0]!;
 
 					// If the current empty view is already in place we have nothing to do
 					if (maybeEmptyView == _currentEmptyView)
@@ -425,11 +431,10 @@ bool TryAddEmptyView(IBindableLayout layout, out IEnumerator enumerator)
 					// So remove it to make room for the new empty view
 					layout.RemoveAt(0);
 
+					// Disconnect platform view so when we clear binding context it doesn't run mappers
+					maybeEmptyView.DisconnectHandlers();
 					// If this is a generated item, we need to clear the BindingContext
-					if (maybeEmptyView.IsSet(BindableLayoutTemplateProperty))
-					{
-						maybeEmptyView.ClearValue(BindableObject.BindingContextProperty);
-					}
+					ClearBindingContext(maybeEmptyView);
 				}
 				else if (layoutChildren.Count > 1)
 				{
@@ -452,14 +457,19 @@ bool TryAddEmptyView(IBindableLayout layout, out IEnumerator enumerator)
 
 		void ClearChildren(IBindableLayout layout)
 		{
-			var index = layout.Children.Count;
-			while (--index >= 0)
+			var layoutChildren = layout.Children.OfType<IView>().ToArray();
+			layout.Clear();
+
+			foreach (var child in layoutChildren)
 			{
-				var child = (View)layout.Children[index]!;
-				layout.RemoveAt(index);
+				// Disconnect platform view so when we clear binding context it doesn't run mappers
+				child.DisconnectHandlers();
 
 				// It's our responsibility to clear the manually-set BindingContext for the generated children
-				child.ClearValue(BindableObject.BindingContextProperty);
+				if (child is BindableObject bindable)
+				{
+					bindable.ClearValue(BindableObject.BindingContextProperty);
+				}
 			}
 		}
 
@@ -515,18 +525,21 @@ void ItemsSourceCollectionChanged(object sender, NotifyCollectionChangedEventArg
 					if (layoutChildren.Count == 1 && layoutChildren[0] == _currentEmptyView)
 					{
 						layout.RemoveAt(0);
+						_currentEmptyView.DisconnectHandlers();
 					}
 
 					layout.Insert(CreateItemView(item, SelectTemplate(item, layout)), index);
 				},
 				removeAt: (item, index) =>
 				{
-					var child = (View)layout.Children[index]!;
+					var child = (IView)layout.Children[index]!;
 					layout.RemoveAt(index);
 
+					// Disconnect platform view so when we clear binding context it doesn't run mappers
+					child.DisconnectHandlers();
 					// It's our responsibility to clear the BindingContext for the children
 					// Given that we've set them manually in CreateItemView
-					child.BindingContext = null;
+					ClearBindingContext(child);
 
 					// If we removed the last item, we need to insert the empty view
 					if (layout.Children.Count == 0 && _currentEmptyView != null)
@@ -540,18 +553,18 @@ void ItemsSourceCollectionChanged(object sender, NotifyCollectionChangedEventArg
 		void ReplaceChild(object item, IBindableLayout layout, IList layoutChildren, int index)
 		{
 			var template = SelectTemplate(item, layout);
-			var child = (BindableObject)layoutChildren[index]!;
-			var currentTemplate = GetBindableLayoutTemplate(child);
-			if (currentTemplate == template)
+			var child = (IView)layoutChildren[index]!;
+			if (child is BindableObject bindable && GetBindableLayoutTemplate(bindable) == template)
 			{
-				child.BindingContext = item;
+				bindable.BindingContext = item;
 			}
 			else
 			{
 				// It's our responsibility to clear the BindingContext for the children
 				// Given that we've set them manually in CreateItemView
-				child.BindingContext = null;
 				layout.Replace(CreateItemView(item, template), index);
+				child.DisconnectHandlers();
+				ClearBindingContext(child);
 			}
 		}
 
@@ -562,5 +575,14 @@ static View CreateItemView(object item, DataTemplate dataTemplate)
 			view.BindingContext = item;
 			return view;
 		}
+
+		
+		static void ClearBindingContext(IView child)
+		{
+			if (child is BindableObject bindable && bindable.IsSet(BindableLayoutTemplateProperty))
+			{
+				bindable.ClearValue(BindableObject.BindingContextProperty);
+			}
+		}
 	}
 }

src/Controls/tests/Core.UnitTests/BindableLayoutTests.cs

@@ -7,6 +7,10 @@
 using System.Linq;
 using System.Reflection;
 using System.Threading.Tasks;
+using Microsoft.Maui.Controls.Hosting;
+using Microsoft.Maui.Handlers;
+using Microsoft.Maui.Hosting;
+using Microsoft.Maui.Platform;
 using Xunit;
 
 namespace Microsoft.Maui.Controls.Core.UnitTests
@@ -652,6 +656,86 @@ public async Task UpdatesAfterGC()
 			Assert.Equal(3, layout.Children.Count);
 		}
 
+		[Fact("BindableLayout disconnects handlers when removing views")]
+		public void DisconnectsHandlersWhenRemovingViews()
+		{
+			var mauiApp = MauiApp.CreateBuilder()
+				.UseMauiApp<ApplicationStub>()
+				.ConfigureMauiHandlers(handlers => handlers.AddHandler<ContentPage, HandlerStub>())
+				.ConfigureMauiHandlers(handlers => handlers.AddHandler<Button, HandlerStub>())
+				.ConfigureMauiHandlers(handlers => handlers.AddHandler<VerticalStackLayout, BindableLayoutHandlerStub>())
+				.Build();
+			
+			MauiContext mauiContext = new MauiContext(mauiApp.Services);
+			
+			var bindableLayout = new VerticalStackLayout();
+			var items = new ObservableCollection<int>(Enumerable.Range(0, 10));
+			BindableLayout.SetItemsSource(bindableLayout, items);
+			BindableLayout.SetItemTemplate(bindableLayout, new DataTemplate(() => new Button()));
+			BindableLayout.SetEmptyView(bindableLayout, new Button());
+			
+			bindableLayout.ToHandler(mauiContext);
+
+			// Ensure we have the handlers on all elements
+			Assert.All(bindableLayout.Children, c => Assert.NotNull(c.Handler));
+
+			// Test removal of an item
+			var lastChildIndex = items.Count - 1;
+			var lastChild = bindableLayout[lastChildIndex];
+			items.RemoveAt(lastChildIndex);
+			Assert.Null(lastChild.Handler);
+
+			// Test removal of all items
+			var children = bindableLayout.Children.ToList();
+			items.Clear();
+			Assert.All(children, c => Assert.Null(c.Handler));
+
+			// Test removal of empty view
+			var emptyView = bindableLayout.FirstOrDefault() as Button;
+			Assert.NotNull(emptyView);
+			Assert.NotNull(emptyView.Handler);
+			items.Add(1000);
+			Assert.Null(emptyView.Handler);
+
+			// Test replacing the items source with an empty enumerable
+			lastChildIndex = 0;
+			lastChild = bindableLayout[lastChildIndex];
+			BindableLayout.SetItemsSource(bindableLayout, Enumerable.Empty<int>());
+			Assert.Null(lastChild.Handler);
+			Assert.NotNull(emptyView.Handler);
+		}
+
+		class BindableLayoutHandlerStub : HandlerStub
+		{
+			public static CommandMapper<IView, IViewHandler> CommandMapper = new()
+			{
+				[nameof(ILayoutHandler.Add)] = MapCreatePlatformHandler,
+				[nameof(ILayoutHandler.Insert)] = MapCreatePlatformHandler,
+			};
+
+			static void MapCreatePlatformHandler(IViewHandler handler, IView view, object arg)
+			{
+				if (arg is LayoutHandlerUpdate args)
+				{
+					args.View.ToHandler(handler.MauiContext!);
+				}
+			}
+
+			public BindableLayoutHandlerStub() : base(new PropertyMapper<IView>(), CommandMapper)
+			{
+			}
+
+			public override void SetVirtualView(IView view)
+			{
+				base.SetVirtualView(view);
+				var bindableLayout = (IBindableLayout)view;
+				foreach (var child in bindableLayout.Children.OfType<IView>())
+				{
+					child.ToHandler(MauiContext!);
+				}
+			}
+		}
+
 		// Checks if for every item in the items source there's a corresponding view
 		static bool IsLayoutWithItemsSource(IEnumerable itemsSource, Compatibility.Layout layout)
 		{

src/Controls/tests/Core.UnitTests/TestClasses/HandlerStub.cs

@@ -15,6 +15,10 @@ public HandlerStub(PropertyMapper mapper) : base(mapper)
 		{
 		}
 
+		public HandlerStub(PropertyMapper mapper, CommandMapper commandMapper) : base(mapper, commandMapper)
+		{
+		}
+
 		protected override object CreatePlatformView()
 		{
 			return new object();