Conductor.OneActive should close children removed with items.Clear()

Conductor.AllActive reacts appropriately to the Reset event on its items
collection. Conductor.OneActive doesn't, and should.

Fixes #18
This commit is contained in:
Antony Male 2016-11-30 21:00:23 +00:00
parent f100a130d8
commit 151371f9f6
5 changed files with 73 additions and 5 deletions

View File

@ -2,11 +2,21 @@
namespace Stylet.Samples.TabNavigation
{
class Page1ViewModel : Screen
class Page1ViewModel : Screen, IDisposable
{
public Page1ViewModel()
{
this.DisplayName = "Page 1";
}
protected override void OnClose()
{
base.OnClose();
}
public void Dispose()
{
}
}
}

View File

@ -20,7 +20,7 @@ namespace Stylet
{
private readonly BindableCollection<T> items = new BindableCollection<T>();
private T[] itemsBeforeReset;
private List<T> itemsBeforeReset;
/// <summary>
/// Gets all items associated with this conductor
@ -40,7 +40,7 @@ namespace Stylet
switch (e.Action)
{
case NotifyCollectionChangedAction.Reset:
this.itemsBeforeReset = this.items.ToArray();
this.itemsBeforeReset = this.items.ToList();
break;
}
};

View File

@ -16,6 +16,8 @@ namespace Stylet
{
private readonly BindableCollection<T> items = new BindableCollection<T>();
private List<T> itemsBeforeReset;
/// <summary>
/// Gets the tems owned by this Conductor, one of which is active
/// </summary>
@ -29,6 +31,16 @@ namespace Stylet
/// </summary>
public OneActive()
{
this.items.CollectionChanging += (o, e) =>
{
switch (e.Action)
{
case NotifyCollectionChangedAction.Reset:
this.itemsBeforeReset = this.items.ToList();
break;
}
};
this.items.CollectionChanged += (o, e) =>
{
switch (e.Action)
@ -49,8 +61,10 @@ namespace Stylet
break;
case NotifyCollectionChangedAction.Reset:
this.SetParentAndSetActive(this.items, false);
this.CloseAndCleanUp(this.itemsBeforeReset.Except(this.items), this.DisposeChildren);
this.SetParentAndSetActive(this.items.Except(this.itemsBeforeReset), false);
this.ActiveItemMayHaveBeenRemovedFromItems();
this.itemsBeforeReset = null;
break;
}
};
@ -64,7 +78,9 @@ namespace Stylet
if (this.items.Contains(this.ActiveItem))
return;
this.ChangeActiveItem(this.items.FirstOrDefault(), true);
// Only close the previous item if it's in this.items - if it isn't, we'll
// have already have closed it as part of reacting to changes in this.items.
this.ChangeActiveItem(this.items.FirstOrDefault(), this.items.Contains(this.ActiveItem));
}
/// <summary>

View File

@ -273,5 +273,25 @@ namespace StyletUnitTests
screen.Verify(x => x.Close());
}
[Test]
public void ClearingItemsClosesAndDisposes()
{
var screen1 = new Mock<IMyScreen>();
var screen2 = new Mock<IMyScreen>();
this.conductor.ActivateItem(screen1.Object);
this.conductor.ActivateItem(screen2.Object);
this.conductor.Items.Clear();
screen1.Verify(x => x.Deactivate());
screen1.Verify(x => x.Close());
screen1.Verify(x => x.Dispose());
screen2.Verify(x => x.Deactivate());
screen2.Verify(x => x.Close());
screen2.Verify(x => x.Dispose());
}
}
}

View File

@ -142,6 +142,8 @@ namespace StyletUnitTests
public void ClosingClosesAllItems()
{
var screen = new Mock<IMyScreen>();
screen.Setup(x => x.Close()).Callback(() => System.Diagnostics.Debug.WriteLine("FOO"));
((IScreenState)this.conductor).Activate();
this.conductor.ActivateItem(screen.Object);
((IScreenState)this.conductor).Close();
@ -348,5 +350,25 @@ namespace StyletUnitTests
screen.Verify(x => x.Dispose());
Assert.Null(this.conductor.ActiveItem);
}
[Test]
public void ClearingItemsClosesAndDisposes()
{
var screen1 = new Mock<IMyScreen>();
var screen2 = new Mock<IMyScreen>();
this.conductor.ActivateItem(screen1.Object);
this.conductor.ActivateItem(screen2.Object);
this.conductor.Items.Clear();
screen1.Verify(x => x.Deactivate());
screen1.Verify(x => x.Close());
screen1.Verify(x => x.Dispose());
screen2.Verify(x => x.Close());
screen2.Verify(x => x.Dispose());
}
}
}