From b3601e6f8114872a82776e4b7d43c4c4ea662469 Mon Sep 17 00:00:00 2001 From: Antony Male Date: Sun, 18 Jan 2015 22:02:06 +0000 Subject: [PATCH 01/14] Actions throw on execute of ActionTarget hasn't changed from the default --- Stylet/Xaml/ActionExtension.cs | 9 +++++++++ Stylet/Xaml/CommandAction.cs | 26 +++++++++++++++++++++++--- Stylet/Xaml/EventAction.cs | 14 ++++++++++++++ StyletUnitTests/CommandActionTests.cs | 16 ++++++++++++++++ StyletUnitTests/EventActionTests.cs | 9 +++++++++ 5 files changed, 71 insertions(+), 3 deletions(-) diff --git a/Stylet/Xaml/ActionExtension.cs b/Stylet/Xaml/ActionExtension.cs index 4c3c044..171d5a4 100644 --- a/Stylet/Xaml/ActionExtension.cs +++ b/Stylet/Xaml/ActionExtension.cs @@ -125,6 +125,15 @@ namespace Stylet.Xaml } } + /// + /// The View.ActionTarget was not set. This probably means the item is in a ContextMenu/Popup + /// + [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Usage", "CA2237:MarkISerializableTypesWithSerializable")] + public class ActionNotSetException : Exception + { + internal ActionNotSetException(string message) : base(message) { } + } + /// /// The Action Target was null, and shouldn't have been (NullTarget = Throw) /// diff --git a/Stylet/Xaml/CommandAction.cs b/Stylet/Xaml/CommandAction.cs index 73e1f90..fb715db 100644 --- a/Stylet/Xaml/CommandAction.cs +++ b/Stylet/Xaml/CommandAction.cs @@ -81,7 +81,10 @@ namespace Stylet.Xaml // If they've opted to throw if the target is null, then this will cause that exception. // We'll just wait until the ActionTarget is assigned, and we're called again if (newTarget == View.InitialActionTarget) + { + this.target = newTarget; return; + } this.guardPropertyGetter = null; if (newTarget == null) @@ -95,7 +98,7 @@ namespace Stylet.Xaml } else { - logger.Info("ActionTarget on element {0} is null (method name is {1}), nut NullTarget is not Throw, so carrying on", this.Subject, this.MethodName); + logger.Info("ActionTarget on element {0} is null (method name is {1}), but NullTarget is not Throw, so carrying on", this.Subject, this.MethodName); } } else @@ -147,12 +150,11 @@ namespace Stylet.Xaml if (oldTarget != null) oldTarget.PropertyChanged -= this.PropertyChangedHandler; - this.target = newTarget; - var inpc = newTarget as INotifyPropertyChanged; if (this.guardPropertyGetter != null && inpc != null) inpc.PropertyChanged += this.PropertyChangedHandler; + this.target = newTarget; this.targetMethodInfo = targetMethodInfo; this.UpdateCanExecute(); @@ -183,6 +185,13 @@ namespace Stylet.Xaml /// true if this command can be executed; otherwise, false. public bool CanExecute(object parameter) { + // This can happen if the ActionTarget hasn't been set from its default - + // e.g. if the button/etc in question is in a ContextMenu/Popup, which attached properties + // aren't inherited by. + // Show the control as enabled, but throw if they try and click on it + if (this.target == View.InitialActionTarget) + return true; + // It's enabled only if both the targetNull and actionNonExistent tests pass // Throw is handled when the target is set @@ -215,6 +224,17 @@ namespace Stylet.Xaml /// Data used by the command. If the command does not require data to be passed, this object can be set to null. public void Execute(object parameter) { + // If we've made it this far and the target is still the default, then something's wrong + // Make sure they know + if (this.target == View.InitialActionTarget) + { + var e = new ActionNotSetException(String.Format("View.ActionTarget not on control {0} (method {1}). " + + "This probably means the control hasn't inherited it from a parent, e.g. because a ContextMenu or Popup sits in the visual tree. " + + "You will need so set 's:View.ActionTarget' explicitly. See the wiki for more details.", this.Subject, this.MethodName)); + logger.Error(e); + throw e; + } + // Any throwing would have been handled prior to this if (this.target == null || this.targetMethodInfo == null) return; diff --git a/Stylet/Xaml/EventAction.cs b/Stylet/Xaml/EventAction.cs index e9edc76..eddc48e 100644 --- a/Stylet/Xaml/EventAction.cs +++ b/Stylet/Xaml/EventAction.cs @@ -77,7 +77,10 @@ namespace Stylet.Xaml // If they've opted to throw if the target is null, then this will cause that exception. // We'll just wait until the ActionTarget is assigned, and we're called again if (newTarget == View.InitialActionTarget) + { + this.target = newTarget; return; + } if (newTarget == null) { @@ -146,6 +149,17 @@ namespace Stylet.Xaml // ReSharper disable once UnusedMember.Local private void InvokeCommand(object sender, EventArgs e) { + // If we've made it this far and the target is still the default, then something's wrong + // Make sure they know + if (this.target == View.InitialActionTarget) + { + var ex = new ActionNotSetException(String.Format("View.ActionTarget not on control {0} (method {1}). " + + "This probably means the control hasn't inherited it from a parent, e.g. because a ContextMenu or Popup sits in the visual tree. " + + "You will need so set 's:View.ActionTarget' explicitly. See the wiki for more details.", this.subject, this.methodName)); + logger.Error(ex); + throw ex; + } + // Any throwing will have been handled above if (this.target == null || this.targetMethodInfo == null) return; diff --git a/StyletUnitTests/CommandActionTests.cs b/StyletUnitTests/CommandActionTests.cs index 1cc9f88..0c25491 100644 --- a/StyletUnitTests/CommandActionTests.cs +++ b/StyletUnitTests/CommandActionTests.cs @@ -221,5 +221,21 @@ namespace StyletUnitTests var e = Assert.Throws(() => cmd.Execute(null)); Assert.AreEqual("woo", e.Message); } + + [Test] + public void ControlIsEnabledIfTargetIsDefault() + { + View.SetActionTarget(this.subject, View.InitialActionTarget); + var cmd = new CommandAction(this.subject, "DoSomething", ActionUnavailableBehaviour.Throw, ActionUnavailableBehaviour.Throw); + Assert.True(cmd.CanExecute(null)); + } + + [Test] + public void ExecuteThrowsIfTargetIsDefault() + { + View.SetActionTarget(this.subject, View.InitialActionTarget); + var cmd = new CommandAction(this.subject, "DoSomething", ActionUnavailableBehaviour.Throw, ActionUnavailableBehaviour.Throw); + Assert.Throws(() => cmd.Execute(null)); + } } } diff --git a/StyletUnitTests/EventActionTests.cs b/StyletUnitTests/EventActionTests.cs index 4221d64..69321f8 100644 --- a/StyletUnitTests/EventActionTests.cs +++ b/StyletUnitTests/EventActionTests.cs @@ -190,5 +190,14 @@ namespace StyletUnitTests Assert.IsInstanceOf(e.InnerException); Assert.AreEqual("foo", e.InnerException.Message); } + + [Test] + public void ExecuteThrowsIfActionTargetIsDefault() + { + View.SetActionTarget(this.subject, View.InitialActionTarget); + var cmd = new EventAction(this.subject, this.eventInfo.EventHandlerType, "DoSomething", ActionUnavailableBehaviour.Throw, ActionUnavailableBehaviour.Throw); + var e = Assert.Throws(() => cmd.GetDelegate().DynamicInvoke(null, null)); + Assert.IsInstanceOf(e.InnerException); + } } } From 3785fbf2afaf14d15cd6bbd4b562cc02d4fda402 Mon Sep 17 00:00:00 2001 From: Antony Male Date: Sun, 18 Jan 2015 22:05:59 +0000 Subject: [PATCH 02/14] Add test to ensure guard method is propagated --- StyletUnitTests/CommandActionTests.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/StyletUnitTests/CommandActionTests.cs b/StyletUnitTests/CommandActionTests.cs index 0c25491..797772a 100644 --- a/StyletUnitTests/CommandActionTests.cs +++ b/StyletUnitTests/CommandActionTests.cs @@ -51,6 +51,15 @@ namespace StyletUnitTests { throw new InvalidOperationException("woo"); } + + public bool CanDoSomethingWithUnsuccessfulGuardMethod + { + get { throw new InvalidOperationException("foo"); } + } + + public void DoSomethingWithUnsuccessfulGuardMethod() + { + } } private class Target2 @@ -222,6 +231,14 @@ namespace StyletUnitTests Assert.AreEqual("woo", e.Message); } + [Test] + public void PropagatesGuardPropertException() + { + var cmd = new CommandAction(this.subject, "DoSomethingWithUnsuccessfulGuardMethod", ActionUnavailableBehaviour.Throw, ActionUnavailableBehaviour.Throw); + var e = Assert.Throws(() => cmd.CanExecute(null)); + Assert.AreEqual("foo", e.Message); + } + [Test] public void ControlIsEnabledIfTargetIsDefault() { From 2481cec3fa7f943dfa8775dc24bb770a4946c164 Mon Sep 17 00:00:00 2001 From: Antony Male Date: Sun, 18 Jan 2015 22:11:21 +0000 Subject: [PATCH 03/14] Shuffle where hooks are called from in BootstrapperBase slightly --- Stylet/BootstrapperBase.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Stylet/BootstrapperBase.cs b/Stylet/BootstrapperBase.cs index cbc1bc6..f5a677d 100644 --- a/Stylet/BootstrapperBase.cs +++ b/Stylet/BootstrapperBase.cs @@ -66,8 +66,11 @@ namespace Stylet }; // Fetch this logger when needed. If we fetch it now, then no-one will have been given the change to enable the LogManager, and we'll get a NullLogger - this.Application.DispatcherUnhandledException += (o, e) => LogManager.GetLogger(typeof(BootstrapperBase)).Error(e.Exception, "Unhandled exception"); - this.Application.DispatcherUnhandledException += (o, e) => this.OnUnhandledExecption(e); + this.Application.DispatcherUnhandledException += (o, e) => + { + LogManager.GetLogger(typeof(BootstrapperBase)).Error(e.Exception, "Unhandled exception"); + this.OnUnhandledExecption(e); + }; } /// @@ -84,6 +87,7 @@ namespace Stylet View.ViewManager = (IViewManager)this.GetInstance(typeof(IViewManager)); this.Launch(); + this.OnStartup(); } /// @@ -93,7 +97,6 @@ namespace Stylet { var windowManager = (IWindowManager)this.GetInstance(typeof(IWindowManager)); windowManager.ShowWindow(this.RootViewModel); - this.OnStartup(); } /// @@ -114,7 +117,7 @@ namespace Stylet protected virtual void OnStartup() { } /// - /// Hook used internall by the Bootstrapper to do things like dispose the IoC container + /// Hook used internally by the Bootstrapper to do things like dispose the IoC container /// /// The exit event data protected virtual void OnExitInternal(ExitEventArgs e) { } From e2dc463d01377c92f90fdfbec8cf3b6d9abd3e95 Mon Sep 17 00:00:00 2001 From: Antony Male Date: Sun, 18 Jan 2015 22:15:05 +0000 Subject: [PATCH 04/14] Sort logging classes into different files --- Stylet/Logging/ILogger.cs | 31 ++++++++++ Stylet/Logging/LogManager.cs | 106 ---------------------------------- Stylet/Logging/NullLogger.cs | 31 ++++++++++ Stylet/Logging/TraceLogger.cs | 55 ++++++++++++++++++ 4 files changed, 117 insertions(+), 106 deletions(-) create mode 100644 Stylet/Logging/ILogger.cs create mode 100644 Stylet/Logging/NullLogger.cs create mode 100644 Stylet/Logging/TraceLogger.cs diff --git a/Stylet/Logging/ILogger.cs b/Stylet/Logging/ILogger.cs new file mode 100644 index 0000000..4954563 --- /dev/null +++ b/Stylet/Logging/ILogger.cs @@ -0,0 +1,31 @@ +using System; + +namespace Stylet.Logging +{ + /// + /// Logger used by Stylet for internal logging + /// + public interface ILogger + { + /// + /// Log the message as info + /// + /// A formatted message + /// format parameters + void Info(string format, params object[] args); + + /// + /// Log the message as a warning + /// + /// A formatted message + /// format parameters + void Warn(string format, params object[] args); + + /// + /// Log an exception as an error + /// + /// Exception to log + /// Additional message to add to the exception + void Error(Exception exception, string message = null); + } +} diff --git a/Stylet/Logging/LogManager.cs b/Stylet/Logging/LogManager.cs index 4135f4e..1fb47da 100644 --- a/Stylet/Logging/LogManager.cs +++ b/Stylet/Logging/LogManager.cs @@ -1,113 +1,7 @@ using System; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; namespace Stylet.Logging { - /// - /// Logger used by Stylet for internal logging - /// - public interface ILogger - { - /// - /// Log the message as info - /// - /// A formatted message - /// format parameters - void Info(string format, params object[] args); - - /// - /// Log the message as a warning - /// - /// A formatted message - /// format parameters - void Warn(string format, params object[] args); - - /// - /// Log an exception as an error - /// - /// Exception to log - /// Additional message to add to the exception - void Error(Exception exception, string message = null); - } - - /// - /// ILogger implementation which does nothing - used by default - /// - public class NullLogger : ILogger - { - /// - /// Log the message as info - /// - /// A formatted message - /// format parameters - public void Info(string format, params object[] args) { } - - /// - /// Log the message as a warning - /// - /// A formatted message - /// format parameters - public void Warn(string format, params object[] args) { } - - /// - /// Log an exception as an error - /// - /// Exception to log - /// Additional message to add to the exception - public void Error(Exception exception, string message = null) { } - } - - /// - /// ILogger implementation which uses Debug.WriteLine - /// - public class TraceLogger : ILogger - { - private readonly string name; - - /// - /// Initialises a new instance of the class, with the given name - /// - /// Name of the DebugLogger - public TraceLogger(string name) - { - this.name = name; - } - - /// - /// Log the message as info - /// - /// A formatted message - /// format parameters - public void Info(string format, params object[] args) - { - Trace.WriteLine(String.Format("INFO [{1}] {0}", String.Format(format, args), this.name), "Stylet"); - } - - /// - /// Log the message as a warning - /// - /// A formatted message - /// format parameters - public void Warn(string format, params object[] args) - { - Trace.WriteLine(String.Format("WARN [{1}] {0}", String.Format(format, args), this.name), "Stylet"); - } - - /// - /// Log an exception as an error - /// - /// Exception to log - /// Additional message to add to the exception - public void Error(Exception exception, string message = null) - { - if (message == null) - Trace.WriteLine(String.Format("ERROR [{1}] {0}", exception, this.name), "Stylet"); - else - Trace.WriteLine(String.Format("ERROR [{2}] {0} {1}", message, exception, this.name), "Stylet"); - } - } - /// /// Manager for ILoggers. Used to create new ILoggers, and set up how ILoggers are created /// diff --git a/Stylet/Logging/NullLogger.cs b/Stylet/Logging/NullLogger.cs new file mode 100644 index 0000000..ba4afba --- /dev/null +++ b/Stylet/Logging/NullLogger.cs @@ -0,0 +1,31 @@ +using System; + +namespace Stylet.Logging +{ + /// + /// ILogger implementation which does nothing - used by default + /// + public class NullLogger : ILogger + { + /// + /// Log the message as info + /// + /// A formatted message + /// format parameters + public void Info(string format, params object[] args) { } + + /// + /// Log the message as a warning + /// + /// A formatted message + /// format parameters + public void Warn(string format, params object[] args) { } + + /// + /// Log an exception as an error + /// + /// Exception to log + /// Additional message to add to the exception + public void Error(Exception exception, string message = null) { } + } +} diff --git a/Stylet/Logging/TraceLogger.cs b/Stylet/Logging/TraceLogger.cs new file mode 100644 index 0000000..1fde875 --- /dev/null +++ b/Stylet/Logging/TraceLogger.cs @@ -0,0 +1,55 @@ +using System; +using System.Diagnostics; + +namespace Stylet.Logging +{ + /// + /// ILogger implementation which uses Debug.WriteLine + /// + public class TraceLogger : ILogger + { + private readonly string name; + + /// + /// Initialises a new instance of the class, with the given name + /// + /// Name of the DebugLogger + public TraceLogger(string name) + { + this.name = name; + } + + /// + /// Log the message as info + /// + /// A formatted message + /// format parameters + public void Info(string format, params object[] args) + { + Trace.WriteLine(String.Format("INFO [{1}] {0}", String.Format(format, args), this.name), "Stylet"); + } + + /// + /// Log the message as a warning + /// + /// A formatted message + /// format parameters + public void Warn(string format, params object[] args) + { + Trace.WriteLine(String.Format("WARN [{1}] {0}", String.Format(format, args), this.name), "Stylet"); + } + + /// + /// Log an exception as an error + /// + /// Exception to log + /// Additional message to add to the exception + public void Error(Exception exception, string message = null) + { + if (message == null) + Trace.WriteLine(String.Format("ERROR [{1}] {0}", exception, this.name), "Stylet"); + else + Trace.WriteLine(String.Format("ERROR [{2}] {0} {1}", message, exception, this.name), "Stylet"); + } + } +} From 768509cacc4d768d52fe1836517ccc5228152fb6 Mon Sep 17 00:00:00 2001 From: Antony Male Date: Mon, 19 Jan 2015 12:08:22 +0000 Subject: [PATCH 05/14] Actually add new Logger files to project --- Stylet/Stylet.csproj | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Stylet/Stylet.csproj b/Stylet/Stylet.csproj index 9d41374..9510433 100644 --- a/Stylet/Stylet.csproj +++ b/Stylet/Stylet.csproj @@ -49,6 +49,9 @@ + + + From 4468740338244ce2a3c3e786b1cd50352e0b5fa4 Mon Sep 17 00:00:00 2001 From: Antony Male Date: Mon, 19 Jan 2015 12:57:13 +0000 Subject: [PATCH 06/14] Implement proper state management in Screen This takes the old flag-based state management in Screen, and replaces it with one based on a ScreenState enum. This also gives a way for interested parties to query the current state, and for an event which notifies of any state transition. This meant combining IActivate, IDeactivate, and IClose. This is a sensible move: these interfaces were inextricably linked anyway, and separating them had no advantages and a few disadvantages. If external parties are using these interfaces directly, then migration is necessary, but these are usually only used by Conductors --- Stylet/ConductorAllActive.cs | 4 +- Stylet/IScreen.cs | 164 +++++++++++++--- Stylet/Screen.cs | 197 +++++++++++--------- Stylet/ScreenExtensions.cs | 36 ++-- Stylet/WindowManager.cs | 10 +- StyletUnitTests/ConductorAllActiveTests.cs | 18 +- StyletUnitTests/ConductorNavigatingTests.cs | 24 +-- StyletUnitTests/ConductorOneActiveTests.cs | 26 +-- StyletUnitTests/ConductorTests.cs | 26 +-- StyletUnitTests/ScreenExtensionTests.cs | 32 ++-- StyletUnitTests/ScreenTests.cs | 139 +++++++++++--- 11 files changed, 437 insertions(+), 239 deletions(-) diff --git a/Stylet/ConductorAllActive.cs b/Stylet/ConductorAllActive.cs index 54d28d0..6680636 100644 --- a/Stylet/ConductorAllActive.cs +++ b/Stylet/ConductorAllActive.cs @@ -86,7 +86,7 @@ namespace Stylet /// protected override void OnActivate() { - foreach (var item in this.items.OfType()) + foreach (var item in this.items.OfType()) { item.Activate(); } @@ -97,7 +97,7 @@ namespace Stylet /// protected override void OnDeactivate() { - foreach (var item in this.items.OfType()) + foreach (var item in this.items.OfType()) { item.Deactivate(); } diff --git a/Stylet/IScreen.cs b/Stylet/IScreen.cs index be3361c..8a6b03e 100644 --- a/Stylet/IScreen.cs +++ b/Stylet/IScreen.cs @@ -23,51 +23,80 @@ namespace Stylet } /// - /// Can be activated, and raises an event when it is actually activated + /// State in which a screen can be /// - public interface IActivate + public enum ScreenState { /// - /// Activate the object. May not actually cause activation (e.g. if it's already active) + /// Screen has been created, but has never had any further transitions /// - void Activate(); - + Initial, + + /// + /// Screen is active. It is likely being displayed to the user + /// + Active, + + /// + /// Screen is deactivated. It has either been hidden in favour of another Screen, or the entire window has been minimised + /// + Deactivated, + + /// + /// Screen has been closed. It has no associated View, but may yet be displayed again + /// + Closed, + } + + /// + /// Has a concept of state, which can be manipulated by its Conductor + /// + public interface IScreenState + { + /// + /// Gets the current state of the Screen + /// + ScreenState State { get; } + + /// + /// Gets a value indicating whether the current state is ScreenState.Active + /// + bool IsActive { get; } + + /// + /// Raised when the Screen's state changed, for any reason + /// + event EventHandler StateChanged; + /// /// Raised when the object is actually activated /// event EventHandler Activated; - } - /// - /// Can be deactivated, and raises an event when it is actually deactivated - /// - public interface IDeactivate - { + /// + /// Raised when the object is actually deactivated + /// + event EventHandler Deactivated; + + /// + /// Raised when the object is actually closed + /// + event EventHandler Closed; + + /// + /// Activate the object. May not actually cause activation (e.g. if it's already active) + /// + void Activate(); + /// /// Deactivate the object. May not actually cause deactivation (e.g. if it's already deactive) /// void Deactivate(); - /// - /// Raised when the object is actually deactivated - /// - event EventHandler Deactivated; - } - - /// - /// Can be closed, and raises an event when it is actually closed - /// - public interface IClose - { /// /// Close the object. May not actually cause closure (e.g. if it's already closed) /// void Close(); - - /// - /// Raised when the object is actually closed - /// - event EventHandler Closed; } /// @@ -120,28 +149,101 @@ namespace Stylet /// /// Generalised 'screen' composing all the behaviours expected of a screen /// - public interface IScreen : IViewAware, IHaveDisplayName, IActivate, IDeactivate, IChild, IClose, IGuardClose, IRequestClose + public interface IScreen : IViewAware, IHaveDisplayName, IScreenState, IChild, IGuardClose, IRequestClose { } /// - /// EventArgs associated with the IActivate.Activated event + /// EventArgs associated with the IScreenState.StateChanged event + /// + public class ScreenStateChangedEventArgs : EventArgs + { + /// + /// Gets the state being transitioned to + /// + public ScreenState NewState { get; private set; } + + /// + /// Gets the state being transitioned away from + /// + public ScreenState PreviousState { get; private set; } + + /// + /// Initialises a new instance of the class + /// + /// State being transitioned to + /// State being transitioned away from + public ScreenStateChangedEventArgs(ScreenState newState, ScreenState previousState) + { + this.NewState = newState; + this.PreviousState = previousState; + } + } + + /// + /// EventArgs associated with the IScreenState.Activated event /// public class ActivationEventArgs : EventArgs { + /// + /// Gets a value indicating whether this is the first time this Screen has been activated, ever + /// + public bool IsInitialActivate { get; private set; } + + /// + /// Gets the state being transitioned away from + /// + public ScreenState PreviousState { get; private set; } + + /// + /// Initialises a new instance of the class + /// + /// State being transitioned away from + /// True if this is the first time this screen has ever been activated + public ActivationEventArgs(ScreenState previousState, bool isInitialActivate) + { + this.IsInitialActivate = isInitialActivate; + this.PreviousState = previousState; + } } /// - /// EventArgs associated with the IDeactivate.Deactivated event + /// EventArgs associated with the IScreenState.Deactivated event /// public class DeactivationEventArgs : EventArgs { + /// + /// Gets the state being transitioned away from + /// + public ScreenState PreviousState { get; private set; } + + /// + /// Initialises a new instance of the class + /// + /// State being transitioned away from + public DeactivationEventArgs(ScreenState previousState) + { + this.PreviousState = previousState; + } } /// - /// EventArgs associated with the IClose.Closed event + /// EventArgs associated with the IScreenState.Closed event /// public class CloseEventArgs : EventArgs { + /// + /// Gets the state being transitioned away from + /// + public ScreenState PreviousState { get; private set; } + + /// + /// Initialises a new instance of the class + /// + /// State being transitioned away from + public CloseEventArgs(ScreenState previousState) + { + this.PreviousState = previousState; + } } } diff --git a/Stylet/Screen.cs b/Stylet/Screen.cs index 13b75cc..34b4d6e 100644 --- a/Stylet/Screen.cs +++ b/Stylet/Screen.cs @@ -46,47 +46,50 @@ namespace Stylet #endregion - #region IActivate + #region IScreenState + + private ScreenState _state = ScreenState.Initial; + + /// + /// Gets or sets the current state of the Screen + /// + public virtual ScreenState State + { + get { return this._state; } + protected set + { + this.SetAndNotify(ref this._state, value); + this.NotifyOfPropertyChange(() => this.IsActive); + } + } + + /// + /// Gets a value indicating whether the current state is ScreenState.Active + /// + public bool IsActive + { + get { return this.State == ScreenState.Active; } + } + + /// + /// Raised when the Screen's state changed, for any reason + /// + public event EventHandler StateChanged; /// /// Fired whenever the Screen is activated /// public event EventHandler Activated; - private bool hasBeenActivatedEver; - - private bool _isActive; + /// + /// Fired whenever the Screen is deactivated + /// + public event EventHandler Deactivated; /// - /// Gets or sets a value indicating whether this Screen is currently active + /// Called whenever this Screen is closed /// - public bool IsActive - { - get { return this._isActive; } - set { this.SetAndNotify(ref this._isActive, value); } - } - - [SuppressMessage("Microsoft.Design", "CA1033:InterfaceMethodsShouldBeCallableByChildTypes", Justification = "As this is a framework type, don't want to make it too easy for users to call this method")] - void IActivate.Activate() - { - if (this.IsActive) - return; - - this.IsActive = true; - this.isClosed = false; - - this.logger.Info("Activating"); - - if (!this.hasBeenActivatedEver) - this.OnInitialActivate(); - this.hasBeenActivatedEver = true; - - this.OnActivate(); - - var handler = this.Activated; - if (handler != null) - handler(this, new ActivationEventArgs()); - } + public event EventHandler Closed; /// /// Called the very first time this Screen is activated, and never again @@ -98,75 +101,87 @@ namespace Stylet /// protected virtual void OnActivate() { } - #endregion - - #region IDeactivate - - /// - /// Fired whenever the Screen is deactivated - /// - public event EventHandler Deactivated; - - [SuppressMessage("Microsoft.Design", "CA1033:InterfaceMethodsShouldBeCallableByChildTypes", Scope = "member", Target = "Stylet.Screen.#Stylet.IDeactivate.Deactivate()", Justification = "As this is a framework type, don't want to make it too easy for users to call this method")] - void IDeactivate.Deactivate() - { - if (!this.IsActive) - return; - - this.IsActive = false; - this.isClosed = false; - - this.logger.Info("Deactivating"); - - this.OnDeactivate(); - - var handler = this.Deactivated; - if (handler != null) - handler(this, new DeactivationEventArgs()); - } - /// /// Called every time this screen is deactivated /// protected virtual void OnDeactivate() { } - #endregion - - #region IClose - - private bool isClosed; - - /// - /// Called whenever this Screen is closed - /// - public event EventHandler Closed; - - [SuppressMessage("Microsoft.Design", "CA1033:InterfaceMethodsShouldBeCallableByChildTypes", Justification = "As this is a framework type, don't want to make it too easy for users to call this method")] - void IClose.Close() - { - if (this.isClosed) - return; - - // This will early-exit if it's already deactive - ((IDeactivate)this).Deactivate(); - - this.View = null; - this.isClosed = true; - - this.logger.Info("Closing"); - - this.OnClose(); - - var handler = this.Closed; - if (handler != null) - handler(this, new CloseEventArgs()); - } - /// /// Called when this screen is closed /// protected virtual void OnClose() { } + /// + /// Sets the screen's state to the given state, if it differs from the current state + /// + /// State to transition to + /// Called if the transition occurs. Arguments are (newState, previousState) + protected virtual void SetState(ScreenState newState, Action changedHandler) + { + if (newState == this.State) + return; + + var previousState = this.State; + this.State = newState; + + this.logger.Info("Setting state from {0} to {1}", previousState, newState); + + changedHandler(previousState, newState); + + var handler = this.StateChanged; + if (handler != null) + Execute.OnUIThread(() => handler(this, new ScreenStateChangedEventArgs(newState, previousState))); + } + + [SuppressMessage("Microsoft.Design", "CA1033:InterfaceMethodsShouldBeCallableByChildTypes", Justification = "As this is a framework type, don't want to make it too easy for users to call this method")] + void IScreenState.Activate() + { + this.SetState(ScreenState.Active, (oldState, newState) => + { + var isInitialActivate = oldState == ScreenState.Initial; + if (isInitialActivate) + this.OnInitialActivate(); + + this.OnActivate(); + + var handler = this.Activated; + if (handler != null) + Execute.OnUIThread(() => handler(this, new ActivationEventArgs(oldState, isInitialActivate))); + }); + } + + [SuppressMessage("Microsoft.Design", "CA1033:InterfaceMethodsShouldBeCallableByChildTypes", Justification = "As this is a framework type, don't want to make it too easy for users to call this method")] + void IScreenState.Deactivate() + { + this.SetState(ScreenState.Deactivated, (oldState, newState) => + { + this.OnDeactivate(); + + var handler = this.Deactivated; + if (handler != null) + Execute.OnUIThread(() => handler(this, new DeactivationEventArgs(oldState))); + }); + } + + [SuppressMessage("Microsoft.Design", "CA1033:InterfaceMethodsShouldBeCallableByChildTypes", Justification = "As this is a framework type, don't want to make it too easy for users to call this method")] + void IScreenState.Close() + { + // Avoid going from Closed back to Deactivated + if (this.State != ScreenState.Closed) + ((IScreenState)this).Deactivate(); + + this.View = null; + + this.SetState(ScreenState.Closed, (oldState, newState) => + { + this.OnClose(); + + var handler = this.Closed; + if (handler != null) + Execute.OnUIThread(() => handler(this, new CloseEventArgs(oldState))); + }); + } + #endregion #region IViewAware diff --git a/Stylet/ScreenExtensions.cs b/Stylet/ScreenExtensions.cs index da04a10..f6f2c56 100644 --- a/Stylet/ScreenExtensions.cs +++ b/Stylet/ScreenExtensions.cs @@ -14,9 +14,9 @@ namespace Stylet /// Screen to activate public static void TryActivate(object screen) { - var screenAsActivate = screen as IActivate; - if (screenAsActivate != null) - screenAsActivate.Activate(); + var screenAsScreenState = screen as IScreenState; + if (screenAsScreenState != null) + screenAsScreenState.Activate(); } /// @@ -25,9 +25,9 @@ namespace Stylet /// Screen to deactivate public static void TryDeactivate(object screen) { - var screenAsDeactivate = screen as IDeactivate; - if (screenAsDeactivate != null) - screenAsDeactivate.Deactivate(); + var screenAsScreenState = screen as IScreenState; + if (screenAsScreenState != null) + screenAsScreenState.Deactivate(); } /// @@ -36,9 +36,9 @@ namespace Stylet /// Screen to close public static void TryClose(object screen) { - var screenAsClose = screen as IClose; - if (screenAsClose != null) - screenAsClose.Close(); + var screenAsScreenState = screen as IScreenState; + if (screenAsScreenState != null) + screenAsScreenState.Close(); } /// @@ -58,9 +58,9 @@ namespace Stylet /// child.ActivateWith(this) /// Child to activate whenever the parent is activated /// Parent to observe - public static void ActivateWith(this IActivate child, IActivate parent) + public static void ActivateWith(this IScreenState child, IScreenState parent) { - WeakEventManager.AddHandler(parent, "Activated", (o, e) => child.Activate()); + WeakEventManager.AddHandler(parent, "Activated", (o, e) => child.Activate()); } /// @@ -69,9 +69,9 @@ namespace Stylet /// child.DeactivateWith(this) /// Child to deactivate whenever the parent is deacgtivated /// Parent to observe - public static void DeactivateWith(this IDeactivate child, IDeactivate parent) + public static void DeactivateWith(this IScreenState child, IScreenState parent) { - WeakEventManager.AddHandler(parent, "Deactivated", (o, e) => child.Deactivate()); + WeakEventManager.AddHandler(parent, "Deactivated", (o, e) => child.Deactivate()); } /// @@ -80,23 +80,19 @@ namespace Stylet /// child.CloseWith(this) /// Child to close when the parent is closed /// Parent to observe - public static void CloseWith(this IClose child, IClose parent) + public static void CloseWith(this IScreenState child, IScreenState parent) { // Using TryCloseAndDispose ensures that Dispose is called if necessary - WeakEventManager.AddHandler(parent, "Closed", (o, e) => TryClose(child)); + WeakEventManager.AddHandler(parent, "Closed", (o, e) => TryClose(child)); } /// /// Activate, Deactivate, or Close the child whenever the parent is Activated, Deactivated, or Closed /// /// child.ConductWith(this) - /// Type of the child - /// Type of the parent /// Child to conduct with the parent /// Parent to observe - public static void ConductWith(this TChild child, TParent parent) - where TChild : IActivate, IDeactivate, IClose - where TParent : IActivate, IDeactivate, IClose + public static void ConductWith(this IScreenState child, IScreenState parent) { child.ActivateWith(parent); child.DeactivateWith(parent); diff --git a/Stylet/WindowManager.cs b/Stylet/WindowManager.cs index 4f629df..808cb27 100644 --- a/Stylet/WindowManager.cs +++ b/Stylet/WindowManager.cs @@ -193,15 +193,15 @@ namespace Stylet ScreenExtensions.TryActivate(this.viewModel); - var viewModelAsClose = this.viewModel as IClose; - if (viewModelAsClose != null) + var viewModelAsScreenState = this.viewModel as IScreenState; + if (viewModelAsScreenState != null) + { + window.StateChanged += this.WindowStateChanged; window.Closed += this.WindowClosed; + } if (this.viewModel is IGuardClose) window.Closing += this.WindowClosing; - - if (this.viewModel is IActivate || this.viewModel is IDeactivate) - window.StateChanged += this.WindowStateChanged; } private void WindowStateChanged(object sender, EventArgs e) diff --git a/StyletUnitTests/ConductorAllActiveTests.cs b/StyletUnitTests/ConductorAllActiveTests.cs index 2259904..ed1a8f9 100644 --- a/StyletUnitTests/ConductorAllActiveTests.cs +++ b/StyletUnitTests/ConductorAllActiveTests.cs @@ -57,7 +57,7 @@ namespace StyletUnitTests public void ActivateItemActivatesfConductorIsActive() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); screen.Verify(x => x.Activate()); } @@ -66,7 +66,7 @@ namespace StyletUnitTests public void ActivateItemDoesNotDeactivateIfConductorIsActive() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); screen.Verify(x => x.Deactivate(), Times.Never); } @@ -75,9 +75,9 @@ namespace StyletUnitTests public void DeactiveDeactivatesItems() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); - ((IDeactivate)this.conductor).Deactivate(); + ((IScreenState)this.conductor).Deactivate(); screen.Verify(x => x.Deactivate()); } @@ -85,9 +85,9 @@ namespace StyletUnitTests public void ClosingClosesAllItems() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); - ((IClose)this.conductor).Close(); + ((IScreenState)this.conductor).Close(); screen.Verify(x => x.Close()); screen.Verify(x => x.Dispose()); Assert.AreEqual(0, this.conductor.Items.Count); @@ -122,7 +122,7 @@ namespace StyletUnitTests [Test] public void AddingItemActivatesAndSetsParent() { - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); var screen = new Mock(); this.conductor.Items.Add(screen.Object); screen.VerifySet(x => x.Parent = this.conductor); @@ -211,7 +211,7 @@ namespace StyletUnitTests this.conductor.ActivateItem(screen1.Object); this.conductor.ActivateItem(screen2.Object); - ((IClose)this.conductor).Close(); + ((IScreenState)this.conductor).Close(); screen1.Verify(x => x.Close()); screen1.Verify(x => x.Dispose()); screen1.VerifySet(x => x.Parent = null); @@ -237,7 +237,7 @@ namespace StyletUnitTests public void AddRangeActivatesAddedItems() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.Items.AddRange(new[] { screen.Object }); diff --git a/StyletUnitTests/ConductorNavigatingTests.cs b/StyletUnitTests/ConductorNavigatingTests.cs index 780fe64..8b87ddb 100644 --- a/StyletUnitTests/ConductorNavigatingTests.cs +++ b/StyletUnitTests/ConductorNavigatingTests.cs @@ -49,7 +49,7 @@ namespace StyletUnitTests [Test] public void InitialActivateActivatesItemIfConductorIsActive() { - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); var screen = new Mock(); this.conductor.ActivateItem(screen.Object); screen.Verify(x => x.Activate()); @@ -70,17 +70,17 @@ namespace StyletUnitTests this.conductor.ActivateItem(screen.Object); screen.Verify(x => x.Activate(), Times.Never); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); screen.Verify(x => x.Activate()); } [Test] public void DeactivatesActiveItemWhenDeactivated() { - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); var screen = new Mock(); this.conductor.ActivateItem(screen.Object); - ((IDeactivate)this.conductor).Deactivate(); + ((IScreenState)this.conductor).Deactivate(); screen.Verify(x => x.Deactivate()); } @@ -89,7 +89,7 @@ namespace StyletUnitTests { var screen1 = new Mock(); var screen2 = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen1.Object); screen1.Setup(x => x.CanCloseAsync()).Returns(Task.FromResult(true)); this.conductor.ActivateItem(screen2.Object); @@ -100,7 +100,7 @@ namespace StyletUnitTests public void ActivatingCurrentScreenReactivatesScreen() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); this.conductor.ActivateItem(screen.Object); screen.Verify(x => x.Activate(), Times.Exactly(2)); @@ -113,7 +113,7 @@ namespace StyletUnitTests { var screen1 = new Mock(); var screen2 = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen1.Object); this.conductor.CloseItem(screen2.Object); @@ -126,7 +126,7 @@ namespace StyletUnitTests public void DeactiveDoesNotChangeActiveItem() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); this.conductor.DeactivateItem(screen.Object); @@ -146,7 +146,7 @@ namespace StyletUnitTests public void SettingActiveItemActivatesItem() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActiveItem =screen.Object; screen.Verify(x => x.Activate()); Assert.AreEqual(this.conductor.ActiveItem, screen.Object); @@ -206,7 +206,7 @@ namespace StyletUnitTests [Test] public void DeactivatingActiveItemGoesBack() { - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); var screen1 = new Mock(); var screen2 = new Mock(); @@ -224,7 +224,7 @@ namespace StyletUnitTests [Test] public void ClearClosesAllItemsExceptCurrent() { - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); var screen1 = new Mock(); var screen2 = new Mock(); this.conductor.ActivateItem(screen1.Object); @@ -248,7 +248,7 @@ namespace StyletUnitTests this.conductor.ActivateItem(screen1.Object); this.conductor.ActivateItem(screen2.Object); - ((IClose)this.conductor).Close(); + ((IScreenState)this.conductor).Close(); screen1.Verify(x => x.Close()); screen1.VerifySet(x => x.Parent = null); screen2.Verify(x => x.Close()); diff --git a/StyletUnitTests/ConductorOneActiveTests.cs b/StyletUnitTests/ConductorOneActiveTests.cs index 630310d..dc45523 100644 --- a/StyletUnitTests/ConductorOneActiveTests.cs +++ b/StyletUnitTests/ConductorOneActiveTests.cs @@ -33,7 +33,7 @@ namespace StyletUnitTests public void ActivatingItemActivatesAndAddsToItems() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); screen.Verify(x => x.Activate()); @@ -46,7 +46,7 @@ namespace StyletUnitTests { var screen1 = new Mock(); var screen2 = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen1.Object); this.conductor.ActivateItem(screen2.Object); @@ -62,7 +62,7 @@ namespace StyletUnitTests public void SettingActiveItemActivatesItem() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActiveItem = screen.Object; screen.Verify(x => x.Activate()); Assert.AreEqual(this.conductor.ActiveItem, screen.Object); @@ -74,7 +74,7 @@ namespace StyletUnitTests var screen1 = new Mock(); var screen2 = new Mock(); var screen3 = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.Items.AddRange(new[] { screen1.Object, screen2.Object, screen3.Object }); this.conductor.ActivateItem(screen2.Object); @@ -92,7 +92,7 @@ namespace StyletUnitTests var screen1 = new Mock(); var screen2 = new Mock(); var screen3 = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.Items.AddRange(new[] { screen1.Object, screen2.Object, screen3.Object }); this.conductor.ActivateItem(screen3.Object); @@ -116,7 +116,7 @@ namespace StyletUnitTests public void ActivateItemDoesActiveIfConductorIsActive() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); screen.Verify(x => x.Activate()); } @@ -125,9 +125,9 @@ namespace StyletUnitTests public void DeactiveDeactivatesItems() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); - ((IDeactivate)this.conductor).Deactivate(); + ((IScreenState)this.conductor).Deactivate(); screen.Verify(x => x.Deactivate()); } @@ -135,9 +135,9 @@ namespace StyletUnitTests public void ClosingClosesAllItems() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); - ((IClose)this.conductor).Close(); + ((IScreenState)this.conductor).Close(); screen.Verify(x => x.Close()); screen.Verify(x => x.Dispose()); Assert.AreEqual(0, this.conductor.Items.Count); @@ -182,7 +182,7 @@ namespace StyletUnitTests { var screen1 = new Mock(); var screen2 = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen1.Object); // This is an implementation detail @@ -229,7 +229,7 @@ namespace StyletUnitTests [Test] public void RemovingActiveItemActivatesAnotherItem() { - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); var screen1 = new Mock(); var screen2 = new Mock(); this.conductor.ActivateItem(screen1.Object); @@ -310,7 +310,7 @@ namespace StyletUnitTests var screen1 = new Mock(); screen1.SetupGet(x => x.Parent).Returns(this.conductor); this.conductor.ActivateItem(screen1.Object); - ((IClose)this.conductor).Close(); + ((IScreenState)this.conductor).Close(); screen1.Verify(x => x.Close()); screen1.Verify(x => x.Dispose()); screen1.VerifySet(x => x.Parent = null); diff --git a/StyletUnitTests/ConductorTests.cs b/StyletUnitTests/ConductorTests.cs index a1deace..18e7c53 100644 --- a/StyletUnitTests/ConductorTests.cs +++ b/StyletUnitTests/ConductorTests.cs @@ -49,7 +49,7 @@ namespace StyletUnitTests [Test] public void InitialActivateActivatesItemIfConductorIsActive() { - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); var screen = new Mock(); this.conductor.ActivateItem(screen.Object); screen.Verify(x => x.Activate()); @@ -62,17 +62,17 @@ namespace StyletUnitTests this.conductor.ActivateItem(screen.Object); screen.Verify(x => x.Activate(), Times.Never); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); screen.Verify(x => x.Activate()); } [Test] public void DeactivatesActiveItemWhenDeactivated() { - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); var screen = new Mock(); this.conductor.ActivateItem(screen.Object); - ((IDeactivate)this.conductor).Deactivate(); + ((IScreenState)this.conductor).Deactivate(); screen.Verify(x => x.Deactivate()); } @@ -81,7 +81,7 @@ namespace StyletUnitTests { var screen1 = new Mock(); var screen2 = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen1.Object); screen1.Setup(x => x.CanCloseAsync()).Returns(Task.FromResult(true)); this.conductor.ActivateItem(screen2.Object); @@ -94,7 +94,7 @@ namespace StyletUnitTests { var screen1 = new Mock(); var screen2 = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen1.Object); screen1.Setup(x => x.CanCloseAsync()).Returns(Task.FromResult(false)); this.conductor.ActivateItem(screen2.Object); @@ -108,7 +108,7 @@ namespace StyletUnitTests public void ActivatingCurrentScreenReactivatesScreen() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); this.conductor.ActivateItem(screen.Object); screen.Verify(x => x.Activate(), Times.Exactly(2)); @@ -120,7 +120,7 @@ namespace StyletUnitTests public void SettingActiveItemActivatesItem() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActiveItem = screen.Object; screen.Verify(x => x.Activate()); Assert.AreEqual(this.conductor.ActiveItem, screen.Object); @@ -131,7 +131,7 @@ namespace StyletUnitTests { var screen1 = new Mock(); var screen2 = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen1.Object); this.conductor.CloseItem(screen2.Object); @@ -144,7 +144,7 @@ namespace StyletUnitTests public void DeactiveDoesNotChangeActiveItem() { var screen = new Mock(); - ((IActivate)this.conductor).Activate(); + ((IScreenState)this.conductor).Activate(); this.conductor.ActivateItem(screen.Object); this.conductor.DeactivateItem(screen.Object); @@ -192,7 +192,7 @@ namespace StyletUnitTests var screen1 = new Mock(); screen1.SetupGet(x => x.Parent).Returns(this.conductor); this.conductor.ActivateItem(screen1.Object); - ((IClose)this.conductor).Close(); + ((IScreenState)this.conductor).Close(); screen1.Verify(x => x.Close()); screen1.VerifySet(x => x.Parent = null); } @@ -202,7 +202,7 @@ namespace StyletUnitTests { var screen = new Mock(); this.conductor.ActivateItem(screen.Object); - ((IClose)this.conductor).Close(); + ((IScreenState)this.conductor).Close(); screen.Verify(x => x.Dispose()); } @@ -212,7 +212,7 @@ namespace StyletUnitTests this.conductor.DisposeChildren = false; var screen = new Mock(); this.conductor.ActivateItem(screen.Object); - ((IClose)this.conductor).Close(); + ((IScreenState)this.conductor).Close(); screen.Verify(x => x.Dispose(), Times.Never); } diff --git a/StyletUnitTests/ScreenExtensionTests.cs b/StyletUnitTests/ScreenExtensionTests.cs index bfaa9ac..381c050 100644 --- a/StyletUnitTests/ScreenExtensionTests.cs +++ b/StyletUnitTests/ScreenExtensionTests.cs @@ -26,39 +26,39 @@ namespace StyletUnitTests } [Test] - public void TryActivateActivatesIActivate() + public void TryActivateActivatesIScreenState() { - var screen = new Mock(); + var screen = new Mock(); ScreenExtensions.TryActivate(screen.Object); screen.Verify(x => x.Activate()); } [Test] - public void TryActivateDoesNothingToNonIActivate() + public void TryActivateDoesNothingToNonIScreenState() { - var screen = new Mock(MockBehavior.Strict); + var screen = new Mock(MockBehavior.Strict); ScreenExtensions.TryActivate(screen.Object); } [Test] - public void TryDeactivateDeactivatesIDeactivate() + public void TryDeactivateDeactivatesIScreenState() { - var screen = new Mock(); + var screen = new Mock(); ScreenExtensions.TryDeactivate(screen.Object); screen.Verify(x => x.Deactivate()); } [Test] - public void TryDeactivateDoesNothingToNonIDeactivate() + public void TryDeactivateDoesNothingToNonIScreenState() { - var screen = new Mock(MockBehavior.Strict); + var screen = new Mock(MockBehavior.Strict); ScreenExtensions.TryDeactivate(screen.Object); } [Test] - public void TryCloseClosesIClose() + public void TryCloseClosesIScreenState() { - var screen = new Mock(); + var screen = new Mock(); ScreenExtensions.TryClose(screen.Object); screen.Verify(x => x.Close()); } @@ -72,9 +72,9 @@ namespace StyletUnitTests } [Test] - public void TryCloseDoesNothingToNonIClose() + public void TryCloseDoesNothingToNonIScreenState() { - var screen = new Mock(MockBehavior.Strict); + var screen = new Mock(MockBehavior.Strict); ScreenExtensions.TryClose(screen.Object); } @@ -82,7 +82,7 @@ namespace StyletUnitTests public void ConductWithActivates() { this.child.Object.ConductWith(this.parent); - ((IActivate)this.parent).Activate(); + ((IScreenState)this.parent).Activate(); this.child.Verify(x => x.Activate()); } @@ -90,9 +90,9 @@ namespace StyletUnitTests public void ConductWithDeactivates() { // Needs to be active.... - ((IActivate)this.parent).Activate(); + ((IScreenState)this.parent).Activate(); this.child.Object.ConductWith(this.parent); - ((IDeactivate)this.parent).Deactivate(); + ((IScreenState)this.parent).Deactivate(); this.child.Verify(x => x.Deactivate()); } @@ -100,7 +100,7 @@ namespace StyletUnitTests public void ConductWithCloses() { this.child.Object.ConductWith(this.parent); - ((IClose)this.parent).Close(); + ((IScreenState)this.parent).Close(); this.child.Verify(x => x.Close()); } diff --git a/StyletUnitTests/ScreenTests.cs b/StyletUnitTests/ScreenTests.cs index 11994da..421c63c 100644 --- a/StyletUnitTests/ScreenTests.cs +++ b/StyletUnitTests/ScreenTests.cs @@ -26,6 +26,11 @@ namespace StyletUnitTests public MyScreen() { } public MyScreen(IModelValidator validator) : base(validator) { } + public new void SetState(ScreenState newState, Action changedHandler) + { + base.SetState(newState, changedHandler); + } + public bool OnActivateCalled; protected override void OnActivate() { @@ -92,7 +97,7 @@ namespace StyletUnitTests [Test] public void ActivateActivatesIfNotAlreadyActive() { - ((IActivate)this.screen).Activate(); + ((IScreenState)this.screen).Activate(); Assert.IsTrue(this.screen.IsActive); } @@ -101,41 +106,81 @@ namespace StyletUnitTests { bool fired = false; this.screen.Activated += (o, e) => fired = true; - ((IActivate)this.screen).Activate(); + ((IScreenState)this.screen).Activate(); Assert.IsTrue(fired); } [Test] public void ActivateCallsOnActivate() { - ((IActivate)this.screen).Activate(); + ((IScreenState)this.screen).Activate(); Assert.IsTrue(this.screen.OnActivateCalled); } [Test] public void DoubleActivationDoesntActivate() { - ((IActivate)this.screen).Activate(); + ((IScreenState)this.screen).Activate(); this.screen.OnActivateCalled = false; - ((IActivate)this.screen).Activate(); + ((IScreenState)this.screen).Activate(); Assert.IsFalse(this.screen.OnActivateCalled); } [Test] public void InitialActivationCallsOnInitialActivate() { - ((IActivate)this.screen).Activate(); + ((IScreenState)this.screen).Activate(); this.screen.OnInitialActivateCalled = false; - ((IDeactivate)this.screen).Deactivate(); - ((IActivate)this.screen).Activate(); + ((IScreenState)this.screen).Deactivate(); + ((IScreenState)this.screen).Activate(); Assert.IsFalse(this.screen.OnInitialActivateCalled); } + [Test] + public void ActivateFiresCorrectEvents() + { + this.screen.SetState(ScreenState.Deactivated, (n, o) => { }); + + var changedEventArgs = new List(); + this.screen.StateChanged += (o, e) => changedEventArgs.Add(e); + var activatedEventArgs = new List(); + this.screen.Activated += (o, e) => activatedEventArgs.Add(e); + + ((IScreenState)this.screen).Activate(); + + Assert.AreEqual(1, changedEventArgs.Count); + Assert.AreEqual(ScreenState.Active, changedEventArgs[0].NewState); + Assert.AreEqual(ScreenState.Deactivated, changedEventArgs[0].PreviousState); + + Assert.AreEqual(1, activatedEventArgs.Count); + Assert.AreEqual(ScreenState.Deactivated, activatedEventArgs[0].PreviousState); + Assert.IsFalse(activatedEventArgs[0].IsInitialActivate); + } + + [Test] + public void InitialActivateFiresCorrectEvents() + { + var changedEventArgs = new List(); + this.screen.StateChanged += (o, e) => changedEventArgs.Add(e); + var activatedEventArgs = new List(); + this.screen.Activated += (o, e) => activatedEventArgs.Add(e); + + ((IScreenState)this.screen).Activate(); + + Assert.AreEqual(1, changedEventArgs.Count); + Assert.AreEqual(ScreenState.Active, changedEventArgs[0].NewState); + Assert.AreEqual(ScreenState.Initial, changedEventArgs[0].PreviousState); + + Assert.AreEqual(1, activatedEventArgs.Count); + Assert.AreEqual(ScreenState.Initial, activatedEventArgs[0].PreviousState); + Assert.IsTrue(activatedEventArgs[0].IsInitialActivate); + } + [Test] public void DeactivateDeactivates() { - ((IActivate)this.screen).Activate(); ; - ((IDeactivate)this.screen).Deactivate(); + ((IScreenState)this.screen).Activate(); ; + ((IScreenState)this.screen).Deactivate(); Assert.IsFalse(this.screen.IsActive); } @@ -144,34 +189,54 @@ namespace StyletUnitTests { bool fired = false; this.screen.Deactivated += (o, e) => fired = true; - ((IActivate)this.screen).Activate(); ; - ((IDeactivate)this.screen).Deactivate(); + ((IScreenState)this.screen).Activate(); ; + ((IScreenState)this.screen).Deactivate(); Assert.IsTrue(fired); } [Test] public void DeactivateCallsOnDeactivate() { - ((IActivate)this.screen).Activate(); - ((IDeactivate)this.screen).Deactivate(); + ((IScreenState)this.screen).Activate(); + ((IScreenState)this.screen).Deactivate(); Assert.IsTrue(this.screen.OnDeactivateCalled); } [Test] public void DoubleDeactivationDoesntDeactivate() { - ((IActivate)this.screen).Activate(); - ((IDeactivate)this.screen).Deactivate(); + ((IScreenState)this.screen).Activate(); + ((IScreenState)this.screen).Deactivate(); this.screen.OnDeactivateCalled = false; - ((IDeactivate)this.screen).Deactivate(); + ((IScreenState)this.screen).Deactivate(); Assert.IsFalse(this.screen.OnDeactivateCalled); } + [Test] + public void DeactivateFiresCorrectEvents() + { + this.screen.SetState(ScreenState.Active, (n, o) => { }); + + var changedEventArgs = new List(); + this.screen.StateChanged += (o, e) => changedEventArgs.Add(e); + var deactivationEventArgs = new List(); + this.screen.Deactivated += (o, e) => deactivationEventArgs.Add(e); + + ((IScreenState)this.screen).Deactivate(); + + Assert.AreEqual(1, changedEventArgs.Count); + Assert.AreEqual(ScreenState.Deactivated, changedEventArgs[0].NewState); + Assert.AreEqual(ScreenState.Active, changedEventArgs[0].PreviousState); + + Assert.AreEqual(1, deactivationEventArgs.Count); + Assert.AreEqual(ScreenState.Active, deactivationEventArgs[0].PreviousState); + } + [Test] public void CloseDeactivates() { - ((IActivate)this.screen).Activate(); - ((IClose)this.screen).Close(); + ((IScreenState)this.screen).Activate(); + ((IScreenState)this.screen).Close(); Assert.IsTrue(this.screen.OnDeactivateCalled); } @@ -179,7 +244,7 @@ namespace StyletUnitTests public void CloseClearsView() { ((IViewAware)this.screen).AttachView(new UIElement()); - ((IClose)this.screen).Close(); + ((IScreenState)this.screen).Close(); Assert.IsNull(this.screen.View); } @@ -188,33 +253,53 @@ namespace StyletUnitTests { bool fired = false; this.screen.Closed += (o, e) => fired = true; - ((IClose)this.screen).Close(); + ((IScreenState)this.screen).Close(); Assert.IsTrue(fired); } [Test] public void CloseCallsOnClose() { - ((IClose)this.screen).Close(); + ((IScreenState)this.screen).Close(); Assert.IsTrue(this.screen.OnCloseCalled); } [Test] public void DoubleCloseDoesNotClose() { - ((IClose)this.screen).Close(); + ((IScreenState)this.screen).Close(); this.screen.OnCloseCalled = false; - ((IClose)this.screen).Close(); + ((IScreenState)this.screen).Close(); Assert.IsFalse(this.screen.OnCloseCalled); } + [Test] + public void CloseFiresCorrectEvents() + { + this.screen.SetState(ScreenState.Deactivated, (n, o) => { }); + + var changedEventArgs = new List(); + this.screen.StateChanged += (o, e) => changedEventArgs.Add(e); + var closeEventArgs = new List(); + this.screen.Closed += (o, e) => closeEventArgs.Add(e); + + ((IScreenState)this.screen).Close(); + + Assert.AreEqual(1, changedEventArgs.Count); + Assert.AreEqual(ScreenState.Closed, changedEventArgs[0].NewState); + Assert.AreEqual(ScreenState.Deactivated, changedEventArgs[0].PreviousState); + + Assert.AreEqual(1, closeEventArgs.Count); + Assert.AreEqual(ScreenState.Deactivated, closeEventArgs[0].PreviousState); + } + [Test] public void ActivatingAllowsScreenToBeClosedAgain() { - ((IClose)this.screen).Close(); + ((IScreenState)this.screen).Close(); this.screen.OnCloseCalled = false; - ((IActivate)this.screen).Activate(); - ((IClose)this.screen).Close(); + ((IScreenState)this.screen).Activate(); + ((IScreenState)this.screen).Close(); Assert.IsTrue(this.screen.OnCloseCalled); } From 64e83c526bf124275291d1c7a26b87543f72bb29 Mon Sep 17 00:00:00 2001 From: Antony Male Date: Tue, 20 Jan 2015 10:18:25 +0000 Subject: [PATCH 07/14] Set WindowStartupLocation appropriately if the user hasn't set it themselves --- Stylet/WindowManager.cs | 6 ++++ StyletUnitTests/WindowManagerTests.cs | 51 +++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/Stylet/WindowManager.cs b/Stylet/WindowManager.cs index 808cb27..321f5b6 100644 --- a/Stylet/WindowManager.cs +++ b/Stylet/WindowManager.cs @@ -153,6 +153,7 @@ namespace Stylet logger.Error(e, "This can occur when the application is closing down"); } } + logger.Info("Displaying ViewModel {0} with View {1} as a Dialog", viewModel, window); } else @@ -160,6 +161,11 @@ namespace Stylet logger.Info("Displaying ViewModel {0} with View {1} as a Window", viewModel, window); } + // If and only if they haven't tried to position the window themselves... + // Has to be done after we're attempted to set the owner + if (window.WindowStartupLocation == WindowStartupLocation.Manual && Double.IsNaN(window.Top) && Double.IsNaN(window.Left)) + window.WindowStartupLocation = window.Owner == null ? WindowStartupLocation.CenterScreen : WindowStartupLocation.CenterOwner; + // This gets itself retained by the window, by registering events // ReSharper disable once ObjectCreationAsStatement new WindowConductor(window, viewModel); diff --git a/StyletUnitTests/WindowManagerTests.cs b/StyletUnitTests/WindowManagerTests.cs index 91d149b..e8987e0 100644 --- a/StyletUnitTests/WindowManagerTests.cs +++ b/StyletUnitTests/WindowManagerTests.cs @@ -283,5 +283,56 @@ namespace StyletUnitTests this.messageBoxViewModel.Verify(x => x.Setup("text", "title", MessageBoxButton.OKCancel, MessageBoxImage.Error, MessageBoxResult.OK, MessageBoxResult.Cancel, MessageBoxOptions.RtlReading, null)); } + + [Test] + public void SetsWindowStartupLocationToCenterScreenIfThereIsNoOwnerAndItHasNotBeenSetAlready() + { + var model = new object(); + var window = new Window(); + this.viewManager.Setup(x => x.CreateAndBindViewForModel(model)).Returns(window); + + this.windowManager.CreateWindow(model, false); + + Assert.AreEqual(WindowStartupLocation.CenterScreen, window.WindowStartupLocation); + } + + [Test] + public void DoesNotSetStartupLocationIfItIsNotManual() + { + var model = new object(); + var window = new Window(); + window.WindowStartupLocation = WindowStartupLocation.CenterOwner; + this.viewManager.Setup(x => x.CreateAndBindViewForModel(model)).Returns(window); + + this.windowManager.CreateWindow(model, false); + + Assert.AreEqual(WindowStartupLocation.CenterOwner, window.WindowStartupLocation); + } + + [Test] + public void DoesNotSetStartupLocationIfLeftSet() + { + var model = new object(); + var window = new Window(); + window.Left = 1; + this.viewManager.Setup(x => x.CreateAndBindViewForModel(model)).Returns(window); + + this.windowManager.CreateWindow(model, false); + + Assert.AreEqual(WindowStartupLocation.Manual, window.WindowStartupLocation); + } + + [Test] + public void DoesNotSetStartupLocationIfTopSet() + { + var model = new object(); + var window = new Window(); + window.Top = 1; + this.viewManager.Setup(x => x.CreateAndBindViewForModel(model)).Returns(window); + + this.windowManager.CreateWindow(model, false); + + Assert.AreEqual(WindowStartupLocation.Manual, window.WindowStartupLocation); + } } } From 68a826e66ffda31486428f6e91a514999b979a5e Mon Sep 17 00:00:00 2001 From: Antony Male Date: Tue, 20 Jan 2015 10:24:12 +0000 Subject: [PATCH 08/14] WindowManager does not set Title binding if Title has a value already --- Stylet/WindowManager.cs | 2 +- StyletUnitTests/WindowManagerTests.cs | 39 ++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/Stylet/WindowManager.cs b/Stylet/WindowManager.cs index 321f5b6..a3260a6 100644 --- a/Stylet/WindowManager.cs +++ b/Stylet/WindowManager.cs @@ -130,7 +130,7 @@ namespace Stylet } var haveDisplayName = viewModel as IHaveDisplayName; - if (haveDisplayName != null && BindingOperations.GetBindingBase(window, Window.TitleProperty) == null) + if (haveDisplayName != null && String.IsNullOrEmpty(window.Title) && BindingOperations.GetBindingBase(window, Window.TitleProperty) == null) { var binding = new Binding("DisplayName") { Mode = BindingMode.TwoWay }; window.SetBinding(Window.TitleProperty, binding); diff --git a/StyletUnitTests/WindowManagerTests.cs b/StyletUnitTests/WindowManagerTests.cs index e8987e0..426f286 100644 --- a/StyletUnitTests/WindowManagerTests.cs +++ b/StyletUnitTests/WindowManagerTests.cs @@ -101,10 +101,41 @@ namespace StyletUnitTests this.windowManager.CreateWindow(model, false); var e = window.GetBindingExpression(Window.TitleProperty); + Assert.NotNull(e); Assert.AreEqual(BindingMode.TwoWay, e.ParentBinding.Mode); Assert.AreEqual("DisplayName", e.ParentBinding.Path.Path); } + [Test] + public void CreateWindowDoesNotSetUpTitleBindingIfTitleHasAValueAlready() + { + var model = new Screen(); + var window = new Window(); + window.Title = "Foo"; + this.viewManager.Setup(x => x.CreateAndBindViewForModel(model)).Returns(window); + + this.windowManager.CreateWindow(model, false); + + var e = window.GetBindingExpression(Window.TitleProperty); + Assert.IsNull(e); + Assert.AreEqual("Foo", window.Title); + } + + [Test] + public void CreateWindowDoesNotSetUpTitleBindingIfTitleHasABindingAlready() + { + var model = new Screen(); + var window = new Window(); + var binding = new Binding("Test") { Mode = BindingMode.TwoWay }; + window.SetBinding(Window.TitleProperty, binding); + this.viewManager.Setup(x => x.CreateAndBindViewForModel(model)).Returns(window); + + this.windowManager.CreateWindow(model, false); + + var e = window.GetBindingExpression(Window.TitleProperty); + Assert.AreEqual("Test", e.ParentBinding.Path.Path); + } + [Test] public void CreateWindowActivatesViewModel() { @@ -285,7 +316,7 @@ namespace StyletUnitTests } [Test] - public void SetsWindowStartupLocationToCenterScreenIfThereIsNoOwnerAndItHasNotBeenSetAlready() + public void CreateWindowSetsWindowStartupLocationToCenterScreenIfThereIsNoOwnerAndItHasNotBeenSetAlready() { var model = new object(); var window = new Window(); @@ -297,7 +328,7 @@ namespace StyletUnitTests } [Test] - public void DoesNotSetStartupLocationIfItIsNotManual() + public void CreateWindowDoesNotSetStartupLocationIfItIsNotManual() { var model = new object(); var window = new Window(); @@ -310,7 +341,7 @@ namespace StyletUnitTests } [Test] - public void DoesNotSetStartupLocationIfLeftSet() + public void CreateWindowDoesNotSetStartupLocationIfLeftSet() { var model = new object(); var window = new Window(); @@ -323,7 +354,7 @@ namespace StyletUnitTests } [Test] - public void DoesNotSetStartupLocationIfTopSet() + public void CreateWindowDoesNotSetStartupLocationIfTopSet() { var model = new object(); var window = new Window(); From 91b925a4581ec259c2af9fd17dc997e0df3e5fd5 Mon Sep 17 00:00:00 2001 From: Antony Male Date: Sat, 7 Feb 2015 13:02:21 +0000 Subject: [PATCH 09/14] Add AddModule and AddModules to IStyletIoCBuilder (oops) --- Stylet/StyletIoC/StyletIoCBuilder.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Stylet/StyletIoC/StyletIoCBuilder.cs b/Stylet/StyletIoC/StyletIoCBuilder.cs index 7c28910..d919e34 100644 --- a/Stylet/StyletIoC/StyletIoCBuilder.cs +++ b/Stylet/StyletIoC/StyletIoCBuilder.cs @@ -173,6 +173,18 @@ namespace StyletIoC /// Assembly(s) to search, or leave empty / null to search the current assembly void Autobind(params Assembly[] assemblies); + /// + /// Add a single module to this builder + /// + /// Module to add + void AddModule(StyletIoCModule module); + + /// + /// Add many modules to this builder + /// + /// Modules to add + void AddModules(params StyletIoCModule[] modules); + /// /// Once all bindings have been set, build an IContainer from which instances can be fetches /// From 02175b6e67680fbb5b8686b83d8254646e68bd7e Mon Sep 17 00:00:00 2001 From: Antony Male Date: Mon, 9 Feb 2015 12:16:25 +0000 Subject: [PATCH 10/14] Fix incorrect use of WeakEventManager in ScreenExtensions --- Stylet/ScreenExtensions.cs | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/Stylet/ScreenExtensions.cs b/Stylet/ScreenExtensions.cs index f6f2c56..da57e1a 100644 --- a/Stylet/ScreenExtensions.cs +++ b/Stylet/ScreenExtensions.cs @@ -60,7 +60,17 @@ namespace Stylet /// Parent to observe public static void ActivateWith(this IScreenState child, IScreenState parent) { - WeakEventManager.AddHandler(parent, "Activated", (o, e) => child.Activate()); + var weakChild = new WeakReference(child); + EventHandler handler = null; + handler = (o, e) => + { + IScreenState strongChild; + if (weakChild.TryGetTarget(out strongChild)) + strongChild.Activate(); + else + parent.Activated -= handler; + }; + parent.Activated += handler; } /// @@ -71,7 +81,17 @@ namespace Stylet /// Parent to observe public static void DeactivateWith(this IScreenState child, IScreenState parent) { - WeakEventManager.AddHandler(parent, "Deactivated", (o, e) => child.Deactivate()); + var weakChild = new WeakReference(child); + EventHandler handler = null; + handler = (o, e) => + { + IScreenState strongChild; + if (weakChild.TryGetTarget(out strongChild)) + strongChild.Deactivate(); + else + parent.Deactivated -= handler; + }; + parent.Deactivated += handler; } /// @@ -82,8 +102,17 @@ namespace Stylet /// Parent to observe public static void CloseWith(this IScreenState child, IScreenState parent) { - // Using TryCloseAndDispose ensures that Dispose is called if necessary - WeakEventManager.AddHandler(parent, "Closed", (o, e) => TryClose(child)); + var weakChild = new WeakReference(child); + EventHandler handler = null; + handler = (o, e) => + { + IScreenState strongChild; + if (weakChild.TryGetTarget(out strongChild)) + TryClose(strongChild); + else + parent.Closed -= handler; + }; + parent.Closed += handler; } /// From 386b7cb0354fb9b55c5a5992c2719b1c35568e15 Mon Sep 17 00:00:00 2001 From: Antony Male Date: Mon, 9 Feb 2015 12:39:21 +0000 Subject: [PATCH 11/14] Bring more sanity to the Bootstrapper --- Bootstrappers/AutofacBootstrapper.cs | 2 +- Bootstrappers/CastleWindsorBootstrapper.cs | 2 +- Bootstrappers/NinjectBootstrapper.cs | 2 +- Bootstrappers/StructureMapBootstrapper.cs | 2 +- Bootstrappers/UnityBootstrapper.cs | 2 +- Stylet/Bootstrapper.cs | 13 ++++------- Stylet/BootstrapperBase.cs | 27 +++++++++++++--------- Stylet/ViewManager.cs | 4 ++-- StyletIntegrationTests/Bootstrapper.cs | 4 ++-- StyletUnitTests/BootstrapperBaseTests.cs | 8 +++---- StyletUnitTests/BootstrapperTests.cs | 9 ++------ 11 files changed, 36 insertions(+), 39 deletions(-) diff --git a/Bootstrappers/AutofacBootstrapper.cs b/Bootstrappers/AutofacBootstrapper.cs index ce551f4..50bec33 100644 --- a/Bootstrappers/AutofacBootstrapper.cs +++ b/Bootstrappers/AutofacBootstrapper.cs @@ -54,7 +54,7 @@ namespace Bootstrappers return this.container.Resolve(type); } - protected override void OnExitInternal(ExitEventArgs e) + public override void Dispose() { this.container.Dispose(); } diff --git a/Bootstrappers/CastleWindsorBootstrapper.cs b/Bootstrappers/CastleWindsorBootstrapper.cs index c8d21e9..2818d13 100644 --- a/Bootstrappers/CastleWindsorBootstrapper.cs +++ b/Bootstrappers/CastleWindsorBootstrapper.cs @@ -60,7 +60,7 @@ namespace Bootstrappers return this.container.Resolve(type); } - protected override void OnExitInternal(ExitEventArgs e) + public override void Dispose() { this.container.Dispose(); } diff --git a/Bootstrappers/NinjectBootstrapper.cs b/Bootstrappers/NinjectBootstrapper.cs index b23fdcd..2b45448 100644 --- a/Bootstrappers/NinjectBootstrapper.cs +++ b/Bootstrappers/NinjectBootstrapper.cs @@ -51,7 +51,7 @@ namespace Bootstrappers return this.kernel.Get(type); } - protected override void OnExitInternal(ExitEventArgs e) + public override void Dispose() { this.kernel.Dispose(); } diff --git a/Bootstrappers/StructureMapBootstrapper.cs b/Bootstrappers/StructureMapBootstrapper.cs index 81a1044..f5a9399 100644 --- a/Bootstrappers/StructureMapBootstrapper.cs +++ b/Bootstrappers/StructureMapBootstrapper.cs @@ -58,7 +58,7 @@ namespace Bootstrappers return this.container.GetInstance(type); } - protected override void OnExitInternal(ExitEventArgs e) + public override void Dispose() { this.container.Dispose(); } diff --git a/Bootstrappers/UnityBootstrapper.cs b/Bootstrappers/UnityBootstrapper.cs index da503ac..8ff9140 100644 --- a/Bootstrappers/UnityBootstrapper.cs +++ b/Bootstrappers/UnityBootstrapper.cs @@ -54,7 +54,7 @@ namespace Bootstrappers return this.container.Resolve(type); } - protected override void OnExitInternal(ExitEventArgs e) + public override void Dispose() { this.container.Dispose(); } diff --git a/Stylet/Bootstrapper.cs b/Stylet/Bootstrapper.cs index 761431f..d26ce49 100644 --- a/Stylet/Bootstrapper.cs +++ b/Stylet/Bootstrapper.cs @@ -31,19 +31,18 @@ namespace Stylet /// protected override sealed void ConfigureBootstrapper() { - // This needs to be called before the container is set up, as it might affect the assemblies - this.Configure(); - var builder = new StyletIoCBuilder(); this.DefaultConfigureIoC(builder); this.ConfigureIoC(builder); this.Container = builder.BuildContainer(); + + this.Configure(); } /// - /// Override to configure your IoC container, and anything else + /// Hook called after the IoC container has been set up /// protected virtual void Configure() { } @@ -80,12 +79,10 @@ namespace Stylet } /// - /// Hook used internall by the Bootstrapper to do things like dispose the IoC container + /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. /// - /// The exit event data - protected override void OnExitInternal(ExitEventArgs e) + public override void Dispose() { - base.OnExitInternal(e); this.Container.Dispose(); } } diff --git a/Stylet/BootstrapperBase.cs b/Stylet/BootstrapperBase.cs index f5a677d..22fb170 100644 --- a/Stylet/BootstrapperBase.cs +++ b/Stylet/BootstrapperBase.cs @@ -12,7 +12,7 @@ namespace Stylet /// /// Bootstrapper to be extended by applications which don't want to use StyletIoC as the IoC container. /// - public abstract class BootstrapperBase : IBootstrapper, IViewManagerConfig + public abstract class BootstrapperBase : IBootstrapper, IViewManagerConfig, IDisposable { /// /// Gets the current application @@ -23,7 +23,7 @@ namespace Stylet /// Gets or sets assemblies which are used for IoC container auto-binding and searching for Views. /// Set this in Configure() if you want to override it /// - public IList Assemblies { get; protected set; } + public IReadOnlyList Assemblies { get; protected set; } /// /// Gets the command line arguments that were passed to the application from either the command prompt or the desktop. @@ -61,15 +61,15 @@ namespace Stylet // Make life nice for the app - they can handle these by overriding Bootstrapper methods, rather than adding event handlers this.Application.Exit += (o, e) => { - this.OnExitInternal(e); this.OnExit(e); + this.Dispose(); }; // Fetch this logger when needed. If we fetch it now, then no-one will have been given the change to enable the LogManager, and we'll get a NullLogger this.Application.DispatcherUnhandledException += (o, e) => { LogManager.GetLogger(typeof(BootstrapperBase)).Error(e.Exception, "Unhandled exception"); - this.OnUnhandledExecption(e); + this.OnUnhandledException(e); }; } @@ -81,13 +81,14 @@ namespace Stylet { // Set this before anything else, so everything can use it this.Args = args; + this.OnStart(); this.ConfigureBootstrapper(); View.ViewManager = (IViewManager)this.GetInstance(typeof(IViewManager)); this.Launch(); - this.OnStartup(); + this.OnLaunch(); } /// @@ -112,15 +113,14 @@ namespace Stylet public abstract object GetInstance(Type type); /// - /// Hook called on application startup. This occurs once the root view has been displayed + /// Called on application startup. This occur after this.Args has been assigned, but before the IoC container has been configured /// - protected virtual void OnStartup() { } + protected virtual void OnStart() { } /// - /// Hook used internally by the Bootstrapper to do things like dispose the IoC container + /// Called just after the root View has been displayed /// - /// The exit event data - protected virtual void OnExitInternal(ExitEventArgs e) { } + protected virtual void OnLaunch() { } /// /// Hook called on application exit @@ -132,6 +132,11 @@ namespace Stylet /// Hook called on an unhandled exception /// /// The event data - protected virtual void OnUnhandledExecption(DispatcherUnhandledExceptionEventArgs e) { } + protected virtual void OnUnhandledException(DispatcherUnhandledExceptionEventArgs e) { } + + /// + /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. + /// + public virtual void Dispose() { } } } diff --git a/Stylet/ViewManager.cs b/Stylet/ViewManager.cs index b0d2fa5..f6bc791 100644 --- a/Stylet/ViewManager.cs +++ b/Stylet/ViewManager.cs @@ -53,7 +53,7 @@ namespace Stylet /// Gets the assemblies which are used for IoC container auto-binding and searching for Views. /// Set this in Configure() if you want to override it /// - IList Assemblies { get; } + IReadOnlyList Assemblies { get; } /// /// Given a type, use the IoC container to fetch an instance of it @@ -73,7 +73,7 @@ namespace Stylet /// /// Gets or sets the assemblies searched for View types /// - protected IList Assemblies { get; set; } + protected IReadOnlyList Assemblies { get; set; } /// /// Gets or sets the factory used to create view instances from their type diff --git a/StyletIntegrationTests/Bootstrapper.cs b/StyletIntegrationTests/Bootstrapper.cs index 78a2393..11abe2e 100644 --- a/StyletIntegrationTests/Bootstrapper.cs +++ b/StyletIntegrationTests/Bootstrapper.cs @@ -17,9 +17,9 @@ namespace StyletIntegrationTests LogManager.Enabled = true; } - protected override void OnUnhandledExecption(System.Windows.Threading.DispatcherUnhandledExceptionEventArgs e) + protected override void OnUnhandledException(System.Windows.Threading.DispatcherUnhandledExceptionEventArgs e) { - base.OnUnhandledExecption(e); // Calling this just to trigger some code coverage + base.OnUnhandledException(e); // Calling this just to trigger some code coverage var message = e.Exception.Message; if (e.Exception is TargetInvocationException) message = e.Exception.InnerException.Message; diff --git a/StyletUnitTests/BootstrapperBaseTests.cs b/StyletUnitTests/BootstrapperBaseTests.cs index 643516e..10d8426 100644 --- a/StyletUnitTests/BootstrapperBaseTests.cs +++ b/StyletUnitTests/BootstrapperBaseTests.cs @@ -51,10 +51,10 @@ namespace StyletUnitTests return null; } - public bool OnStartupCalled; - protected override void OnStartup() + public bool OnStartCalled; + protected override void OnStart() { - this.OnStartupCalled = true; + this.OnStartCalled = true; } public bool OnExitCalled; @@ -147,7 +147,7 @@ namespace StyletUnitTests public void StartCallsOnStartup() { this.bootstrapper.Start(new string[0]); - Assert.True(this.bootstrapper.OnStartupCalled); + Assert.True(this.bootstrapper.OnStartCalled); } } } diff --git a/StyletUnitTests/BootstrapperTests.cs b/StyletUnitTests/BootstrapperTests.cs index db7d9f9..5555910 100644 --- a/StyletUnitTests/BootstrapperTests.cs +++ b/StyletUnitTests/BootstrapperTests.cs @@ -40,11 +40,6 @@ namespace StyletUnitTests builder.Bind().To(); base.ConfigureIoC(builder); } - - public new void OnExitInternal(ExitEventArgs e) - { - base.OnExitInternal(e); - } } private MyBootstrapper bootstrapper; @@ -92,11 +87,11 @@ namespace StyletUnitTests } [Test] - public void OnExitInternalDisposesContainer() + public void DisposeDisposesContainer() { var container = new Mock(); this.bootstrapper.Container = container.Object; - this.bootstrapper.OnExitInternal(null); + this.bootstrapper.Dispose(); container.Verify(x => x.Dispose()); } } From 0b088b04fd44383cbc78a089e973e354afb37022 Mon Sep 17 00:00:00 2001 From: Antony Male Date: Mon, 9 Feb 2015 12:48:18 +0000 Subject: [PATCH 12/14] Get coverage up slightly --- Stylet/Bootstrapper.cs | 1 + StyletUnitTests/ScreenExtensionTests.cs | 68 +++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/Stylet/Bootstrapper.cs b/Stylet/Bootstrapper.cs index d26ce49..7586c4d 100644 --- a/Stylet/Bootstrapper.cs +++ b/Stylet/Bootstrapper.cs @@ -84,6 +84,7 @@ namespace Stylet public override void Dispose() { this.Container.Dispose(); + base.Dispose(); } } } diff --git a/StyletUnitTests/ScreenExtensionTests.cs b/StyletUnitTests/ScreenExtensionTests.cs index 381c050..a0bd0a3 100644 --- a/StyletUnitTests/ScreenExtensionTests.cs +++ b/StyletUnitTests/ScreenExtensionTests.cs @@ -78,6 +78,28 @@ namespace StyletUnitTests ScreenExtensions.TryClose(screen.Object); } + [Test] + public void ActivateWithActivates() + { + this.child.Object.ActivateWith(this.parent); + ((IScreenState)this.parent).Activate(); + this.child.Verify(x => x.Activate()); + } + + [Test] + public void ActivateWithDoesNotRetainChild() + { + var child = new Screen(); + child.ActivateWith(this.parent); + + var weakChild = new WeakReference(child); + child = null; + GC.Collect(); + + ((IScreenState)this.parent).Activate(); + Assert.Null(weakChild.Target); + } + [Test] public void ConductWithActivates() { @@ -86,6 +108,30 @@ namespace StyletUnitTests this.child.Verify(x => x.Activate()); } + [Test] + public void DeactivateWithDeactivates() + { + // Needs to be active.... + ((IScreenState)this.parent).Activate(); + this.child.Object.DeactivateWith(this.parent); + ((IScreenState)this.parent).Deactivate(); + this.child.Verify(x => x.Deactivate()); + } + + [Test] + public void DeactivateDoesNotRetainChild() + { + var child = new Screen(); + child.DeactivateWith(this.parent); + + var weakChild = new WeakReference(child); + child = null; + GC.Collect(); + + ((IScreenState)this.parent).Deactivate(); + Assert.Null(weakChild.Target); + } + [Test] public void ConductWithDeactivates() { @@ -96,6 +142,28 @@ namespace StyletUnitTests this.child.Verify(x => x.Deactivate()); } + [Test] + public void CloseWithCloses() + { + this.child.Object.CloseWith(this.parent); + ((IScreenState)this.parent).Close(); + this.child.Verify(x => x.Close()); + } + + [Test] + public void CloseWithDoesNotRetain() + { + var child = new Screen(); + child.CloseWith(this.parent); + + var weakChild = new WeakReference(child); + child = null; + GC.Collect(); + + ((IScreenState)this.parent).Close(); + Assert.Null(weakChild.Target); + } + [Test] public void ConductWithCloses() { From b34a59f0fe7bf1ec741f044e268ac35e8c62c4bb Mon Sep 17 00:00:00 2001 From: Antony Male Date: Wed, 11 Feb 2015 12:50:49 +0000 Subject: [PATCH 13/14] Update changelog --- CHANGELOG.txt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 0ce6a97..debb8c6 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,6 +1,17 @@ Stylet Changelog ================ +v1.1.0 +------ + + - Backwards-incompatible changes to Bootstrapper: Configure is now called *after* ConfigureIoC, OnStart came before ConfigureIoC, and OnLaunch now happens after Launch + - Screen now uses a property enum-based state machine to manage state. IActivate, IDeactivate and IClose have all been rolled into IScreenState + - Fix incorrect use of WeakEventManager in ScreenExtensions: ConductWith won't have been working properly + - Set WindowStartupLocation to CenterOwner if the user hasn't set it themselves + - WindowManager does not set the Title binding (to DisplayName) if it's been set by the user + - Actions throw on execute if ActionTarget hasn't changed from the default. This catches an edge-case where Actions are used inside something like a ContextMenu which breaks the visual tree + - + v1.0.7 ------ From b1059cf7fff69f1a81050a217ab2037c7e5b715b Mon Sep 17 00:00:00 2001 From: Antony Male Date: Wed, 11 Feb 2015 12:51:26 +0000 Subject: [PATCH 14/14] Bump version --- NuGet/Stylet.nuspec | 2 +- Stylet/Properties/AssemblyInfo.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/NuGet/Stylet.nuspec b/NuGet/Stylet.nuspec index 36aae31..b0369d4 100644 --- a/NuGet/Stylet.nuspec +++ b/NuGet/Stylet.nuspec @@ -2,7 +2,7 @@ Stylet - 1.0.7 + 1.1.0 Stylet Antony Male Antony Male diff --git a/Stylet/Properties/AssemblyInfo.cs b/Stylet/Properties/AssemblyInfo.cs index e2b2012..265d1c0 100644 --- a/Stylet/Properties/AssemblyInfo.cs +++ b/Stylet/Properties/AssemblyInfo.cs @@ -35,5 +35,5 @@ using System.Windows.Markup; // You can specify all the values or you can default the Build and Revision Numbers // by using the '*' as shown below: // [assembly: AssemblyVersion("1.0.*")] -[assembly: AssemblyVersion("1.0.7.0")] -[assembly: AssemblyFileVersion("1.0.7.0")] +[assembly: AssemblyVersion("1.1.0.0")] +[assembly: AssemblyFileVersion("1.1.0.0")]