From d3e33ed0bf4b8e2d20b362a1bbb8ece497081a0a Mon Sep 17 00:00:00 2001 From: Adam Gauthier Date: Sat, 7 Sep 2019 14:11:53 -0400 Subject: [PATCH] Unset Parent on Conductors' Items.Clear() ObservableCollection 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. --- .../BindableCollectionTests.cs | 21 +++++++++++++++++++ .../ConductorWithCollectionAllActiveTests.cs | 19 +++++++++++++++++ .../ConductorWithCollectionOneActiveTests.cs | 6 +++--- src/Caliburn.Micro.Core/BindableCollection.cs | 7 +++++++ .../CollectionClearedEventArgs.cs | 15 +++++++++++++ .../ConductorWithCollectionAllActive.cs | 9 +++++--- .../ConductorWithCollectionOneActive.cs | 6 +++++- 7 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 src/Caliburn.Micro.Core.Tests/BindableCollectionTests.cs create mode 100644 src/Caliburn.Micro.Core.Tests/ConductorWithCollectionAllActiveTests.cs create mode 100644 src/Caliburn.Micro.Core/CollectionClearedEventArgs.cs diff --git a/src/Caliburn.Micro.Core.Tests/BindableCollectionTests.cs b/src/Caliburn.Micro.Core.Tests/BindableCollectionTests.cs new file mode 100644 index 00000000..308d0339 --- /dev/null +++ b/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(objects); + + var collectionClearedEvent = Assert.Raises>( + h => bindableCollection.CollectionCleared += h, h => bindableCollection.CollectionCleared -= h, + () => bindableCollection.Clear() + ); + + Assert.Equal(objects, collectionClearedEvent.Arguments.ClearedItems); + } + } +} diff --git a/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionAllActiveTests.cs b/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionAllActiveTests.cs new file mode 100644 index 00000000..ee397d19 --- /dev/null +++ b/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.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)); + } + } +} diff --git a/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs b/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs index 6e3371cd..22d9415f 100644 --- a/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs +++ b/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.Collection.OneActive(); diff --git a/src/Caliburn.Micro.Core/BindableCollection.cs b/src/Caliburn.Micro.Core/BindableCollection.cs index 0a43719f..110a1304 100644 --- a/src/Caliburn.Micro.Core/BindableCollection.cs +++ b/src/Caliburn.Micro.Core/BindableCollection.cs @@ -35,6 +35,11 @@ public BindableCollection(IEnumerable collection) /// public bool IsNotifying { get; set; } + /// + /// Occurs when the collection has been cleared. + /// + public event EventHandler> CollectionCleared; + /// /// Notifies subscribers of the property change. /// @@ -180,7 +185,9 @@ protected override sealed void ClearItems() /// protected virtual void ClearItemsBase() { + var clearedItems = new List(collection: this); base.ClearItems(); + CollectionCleared?.Invoke(this, new CollectionClearedEventArgs(clearedItems)); } /// diff --git a/src/Caliburn.Micro.Core/CollectionClearedEventArgs.cs b/src/Caliburn.Micro.Core/CollectionClearedEventArgs.cs new file mode 100644 index 00000000..4aaa6f69 --- /dev/null +++ b/src/Caliburn.Micro.Core/CollectionClearedEventArgs.cs @@ -0,0 +1,15 @@ +using System; +using System.Collections.Generic; + +namespace Caliburn.Micro +{ + public class CollectionClearedEventArgs : EventArgs + { + public CollectionClearedEventArgs(IReadOnlyCollection clearedItems) + { + ClearedItems = clearedItems; + } + + public IReadOnlyCollection ClearedItems { get; } + } +} diff --git a/src/Caliburn.Micro.Core/ConductorWithCollectionAllActive.cs b/src/Caliburn.Micro.Core/ConductorWithCollectionAllActive.cs index ddcdfc71..d5c27e8e 100644 --- a/src/Caliburn.Micro.Core/ConductorWithCollectionAllActive.cs +++ b/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) /// public AllActive() { + _items.CollectionCleared += (s, e) => + { + e.ClearedItems.OfType().Apply(x => x.Parent = null); + }; _items.CollectionChanged += (s, e) => { switch (e.Action) @@ -80,7 +83,7 @@ protected override Task OnActivateAsync(CancellationToken cancellationToken) /// A task that represents the asynchronous operation. protected override async Task OnDeactivateAsync(bool close, CancellationToken cancellationToken) { - foreach(var deactivate in _items.OfType()) + foreach (var deactivate in _items.OfType()) { await deactivate.DeactivateAsync(close, cancellationToken); } diff --git a/src/Caliburn.Micro.Core/ConductorWithCollectionOneActive.cs b/src/Caliburn.Micro.Core/ConductorWithCollectionOneActive.cs index 9a774acb..abb2f007 100644 --- a/src/Caliburn.Micro.Core/ConductorWithCollectionOneActive.cs +++ b/src/Caliburn.Micro.Core/ConductorWithCollectionOneActive.cs @@ -25,6 +25,10 @@ public class OneActive : ConductorBaseWithActiveItem /// public OneActive() { + _items.CollectionCleared += (s, e) => + { + e.ClearedItems.OfType().Apply(x => x.Parent = null); + }; _items.CollectionChanged += (s, e) => { switch (e.Action) @@ -172,7 +176,7 @@ public override async Task CanCloseAsync(CancellationToken cancellationTok closable = stillToClose; } - foreach(var deactivate in closable.OfType()) + foreach (var deactivate in closable.OfType()) { await deactivate.DeactivateAsync(true, cancellationToken); }