Unset Parent on Conductors' Items.Clear()

ObservableCollection<T> is designed so
NotifyCollectionChangedEventArgs.OldItems is not populated when .Clear()
is called on the collection. Because of this, Caliburn conductors can't
unset the Parent property on all conducted IChild.

There are multiple solutions to this issue that have been discussed, but
I feel like adding a new CollectionCleared event to BindableCollection
is the best compromise considering it doesn't break any existing code
and only extends existing ObservableCollection functionality.

Author: adamgauthier

src/Caliburn.Micro.Core.Tests/BindableCollectionTests.cs

@@ -0,0 +1,21 @@
+using Xunit;
+
+namespace Caliburn.Micro.Core.Tests
+{
+    public class BindableCollectionTests
+    {
+        [Fact]
+        public void Clear_ThenCollectionClearedIsRaisedWithAllItems()
+        {
+            var objects = new[] { new object(), new object(), new object() };
+            var bindableCollection = new BindableCollection<object>(objects);
+
+            var collectionClearedEvent = Assert.Raises<CollectionClearedEventArgs<object>>(
+                h => bindableCollection.CollectionCleared += h, h => bindableCollection.CollectionCleared -= h,
+                () => bindableCollection.Clear()
+            );
+
+            Assert.Equal(objects, collectionClearedEvent.Arguments.ClearedItems);
+        }
+    }
+}

src/Caliburn.Micro.Core.Tests/ConductorWithCollectionAllActiveTests.cs

@@ -0,0 +1,19 @@
+using Xunit;
+
+namespace Caliburn.Micro.Core.Tests
+{
+    public class ConductorCollectionAllActiveTests
+    {
+        [Fact]
+        public void ParentItemIsUnsetOnClear()
+        {
+            var conductor = new Conductor<IScreen>.Collection.AllActive();
+            var conducted = new[] { new Screen(), new Screen() };
+            conductor.Items.AddRange(conducted);
+
+            conductor.Items.Clear();
+
+            Assert.All(conducted, screen => Assert.NotEqual(conductor, screen.Parent));
+        }
+    }
+}

src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs

@@ -44,7 +44,7 @@ public async Task CanCloseIsTrueWhenItemsAreClosable()
             conductor.Items.Add(conducted);
 
             await ((IActivate)conductor).ActivateAsync(CancellationToken.None);
-            var canClose =await conductor.CanCloseAsync(CancellationToken.None);
+            var canClose = await conductor.CanCloseAsync(CancellationToken.None);
 
             Assert.True(canClose);
             Assert.False(conducted.IsClosed);
@@ -62,7 +62,7 @@ public async Task CanCloseIsTrueWhenItemsAreNotClosableAndCloseStrategyCloses()
                 IsClosable = true
             };
             conductor.Items.Add(conducted);
-            await((IActivate)conductor).ActivateAsync(CancellationToken.None);
+            await ((IActivate)conductor).ActivateAsync(CancellationToken.None);
             var canClose = await conductor.CanCloseAsync(CancellationToken.None);
 
             Assert.True(canClose);
@@ -119,7 +119,7 @@ public void ParentItemIsSetOnReplacedConductedItem()
             Assert.Equal(conductor, newConducted.Parent);
         }
 
-        [Fact(Skip = "This is not possible as we don't get the removed items in the event handler.")]
+        [Fact]
         public void ParentItemIsUnsetOnClear()
         {
             var conductor = new Conductor<IScreen>.Collection.OneActive();

src/Caliburn.Micro.Core/BindableCollection.cs

@@ -35,6 +35,11 @@ public BindableCollection(IEnumerable<T> collection)
         /// </summary>
         public bool IsNotifying { get; set; }
 
+        /// <summary>
+        /// Occurs when the collection has been cleared.
+        /// </summary>
+        public event EventHandler<CollectionClearedEventArgs<T>> CollectionCleared;
+
         /// <summary>
         /// Notifies subscribers of the property change.
         /// </summary>
@@ -180,7 +185,9 @@ protected override sealed void ClearItems()
         /// </remarks>
         protected virtual void ClearItemsBase()
         {
+            var clearedItems = new List<T>(collection: this);
             base.ClearItems();
+            CollectionCleared?.Invoke(this, new CollectionClearedEventArgs<T>(clearedItems));
         }
 
         /// <summary>

src/Caliburn.Micro.Core/CollectionClearedEventArgs.cs

@@ -0,0 +1,15 @@
+using System;
+using System.Collections.Generic;
+
+namespace Caliburn.Micro
+{
+    public class CollectionClearedEventArgs<T> : EventArgs
+    {
+        public CollectionClearedEventArgs(IReadOnlyCollection<T> clearedItems)
+        {
+            ClearedItems = clearedItems;
+        }
+
+        public IReadOnlyCollection<T> ClearedItems { get; }
+    }
+}

src/Caliburn.Micro.Core/ConductorWithCollectionAllActive.cs

@@ -1,5 +1,4 @@
-using System;
-using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Linq;
 using System.Reflection;
@@ -38,6 +37,10 @@ public AllActive(bool openPublicItems)
                 /// </summary>
                 public AllActive()
                 {
+                    _items.CollectionCleared += (s, e) =>
+                    {
+                        e.ClearedItems.OfType<IChild>().Apply(x => x.Parent = null);
+                    };
                     _items.CollectionChanged += (s, e) =>
                     {
                         switch (e.Action)
@@ -80,7 +83,7 @@ protected override Task OnActivateAsync(CancellationToken cancellationToken)
                 /// <returns>A task that represents the asynchronous operation.</returns>
                 protected override async Task OnDeactivateAsync(bool close, CancellationToken cancellationToken)
                 {
-                    foreach(var deactivate in _items.OfType<IDeactivate>())
+                    foreach (var deactivate in _items.OfType<IDeactivate>())
                     {
                         await deactivate.DeactivateAsync(close, cancellationToken);
                     }

src/Caliburn.Micro.Core/ConductorWithCollectionOneActive.cs

@@ -25,6 +25,10 @@ public class OneActive : ConductorBaseWithActiveItem<T>
                 /// </summary>
                 public OneActive()
                 {
+                    _items.CollectionCleared += (s, e) =>
+                    {
+                        e.ClearedItems.OfType<IChild>().Apply(x => x.Parent = null);
+                    };
                     _items.CollectionChanged += (s, e) =>
                     {
                         switch (e.Action)
@@ -172,7 +176,7 @@ public override async Task<bool> CanCloseAsync(CancellationToken cancellationTok
                             closable = stillToClose;
                         }
 
-                        foreach(var deactivate in closable.OfType<IDeactivate>())
+                        foreach (var deactivate in closable.OfType<IDeactivate>())
                         {
                             await deactivate.DeactivateAsync(true, cancellationToken);
                         }