tags:

views:

11340

answers:

12

I'm trying to learn WPF and the MVVM problem, but have hit a snag. This question is similar but not quite the same as this one (handling-dialogs-in-wpf-with-mvvm)...

I have a "Login" form written using the MVVM pattern.

This form has a ViewModel which holds the Username and Password, which are bound to the view in the XAML using normal data bindings. It also has a "Login" command which is bound to the "Login" button on the form, agan using normal databinding.

When the "Login" command fires, it invokes a function in the ViewModel which goes off and sends data over the network to log in. When this function completes, there are 2 actions:

  1. The login was invalid - we just show a MessageBox and all is fine

  2. The login was valid, we need to close the Login form and have it return true as its DialogResult...

The problem is, the ViewModel knows nothing about the actual view, so how can it close the view and tell it to return a particular DialogResult?? I could stick some code in the CodeBehind, and/or pass the View through to the ViewModel, but that seems like it would defeat the whole point of MVVM entirely...


Update

In the end I just violated the "purity" of the MVVM pattern and had the View publish a Closed event, and expose a Close method. The ViewModel would then just call view.Close. The view is only known via an interface and wired up via an IOC container, so no testability or maintainability is lost.

It seems rather silly that the accepted answer is at -5 votes! While I'm well aware of the good feelings that one gets by solving a problem while being "pure", Surely I'm not the only one that thinks that 200 lines of events, commands and behaviors just to avoid a one line method in the name of "patterns" and "purity" is a bit ridiculous....

+1  A: 

Here's what I initially did, which does work, however it seems rather long-winded and ugly (global static anything is never good)

1: App.xaml.cs

public partial class App : Application
{
    // create a new global custom WPF Command
    public static readonly RoutedUICommand LoggedIn = new RoutedUICommand();
}

2: LoginForm.xaml

// bind the global command to a local eventhandler
<CommandBinding Command="client:App.LoggedIn" Executed="OnLoggedIn" />

3: LoginForm.xaml.cs

// implement the local eventhandler in codebehind
private void OnLoggedIn( object sender, ExecutedRoutedEventArgs e )
{
    DialogResult = true;
    Close();
}

4: LoginFormViewModel.cs

// fire the global command from the viewmodel
private void OnRemoteServerReturnedSuccess()
{
    App.LoggedIn.Execute(this, null);
}

I later on then removed all this code, and just had the LoginFormViewModel call the Close method on it's view. It ended up being much nicer and easier to follow. IMHO the point of patterns is to give people an easier way to understand what your app is doing, and in this case, MVVM was making it far harder to understand than if I hadn't used it, and was now an anti-pattern.

Orion Edwards
A: 

Why are you using MVVM for this dialog?

Testability? for a two fields dialog that nobody can even get into the application if you break?

It's cleaner to have "old fashioned" event handlers and code-behind then to use some global command hack.

Not everything is a good fit for MVVM.

Nir
I think because if you're following a pattern it doesn't hurt to keep it consistent throughout the application.
Ray Booysen
Well, in this particular case it does hurt, doesn't it?or maybe you don't think that spreading something over 4 source files and using global static state for what should be a one line code-behind method qualifies as "hurt"?
Nir
Who is using Global static state? I wouldn't agree that using that approach is a good idea however following the pattern to aid testability of something that is CRUCIAL to securing the app, it would seem appropriate to design it properly, instead of a one line call which isn't testable.
Ray Booysen
At least design things properly instead of one liner hacks which will just break further down the line with no unit tests to back it up. I would hope more developers spent time testing and making sure everything worked well on something you you think doesn't require it.
Ray Booysen
While the login form is a "two fields" dialog, I have many others which are a lot more complex (and hence warrant MVVM), but still need to be closed...
Orion Edwards
+4  A: 

There are a lot of comments arguing the pros and cons of MVVM here. For me, I agree with Nir; it's a matter of using the pattern appropriately and MVVM doesn't always fit. People seems to have become willing to sacrifice all of the most important principles of software design JUST to get it to fit MVVM.

