Good or bad practice for Dialogs in wpf with MVVM?

I lately had the problem of creating add and edit dialogs for my wpf app.

All I want to do in my code was something like this. (I mostly use viewmodel first approach with mvvm)

ViewModel which calls a dialog window:

var result = this.uiDialogService.ShowDialog("Dialogwindow Title", dialogwindowVM);
// Do anything with the dialog result

How does it work?

First, I created a dialog service:

public interface IUIWindowDialogService
{
    bool? ShowDialog(string title, object datacontext);
}

public class WpfUIWindowDialogService : IUIWindowDialogService
{
    public bool? ShowDialog(string title, object datacontext)
    {
        var win = new WindowDialog();
        win.Title = title;
        win.DataContext = datacontext;

        return win.ShowDialog();
    }
}

WindowDialog is a special but simple window. I need it to hold my content:

<Window x:Class="WindowDialog"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation" 
    Title="WindowDialog" 
    WindowStyle="SingleBorderWindow" 
    WindowStartupLocation="CenterOwner" SizeToContent="WidthAndHeight">
    <ContentPresenter x:Name="DialogPresenter" Content="{Binding .}">
    
    </ContentPresenter>
</Window>

A problem with dialogs in wpf is the dialogresult = true can only be achieved in code. That's why I created an interface for my dialogviewmodel to implement it.

public class RequestCloseDialogEventArgs : EventArgs
{
    public bool DialogResult { get; set; }
    public RequestCloseDialogEventArgs(bool dialogresult)
    {
        this.DialogResult = dialogresult;
    }
}

public interface IDialogResultVMHelper
{
    event EventHandler<RequestCloseDialogEventArgs> RequestCloseDialog;
}

Whenever my ViewModel thinks it's time for dialogresult = true, then raise this event.

public partial class DialogWindow : Window
{
    // Note: If the window is closed, it has no DialogResult
    private bool _isClosed = false;

    public DialogWindow()
    {
        InitializeComponent();
        this.DialogPresenter.DataContextChanged += DialogPresenterDataContextChanged;
        this.Closed += DialogWindowClosed;
    }

    void DialogWindowClosed(object sender, EventArgs e)
    {
        this._isClosed = true;
    }

    private void DialogPresenterDataContextChanged(object sender,
                              DependencyPropertyChangedEventArgs e)
    {
        var d = e.NewValue as IDialogResultVMHelper;

        if (d == null)
            return;

        d.RequestCloseDialog += new EventHandler<RequestCloseDialogEventArgs>
                                    (DialogResultTrueEvent).MakeWeak(
                                        eh => d.RequestCloseDialog -= eh;);
    }

    private void DialogResultTrueEvent(object sender, 
                              RequestCloseDialogEventArgs eventargs)
    {
        // Important: Do not set DialogResult for a closed window
        // GC clears windows anyways and with MakeWeak it
        // closes out with IDialogResultVMHelper
        if(_isClosed) return;

        this.DialogResult = eventargs.DialogResult;
    }
 }

Now at least I have to create a DataTemplate in my resource file(app.xaml or something):

<DataTemplate DataType="{x:Type DialogViewModel:EditOrNewAuswahlItemVM}" >
        <DialogView:EditOrNewAuswahlItem/>
</DataTemplate>

Well thats all, I can now call dialogs from my viewmodels:

 var result = this.uiDialogService.ShowDialog("Dialogwindow Title", dialogwindowVM);

Now my question, do you see any problems with this solution?

Edit: for completeness. The ViewModel should implement IDialogResultVMHelper and then it can raise it within a OkCommand or something like this:

public class MyViewmodel : IDialogResultVMHelper
{
    private readonly Lazy<DelegateCommand> _okCommand;

    public MyViewmodel()
    {
         this._okCommand = new Lazy<DelegateCommand>(() => 
             new DelegateCommand(() => 
                 InvokeRequestCloseDialog(
                     new RequestCloseDialogEventArgs(true)), () => 
                         YourConditionsGoesHere = true));
    }

    public ICommand OkCommand
    { 
        get { return this._okCommand.Value; } 
    }

    public event EventHandler<RequestCloseDialogEventArgs> RequestCloseDialog;
    private void InvokeRequestCloseDialog(RequestCloseDialogEventArgs e)
    {
        var handler = RequestCloseDialog;
        if (handler != null) 
            handler(this, e);
    }
 }

EDIT 2: I used the code from here to make my EventHandler register weak:
http://diditwith.net/2007/03/23/SolvingTheProblemWithEventsWeakEventHandlers.aspx
(Website no longer exists, WebArchive Mirror)

public delegate void UnregisterCallback<TE>(EventHandler<TE> eventHandler) 
    where TE : EventArgs;

