Set Dialog-Task-Result after IsOpen to prevent "DialogHost is already open." (#1750)

* set _dialogTaskCompletionSource after IsOpen with parameter via DialogSession

* Cleaning up DialogClosingEventArgs ctor

Added additional unit test for opening and closing the dialog host

Co-authored-by: Kevin Bost <[email protected]>

Author: greuelpirat

MaterialDesignThemes.Wpf.Tests/DialogHostTests.cs

@@ -63,6 +63,22 @@ public async Task CanOpenDialogWithShowMethodAndCloseWithIsOpen()
             Assert.False(_dialogHost.IsOpen);
         }
 
+        [StaFact]
+        public async Task CanCloseDialogWithRoutedEvent()
+        {
+            Guid closeParameter = Guid.NewGuid();
+            Task<object> showTask = _dialogHost.ShowDialog("Content");
+            DialogSession session = _dialogHost.CurrentSession;
+            Assert.False(session.IsEnded);
+            
+            DialogHost.CloseDialogCommand.Execute(closeParameter, _dialogHost);
+
+            Assert.False(_dialogHost.IsOpen);
+            Assert.Null(_dialogHost.CurrentSession);
+            Assert.True(session.IsEnded);
+            Assert.Equal(closeParameter, await showTask);
+        }
+
         [StaFact]
         public async Task DialogHostExposesSessionAsProperty()
         {
@@ -210,9 +226,34 @@ public void WhenDialogHostIsUnloadedIsOpenRemainsTrue()
             Assert.True(_dialogHost.IsOpen);
         }
 
-        private class TestDialog : Control
+        [StaFact]
+        [Description("Issue 1750")]
+        public async Task WhenSettingIsOpenToFalseItReturnsClosingParameterToShow()
+        {
+            Guid closeParameter = Guid.NewGuid();
+
+            Task<object> showTask = _dialogHost.ShowDialog("Content");
+            _dialogHost.CurrentSession.CloseParameter = closeParameter;
+
+            _dialogHost.IsOpen = false;
+
+            Assert.Equal(closeParameter, await showTask);
+        }
+
+        [StaFact]
+        [Description("Issue 1750")]
+        public async Task WhenClosingDialogReturnValueCanBeSpecifiedInClosingEventHandler()
         {
-            public void CloseDialog() => DialogHost.CloseDialogCommand.Execute(null, this);
+            Guid closeParameter = Guid.NewGuid();
+
+            Task<object> showTask = _dialogHost.ShowDialog("Content", (object sender, DialogClosingEventArgs args) =>
+            {
+                args.Session.CloseParameter = closeParameter;
+            });
+
+            DialogHost.CloseDialogCommand.Execute(null, _dialogHost);
+
+            Assert.Equal(closeParameter, await showTask);
         }
     }
 }

MaterialDesignThemes.Wpf/DialogClosingEventArgs.cs

@@ -5,11 +5,14 @@ namespace MaterialDesignThemes.Wpf
 {
     public class DialogClosingEventArgs : RoutedEventArgs
     {
-        public DialogClosingEventArgs(DialogSession session, object parameter, RoutedEvent routedEvent) : base(routedEvent)
+        [Obsolete("Use DialogClosingEventArgs(DialogSession, RoutedEvent), the parameter should be set on the DialogSession.CloseParameter")]
+        public DialogClosingEventArgs(DialogSession session, object parameter, RoutedEvent routedEvent) 
+            : this(session, routedEvent)
+        { }
+
+        public DialogClosingEventArgs(DialogSession session, RoutedEvent routedEvent) : base(routedEvent)
         {
             Session = session ?? throw new ArgumentNullException(nameof(session));
-
-            Parameter = parameter;
         }
 
         /// <summary>
@@ -28,7 +31,7 @@ public void Cancel()
         /// <summary>
         /// Gets the parameter originally provided to <see cref="DialogHost.CloseDialogCommand"/>/
         /// </summary>
-        public object Parameter { get; }
+        public object Parameter => Session.CloseParameter;
 
         /// <summary>
         /// Allows interaction with the current dialog session.

MaterialDesignThemes.Wpf/DialogHost.cs

@@ -75,7 +75,7 @@ static DialogHost()
             DefaultStyleKeyProperty.OverrideMetadata(typeof(DialogHost), new FrameworkPropertyMetadata(typeof(DialogHost)));
         }
 
-        #region .Show overloads
+        #region Show overloads
 
         /// <summary>
         /// Shows a modal dialog. To use, a <see cref="DialogHost"/> instance must be in a visual tree (typically this may be specified towards the root of a Window's XAML).
@@ -249,12 +249,12 @@ private static void IsOpenPropertyChangedCallback(DependencyObject dependencyObj
                     dialogHost._currentSnackbarMessageQueueUnPauseAction();
                     dialogHost._currentSnackbarMessageQueueUnPauseAction = null;
                 }
+
+                var closeParameter = dialogHost.CurrentSession.CloseParameter;
                 dialogHost.CurrentSession.IsEnded = true;
                 dialogHost.CurrentSession = null;
                 //NB: _dialogTaskCompletionSource is only set in the case where the dialog is shown with Show
-                //To get into this case you need to display the dialog with Show and then hide it by setting IsOpen to false
-                //Setting this here ensures the other 
-                dialogHost._dialogTaskCompletionSource?.TrySetResult(null);
+                dialogHost._dialogTaskCompletionSource?.TrySetResult(closeParameter);
 
                 // Don't attempt to Invoke if _restoreFocusDialogClose hasn't been assigned yet. Can occur
                 // if the MainWindow has started up minimized. Even when Show() has been called, this doesn't
@@ -578,8 +578,9 @@ internal void AssertTargetableContent()
 
         internal void Close(object parameter)
         {
-            var dialogClosingEventArgs = new DialogClosingEventArgs(CurrentSession, parameter, DialogClosingEvent);
+            var dialogClosingEventArgs = new DialogClosingEventArgs(CurrentSession, DialogClosingEvent);
 
+            CurrentSession.CloseParameter = parameter;
             CurrentSession.IsEnded = true;
 
             //multiple ways of calling back that the dialog is closing:
@@ -592,16 +593,13 @@ internal void Close(object parameter)
             DialogClosingCallback?.Invoke(this, dialogClosingEventArgs);
             _asyncShowClosingEventHandler?.Invoke(this, dialogClosingEventArgs);
 
-
-            if (!dialogClosingEventArgs.IsCancelled)
-            {
-                _dialogTaskCompletionSource?.TrySetResult(parameter);
-                SetCurrentValue(IsOpenProperty, false);
-            }
-            else
+            if (dialogClosingEventArgs.IsCancelled)
             {
                 CurrentSession.IsEnded = false;
+                return;
             }
+            
+            SetCurrentValue(IsOpenProperty, false);
         }
 
         /// <summary>

MaterialDesignThemes.Wpf/DialogSession.cs

@@ -22,6 +22,11 @@ internal DialogSession(DialogHost owner)
         /// Client code cannot set this directly, this is internally managed.  To end the dialog session use <see cref="Close()"/>.
         /// </remarks>
         public bool IsEnded { get; internal set; }
+        
+        /// <summary>
+        /// The parameter passed to the <see cref="DialogHost.CloseDialogCommand" /> and return by <see cref="DialogHost.Show(object)"/>
+        /// </summary>
+        internal object CloseParameter { get; set; }
 
         /// <summary>
         /// Gets the <see cref="DialogHost.DialogContent"/> which is currently displayed, so this could be a view model or a UI element.