That said,..i think your case could be a good fit with a bit of refactoring.

In most cases I've come across, WPF enables you to get by WITHOUT multiple Windows. Maybe you could try using Frames and Pages instead of Windows with DialogResults.

In your case my suggestion would be have LoginFormViewModel handle the LoginCommand and if the login is invalid, set a property on LoginFormViewModel to an appropriate value (false or some enum value like UserAuthenticationStates.FailedAuthentication). You'd do the same for a successful login (true or some other enum value). You'd then use a DataTrigger which responds to the various user authentication states and could use a simple Setter to change the Source property of the Frame.

Having your login Window return a DialogResult i think is where you're getting confused; that DialogResult is really a property of your ViewModel. In my, admittedly limited experience with WPF, when something doesn't feel right it usually because I'm thinking in terms of how i would've done the same thing in WinForms.

Hope that helps.

Stimul8d
+1  A: 

The way I would handle it is to add an event handler in my ViewModel. When the user was successfully logged in I would fire the event. In my View I would attach to this event and when it fired I would close the window.

That's what i usually do, too. Even though that seems a bit dirty considering all that newfangled wpf-commanding stuff.
Botz3000
+2  A: 

FYI, I ran into this same problem and I think I figured out a work around that doesn't require globals or statics, although it may not be the best answer. I let the you guys decide that for yourself.

In my case, the ViewModel that instantiates the Window to be displayed (lets call it ViewModelMain) also knows about the LoginFormViewModel (using the situation above as an example).

So what I did was to create a property on the LoginFormViewModel that was of type ICommand (Lets call it CloseWindowCommand). Then, before I call .ShowDialog() on the Window, I set the CloseWindowCommand property on the LoginFormViewModel to the window.Close() method of the Window I instantiated. Then inside the LoginFormViewModel all I have to do is call CloseWindowCommand.Execute() to close the window.

It is a bit of a workaround/hack I suppose, but it works well without really breaking the MVVM pattern.

Feel free to critique this process as much as you like, I can take it! :)

I'm not sure I fully grok it, but doesn't this mean that your MainWindow has to be instantiated before your LoginWindow?That's something I'd like to avoid if possible
Orion Edwards
+1  A: 

This is probably very late, but I came across the same problem and I found a solution that works for me.

I can't figure out how to create an app without dialogs(maybe it's just a mind block). So I was at an impasse with MVVM and showing a dialog. So I came across this CodeProject article:

http://www.codeproject.com/KB/WPF/XAMLDialog.aspx

Which is a UserControl that basically allows a window to be within the visual tree of another window(not allowed in xaml). It also exposes a boolean DependencyProperty called IsShowing.

You can set a style like,typically in a resourcedictionary, that basically displays the dialog whenever the Content property of the control != null via triggers:

<Style TargetType="{x:Type d:Dialog}">
    <Style.Triggers>
        <Trigger Property="HasContent"  Value="True">
            <Setter Property="Showing" Value="True" />
        </Trigger>
    </Style.Triggers>
</Style>

In the view where you want to display the dialog simply have this:

<d:Dialog Content="{Binding Path=DialogViewModel}"/>

And in your ViewModel all you have to do is set the property to a value(Note: the ViewModel class must support INotifyPropertyChanged for the view to know something happened ).

like so:

DialogViewModel = new DisplayViewModel();

To match the ViewModel with the View you should have something like this in a resourcedictionary:

<DataTemplate DataType="{x:Type vm:DisplayViewModel}">
    <vw:DisplayView/>
</DataTemplate>

With all of that you get a one-liner code to show dialog. The problem you get is you can't really close the dialog with just the above code. So that's why you have to put in an event in a ViewModel base class which DisplayViewModel inherits from and instead of the code above, write this

        var vm = new DisplayViewModel();
        vm.RequestClose += new RequestCloseHandler(DisplayViewModel_RequestClose);
        DialogViewModel = vm;

Then you can handle the result of the dialog via the callback.