public interface IWeakEventHandler<TE> 
    where TE : EventArgs
{
    EventHandler<TE> Handler { get; }
}

public class WeakEventHandler<T, TE> : IWeakEventHandler<TE> 
    where T : class 
    where TE : EventArgs
{
    private delegate void OpenEventHandler(T @this, object sender, TE e);

    private readonly WeakReference mTargetRef;
    private readonly OpenEventHandler mOpenHandler;
    private readonly EventHandler<TE> mHandler;
    private UnregisterCallback<TE> mUnregister;

    public WeakEventHandler(EventHandler<TE> eventHandler,
                                UnregisterCallback<TE> unregister)
    {
        mTargetRef = new WeakReference(eventHandler.Target);

        mOpenHandler = (OpenEventHandler)Delegate.CreateDelegate(
                           typeof(OpenEventHandler),null, eventHandler.Method);

        mHandler = Invoke;
        mUnregister = unregister;
    }

    public void Invoke(object sender, TE e)
    {
        T target = (T)mTargetRef.Target;

        if (target != null)
            mOpenHandler.Invoke(target, sender, e);
        else if (mUnregister != null)
        {
            mUnregister(mHandler);
            mUnregister = null;
        }
    }

    public EventHandler<TE> Handler
    {
        get { return mHandler; }
    }

    public static implicit operator EventHandler<TE>(WeakEventHandler<T, TE> weh)
    {
        return weh.mHandler;
    }
}

public static class EventHandlerUtils
{
    public static EventHandler<TE> MakeWeak<TE>(this EventHandler<TE> eventHandler, 
                                                    UnregisterCallback<TE> unregister)
        where TE : EventArgs
    {
        if (eventHandler == null)
            throw new ArgumentNullException("eventHandler");

        if (eventHandler.Method.IsStatic || eventHandler.Target == null)
            throw new ArgumentException("Only instance methods are supported.",
                                            "eventHandler");

        var wehType = typeof(WeakEventHandler<,>).MakeGenericType(
                          eventHandler.Method.DeclaringType, typeof(TE));

        var wehConstructor = wehType.GetConstructor(new Type[] 
                             { 
                                 typeof(EventHandler<TE>), typeof(UnregisterCallback<TE>) 
                             });

        IWeakEventHandler<TE> weh = (IWeakEventHandler<TE>)wehConstructor.Invoke(
                                        new object[] { eventHandler, unregister });

        return weh.Handler;
    }
}

This is a good approach and I used similar ones in the past. Go for it!

One minor thing I'd definitely do is make the event receive a boolean for when you need to set "false" in the DialogResult.

event EventHandler<RequestCloseEventArgs> RequestCloseDialog;

and the EventArgs class:

public class RequestCloseEventArgs : EventArgs
{
    public RequestCloseEventArgs(bool dialogResult)
    {
        this.DialogResult = dialogResult;
    }

    public bool DialogResult { get; private set; }
}

I've been using an almost identical approach for several months now, and I'm very happy with it (i.e. I haven't yet felt the urge to rewrite it completely...)

In my implementation, I use a IDialogViewModel that exposes things such as the title, the standad buttons to show (in order to have a consistent apparence across all dialogs), a RequestClose event, and a few other things to be able to control the window size and behavior

If you are talking about dialogue windows and not just about the pop-up message boxes, please consider my approach below. The key points are:

1. I pass a reference to Module Controller into the constructor of each ViewModel (you can use injection).
2. That Module Controller has public/internal methods for creating dialogue windows (just creating, without returning a result). Hence to open a dialogue window in ViewModel I write: controller.OpenDialogEntity(bla, bla...)
3. Each dialogue window notifies about its result (like OK, Save, Cancel, etc.) via Weak Events. If you use PRISM, then it's easier to publish notifications using this EventAggregator.
4. To handle dialogue results, I'm using subscription to notifications (again Weak Events and EventAggregator in case of PRISM). To reduce dependency on such notifications, use independent classes with standard notifications.

Pros:

• Less code. I don't mind using interfaces, but I've seen too many projects where excessiveness of using interfaces and abstraction layers cause more trouble than help.
• Open dialogue windows through Module Controller is a simple way to avoid strong references and still allows to use mock-ups for testing.
• Notification through weak events reduce number of potential memory leaks.

Cons:

• Not easy to distinguish required notification from others in the handler. Two solutions:
  • send a unique token on opening a dialogue window and check that token in the subscription
  • use generic notification classes <T> where T is enumeration of entities (or for simplicity it can be type of ViewModel).
• For a project should be an agreement about using notification classes to prevent duplicating them.
• For enormously large projects the Module Controller can be overwhelmed by methods for creating windows. In this case it's better to split it up in several modules.

P.S. I have been using this approach for quite a long time now and ready to defend its eligibility in comments and provide some examples if required.