This may seem a little complex, but once the groundwork is laid, it's pretty straightforward. Again this is my implementation, I'm sure there are others :)

Hope this helps, it saved me.

Jose
+1  A: 

I used attached behaviours to close the window. Bind a "signal" property on your ViewModel to the attached behaviour (I actually use a trigger) When it's set to true, the behaviour closes the window.

http://adammills.wordpress.com/2009/07/01/window-close-from-xaml/

Thejuan
This is the only answer so far that doesn't require any codebehind in the Window (and does actually close a modal Window, instead of suggesting another approach). Pity it requires so much complexity, with the Style and Trigger and all that muck -- it seems like this really should be doable with a one-line attached behavior.
Joe White
Now it is doable with a one-line attached behavior. See my answer: http://stackoverflow.com/questions/501886/wpf-mvvm-newbie-how-should-the-viewmodel-close-the-form/3329467#3329467
Joe White
A: 

Assuming your login dialog is the first window that gets created, try this inside your LoginViewModel class:

    void OnLoginResponse(bool loginSucceded)
    {
        if (loginSucceded)
        {
            Window1 window = new Window1() { DataContext = new MainWindowViewModel() };
            window.Show();

            App.Current.MainWindow.Close();
            App.Current.MainWindow = window;
        }
        else
        {
            LoginError = true;
        }
    }
Jim Wallace
+15  A: 

From my perspective the question is pretty good as same approach would be used not only for "Login" window, but for any kind of them. I've passed through a lot of suggestions and no one is ok for me. Please see my kind, that was taken from the MVVM design pattern article (http://msdn.microsoft.com/en-us/magazine/dd419663.aspx).

Each ViewModel class should be inherited from "WorkspaceViewModel" that has "RequestClose" envent, and "CloseCommand" property of the ICommand type. Default implementation of the CloseCommand property will Raise RequestClose event.

And in order to get window closed the "OnLoaded" method of your window should be overrided:

void CustomerWindow_Loaded(object sender, RoutedEventArgs e)
{
    CustomerViewModel customer = CustomerViewModel.GetYourCustomer();
    DataContext = customer;
    customer.RequestClose += () => { Close(); };
}

or "OnStartup" method of you app:

    protected override void OnStartup(StartupEventArgs e)
    {
        base.OnStartup(e);

        MainWindow window = new MainWindow();
        var viewModel = new MainWindowViewModel();
        viewModel.RequestClose += window.Close;
        window.DataContext = viewModel;

        window.Show();
    }

I guess that RequestClose event and CloseCommand property implementation in the WorkspaceViewModel are pretty clear, but I will show them to be consistent:

public abstract class WorkspaceViewModel : ViewModelBase // There are nothing interest in ViewModelBase, it only implements INotifyPropertyChanged interface only
{
    RelayCommand _closeCommand;
    public ICommand CloseCommand
    {
        get
        {
            if (_closeCommand == null)
            {
                _closeCommand = new RelayCommand(
                   param => Close(),
                   param => CanClose()
                   );
            }
            return _closeCommand;
        }
    }

    public event Action RequestClose;

    public virtual void Close()
    {
        if ( RequestClose!=null )
        {
            RequestClose();
        }
    }

    public virtual bool CanClose()
    {
        return true;
    }
}

And the source code of the RelayCommand:

public class RelayCommand : ICommand
{
    #region Constructors

    public RelayCommand(Action<object> execute, Predicate<object> canExecute)
    {
        if (execute == null)
            throw new ArgumentNullException("execute");

        _execute = execute;
        _canExecute = canExecute;
    }
    #endregion // Constructors

    #region ICommand Members

    [DebuggerStepThrough]
    public bool CanExecute(object parameter)
    {
        return _canExecute == null ? true : _canExecute(parameter);
    }

    public event EventHandler CanExecuteChanged
    {
        add { CommandManager.RequerySuggested += value; }
        remove { CommandManager.RequerySuggested -= value; }
    }

    public void Execute(object parameter)
    {
        _execute(parameter);
    }

    #endregion // ICommand Members

    #region Fields

    readonly Action<object> _execute;
    readonly Predicate<object> _canExecute;

    #endregion // Fields
}

P.S. Don't treat me badly for those sources, If I had that yesterday that would save me few hours...

P.P.S Any comments or suggestions are welcome.

Budda
Exactly how I did it.Highly recommanding this solution.
esylvestre
+1 I like the way you didn't just copy and paste Josh Smith's code, i.e. changing EventHandler to Action to avoid unnecessary params and lambda code in OnStartup.
Si
A: 

I tried this customerView window = new CustomerView(); var viewModel = new CustomerViewModel(_CurrentCustomer); viewModel.CloseCommand = window.Close; Compile error : Cannot convert method group 'Close' to non-delegate type 'RelayCommand'

I tried also viewModel.RequestClose += window.Close; Compile error : No overload to 'Close' matches delegate System.EventHandler.

What is the problem? Maymone

Maymoned
+15  A: 

I was inspired by Thejuan's answer to write a simpler attached property. No styles, no triggers; instead, you can just do this:

<Window ...
        xmlns:xc="clr-namespace:ExCastle.Wpf"
        xc:DialogCloser.DialogResult="{Binding DialogResult}">

This is almost as clean as if the WPF team had gotten it right and made DialogResult a dependency property in the first place. Just put a bool? DialogResult property on your ViewModel and implement INotifyPropertyChanged, and voilà, your ViewModel can close the Window (and set its DialogResult) just by setting a property. MVVM as it should be.

Here's the code for DialogCloser:

using System.Windows;

namespace ExCastle.Wpf
{
    public static class DialogCloser
    {
        public static readonly DependencyProperty DialogResultProperty =
            DependencyProperty.RegisterAttached(
                "DialogResult",
                typeof(bool?),
                typeof(DialogCloser),
                new PropertyMetadata(DialogResultChanged));

        private static void DialogResultChanged(
            DependencyObject d,
            DependencyPropertyChangedEventArgs e)
        {
            var window = d as Window;
            if (window != null)
                window.DialogResult = e.NewValue as bool?;
        }
        public static void SetDialogResult(Window target, bool? value)
        {
            target.SetValue(DialogResultProperty, value);
        }
    }
}

I've also posted this on my blog.

Joe White
+1 but why not post the source in the answer?
Si
I suppose I could -- it's pretty short. Added.
Joe White
This one is the answer I like the most! Good job writing that attached property.
Jorge Vargas
Besides, this is what happens when you set the attribute IsCancel="True" in a button, it sets the property DialogResult of the Window to false, which closes the window.
Jorge Vargas
@Jorge: no, the IsCancel property doesn't do that. It just means that pressing Esc will have the same effect as a click on the button, but you still have to set the DialogResult yourself
Thomas Levesque
@Thomas: As long as you show the window with Window.ShowDialog(), what I said is correct. Check this answer: http://stackoverflow.com/questions/419596/how-does-the-wpf-button-iscancel-property-work/419615#419615
Jorge Vargas
@Jorge, I just tried again, and it appears that you're right... I remembered that it didn't work for IsDefault, so I wrongly assumed it didn't work for IsCancel either.
Thomas Levesque
A: 

Why not just pass the window as a command parameter?

C#:

 private void Cancel( Window window )
  {
     window.Close();
  }

  private ICommand _cancelCommand;
  public ICommand CancelCommand
  {
     get
     {
        return _cancelCommand ?? ( _cancelCommand = new Command.RelayCommand<Window>(
                                                      ( window ) => Cancel( window ),
                                                      ( window ) => ( true ) ) );
     }
  }

XAML:

<Window x:Class="WPFRunApp.MainWindow"
        x:Name="_runWindow"
...
   <Button Content="Cancel"
           Command="{Binding Path=CancelCommand}"
           CommandParameter="{Binding ElementName=_runWindow}" />
chrislarson