Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for dialogs #24

Closed
JohnStov opened this issue Nov 7, 2017 · 32 comments · Fixed by #97
Closed

Support for dialogs #24

JohnStov opened this issue Nov 7, 2017 · 32 comments · Fixed by #97

Comments

@JohnStov
Copy link

JohnStov commented Nov 7, 2017

It's not clear how to implement a modal dialog in Elmish.WPF.

Seems like using Window.ShowDialog() should be the way to go, but it's not clear how to hook into the dialog's dispatch loop. Is there a need for explicit dialog support, or is it just a case of providing an example?

I'd be happy to provide a sample if someone could give me some guidance.

@2sComplement
Copy link
Collaborator

I would recommend implementing your own "dialog", which is fairly easy and imho looks much better than windows dialogs, which are also notoriously difficult to deal with when using MVVM, let alone elmish.
I've added a concept of this to the design data context sample, does that work for you?

@2sComplement
Copy link
Collaborator

Closing unless this becomes an issue again.

@JohnStov
Copy link
Author

Sorry I missed this. Yes, this is great. Thanks for the example.

@cmeeren
Copy link
Member

cmeeren commented Sep 6, 2018

@giuliohome (and @2sComplement):

  • Can you elaborate a bit on the modal vs. non-modal (Show vs. ShowDialog) you mentioned? How is it more or less responsive?
  • Do you see any drawbacks to using the same binding context as the main window? Semantically it seems to make more sense to use something akin to Binding.subModel.
  • If there is a good way to do it, I'll happily implement direct support in Elmish.WPF for opening new windows to ensure it's "done right" if users want to do it. Do you have any good suggestions for an API? I'm thinking that maybe we need to use Cmd for this, since it's a side-effect. Also, how do you reliably know when a window/dialog has closed, and its result?

@JDiLenarda
Copy link
Contributor

Few thoughts :

ShowDialog may block the update loop (MSDN says Dispatcher.Invoke is synchronous). Not a big deal for a Yes/No dialog, but may be problematic for a configuration dialog. There either the dialog run its own loop, or is called in async context, hopefully blocking the calling window but not the loop.

Since the use of Cmd allows to send message back in the loop, we can await for the closing of a modeless window and inform the model, though I don’t think it’s a common need.

I agree that a window call for a submodel binding. Since the window DataContext is a ViewModel it should be possible to call the submodel by member name at call time, if no xaml syntax allows it already.

@JDiLenarda
Copy link
Contributor

Binding.subModelSeq is surely the way to deal with master-details pattern.

Of course, there should be no prohibition to use the very same DataContext between a caller window and its called dialog and that would be hard to prohibit in the first place. Simply binding a called window with a subModel makes sense.

I'll look at this this week-end.

@cmeeren
Copy link
Member

cmeeren commented Sep 7, 2018

I fail to see a meaningful use case for catching the closing of a modeless window (as opposite to a modal one)

Just from the top of my head (because I'm working with such an app, though I ended up using material dialogs in the same window instead): You could have a data input app where the input form was a separate modeless window. This would allow you to browse the existing data while creating a new entry. Closing the modeless window via the "Submit" button would send an action containing the new data to be added. And the "Cancel" button would just close the window.

Note that in this case, I can see some potential concerns right away. On the one hand, the actual closing of the window must be controlled outside of the Elmish loop (e.g. in the codebehind). On the other hand, the "Submit" button may first validate the form, and if it's valid, close the window and send the message containing the new data, or if it's invalid, display validation errors (and not close the window). The submit/validation message can be part of the Elmish loop/model (make Binding.cmd return the correct message based on the current model), but in order to prevent closing the window in the second case, the codebehind would need to know what action the button currently will submit (impossible AFAIK since it's wrapped in a delegate wrapped in a Command). I think also the same problem appears if you want to do change detection on the Cancel button, warning the user that changes will be lost and allowing them to return to the window and not close it.

This is what I was getting at with the FAQ answer: Controlling whether windows are open or closed cannot be part of the Elmish model. This is, however, trivial with a custom/material dialog as suggested (and demoed in the SubModelOpt sample).

IMHO any official Elmish.WPF solution to opening new windows should be flexible enough to cater to many kinds of usecases such as this. If that's not possible or feasible, we might just as well add some documentation tipping users to use something akin to @giuliohome's example in a Command, which is a perfectly viable workaround if you want to open a new window.

consider also that in many cases one simply wants to stick to the same ViewModel instance

You can easily do this using subModel (e.g. by passing id as some of the arguments).

@cmeeren
Copy link
Member

cmeeren commented Sep 7, 2018

This is very unclear to me: usually modal windows are closed with "submit" buttons, I'm not sure how that would apply to modeless windows.

IIRC (based on the version of my app that used windows) there's no significant difference code-wise (apart from calling ShowDialog instead of Show). I agree that modal windows may be the canonical choice for the situation I described, but one could easily imagine use-cases or business requirements that the rest of the app should still be accessible even though a data entry window is open.

Aside from that, I imagine a "Command" binding for the "Submit" button and I'm confused when you speak about "codebehind" instead...

This may be my inexperience talking, but I see two ways of closing windows that are relevant to the current discussion:

  1. The window can be closed from the code-behind (e.g. the .xaml.cs file, just to be clear), in a handler for the Button.Click event. This places non-view logic* outside the Elmish loop and has the problems I described. (*I consider opening/closing windows "non-view logic" since it's more than just specifying how a view behaves given some bindings.)

  2. In the command that opens the window, you add the window and some handle for it to a global, mutable dictionary. The handle could anything that can be sent in a message and allows to retrieve the window object later, e.g. a GUID that you assign to a custom property on your Window subclass (or the window object itself, but that has significant drawbacks, see below). The command binding for the Submit/Cancel buttons must then be bound to a Binding.cmd or similar that returns a message that contains this handle (meaning the handle would need to be part of your Elmish model, so using the Window object directly seems very messy and not testable). When this message is dispatched, your update function can return a command that looks up the Window object in the mutable dictionary based on the handle, and closes the window (and removes it from the dictionary).

    As you can see, this approach seems rather messy in its own right.

Let me know if I need to elaborate further.

On another note: Could you elaborate on a couple of use-cases for modeless windows? Just to ensure we are on the same page.

@cmeeren
Copy link
Member

cmeeren commented Sep 7, 2018

Ah, I think I see where each of us stand now.

While I have made some apps in Xamarin.Forms, my experience with WPF is limited and mostly concerned with simple data entry. When I think of new windows, I think of a (temporary) window that in some way produces a result (such as a new business entity to be saved). The closing of the window is inextricably linked to some domain event (in this case, dispatching of an Elmish message containing the new business entity).

You seem to be talking about windows that do not produce a result. If I understand you correctly, an example would be a "Preferences" window that automatically saves settings as you edit them. Domain events (e.g. message dispatches) happen while you use the window, but when you're done, you can just close it, e.g. using the X in the corner. (Though in this particular example, one could argue that a dialog works just as well as a new window.)

These seem to be two different use-cases, and could be supported (or not) individually. I can envision a couple of Cmd.showWindow functions to display a given window, either using the DataContext of the main window, or using a sub-model akin to Binding.subModel.

A significant caveat though is that this requires the app's core Elmish project (where the update functions live) to reference the view projects (containing the window definitions), and this is an inversion of responsibilities I don't like. In MVVM, you shouldn't reference the view from a view model. Similarly, in an Elmish architecture, I'd like to keep the central model definitions and update functions completely separate from the UI. Ideally, only the bindings (view) function and mkProgram calls should be in the UI project; everything else should not reference any UI architecture, similar to how it's ideally done in MVVM. (Note that the samples in this repo keep all the Elmish stuff in a single project for simplicity.)

I'd like your (and anyone else's) input on this. I may of course have overlooked a better solution.

In any case, given my lack of WPF experience, I'd be very happy if you could explain a couple of use-cases you have for modeless windows, why a modeless window is a better solution than a Visibility-controlled dialog in these cases, and if relevant, how the windows would work in relation to the Elmish architecture.

@cmeeren
Copy link
Member

cmeeren commented Sep 7, 2018

Thank you, it seems we understand each other then.

You have shown a very simple way of opening a window from the update function, which I'm sure is trivial to use from a command instead of the update function. The fact that this is fairly simple, together with the fact that proper separation of concerns would include some kind of navigation service, makes me think that this does not really belong in Elmish.WPF. I think we could be fine with just a sample that shows how to open a new window and set the DataContext in a command. Do you agree?

@cmeeren
Copy link
Member

cmeeren commented Sep 8, 2018

I have now added a new sample called NewWindow and updated the readme with more comprehensive notes on new windows (as well as project structure). @giuliohome (and anyone else interested), could you take a look and see if you're happy?

giuliohome added a commit to giuliohome/ElmCounterWPF that referenced this issue Sep 8, 2018
@cmeeren cmeeren closed this as completed Sep 8, 2018
@cmeeren
Copy link
Member

cmeeren commented Sep 9, 2018

  1. Noted. The FAQ entry does not exclude such a scenario; it only states that there are difficulties inherent to the situation that a button both dispatches a message and closes the window. Your scenario is more akin to the "modeless windows not producing a result" scenario, which is covered. If there's anything in the readme you want changed, feel free to submit a PR.

  2. I don't really get your meaning here. I do not mention anything about master-detail in the FAQ or in the sample. The sample is just a very simple way to show how one can use a command to open windows and set the window's DataContext. The sample also sets the DataContext of top-level wrappers in the new windows to subModel bindings of the main DataContext. The user may or may not want to do so; I'm just showing that it's possible (just like one might want to do with some UserControls). Also note that there is only ever a single update loop. As stated in the FAQ, from the point of view of Elmish.WPF there is absolutely no difference between using new modeless windows or using UserControls in the same window.

  3. INotifyDataErrorInfo is implemented and working, as shown in the Validation sample. It was also working before the rewrite, but in a more limited fashion; it was a set-only validation where no message was dispatched if the input was invalid. This has certain important limitations, as discussed in How to use invalid input in update function? #45 and in the Validation sample.

@cmeeren
Copy link
Member

cmeeren commented Sep 9, 2018

Thanks for the clarification and concrete example, I understand what you mean now. Though I can't promise when I'll get to it, I'll try to come up with a sample that demonstrates how this can be done.

@JDiLenarda
Copy link
Contributor

With my tinkerings this week end, I found out that it's a good idea to call a dialog through a Binding.cmd. There, it automatically run in the application dispatcher in an asynchronous way. Shown there, they don't block the update loop. We can use the elmish dispatch function passed to the bindings/views function.

Though there may be case where it's needed to call a window in the command part of the loop, that may help to keep the update function out of UI consideration.

Also, a few week ago, I wanted to see if I can write an AutoCAD palette with Elmish. This lead me to write a Program.runUserControl that actually just bind a user control with an update loop (there's nothing to run in fact - just find a way to bind the control to the loop before/when the calling app load it). I was writing an exemple palatable to people who doesn't have an AutoCAD licence paid by their boss, but this issue make me consider a few things, notably that we can bind a window with their own elmish loop. That may feel anti-pattern, but I can see it used to implement something like the Outlook contact book (something that resembles @cmeeren submit modeless window). However for now, I just looked how to deal with modal dialogs.

Please look here, see the OpenDialog example.

@JDiLenarda
Copy link
Contributor

Updated.

@giuliohome : please, take a look at sample project ModelessWindow.SeparatedLoop (and at SharedLoop as well)

@JDiLenarda
Copy link
Contributor

Updated anew. No new functions, just refactoring, including samples where I try to define some good practices to deal with called windows. Feedback are welcome on this point, especially on what can be done with xaml.

@JDiLenarda
Copy link
Contributor

Fun fact : with Binding.paramCmd one can retreive the ViewModel from a sub-model, then assign it to any Window.DataContext.

@cmeeren
Copy link
Member

cmeeren commented Oct 16, 2018

Yes, that's true - if a bit hacky, and liable to break in the future since one is delving into implementation details. If there is a real need for such functionality, we should consider implementing a proper API for it. :)

@JDiLenarda
Copy link
Contributor

Reading the new v3 ReadMe.md, I'd like to point out it is possible to invoke a modal dialog without blocking the Elmish dispatch loop :

Application.Current.Dispatcher.Invoke (fun () ->
    let guiCtx = System.Threading.SynchronizationContext.Current
    async {
        let dlg = SomeModalWindow ()
        dlg.Owner <- Application.Current.MainWindow
        dlg.DataContext <- Application.Current.MainWindow.DataContext
        let currentCtx = System.Threading.SynchronizationContext.Current
        do! Async.SwitchToContext guiCtx
        dlg.ShowDialog () |> ignore
        do! Async.SwitchToContext currentCtx
    } |> Async.StartImmediate)

(also, unrelated, link to CmdMsg pattern from Fabulous documentation is broken)

@cmeeren
Copy link
Member

cmeeren commented Jul 24, 2019

Thanks @JDiLenarda, that's great! I've experimented a bit now and it seems that the minimal code required is the below (a function similar to Cmd.showWindow). Could you verify that it works in your case? Then I can release it on nuget.

let showDialog (getWindow: unit -> #Window) =
  let showWin () =
    Application.Current.Dispatcher.Invoke(fun () ->
      let guiCtx = System.Threading.SynchronizationContext.Current
      async {
        let win = getWindow ()
        win.DataContext <- Application.Current.MainWindow.DataContext
        do! Async.SwitchToContext guiCtx
        win.ShowDialog () |> ignore
      }
    )
  Cmd.OfAsync.attempt showWin () raise

@JDiLenarda
Copy link
Contributor

JDiLenarda commented Jul 30, 2019

After testing, I suggest to keep the part where ownership of the dialog is given to the main window. This prevent the dialog to disappear behind the main window if it is clicked.

let showDialog (getWindow: unit -> #Window) =
      let showWin () =
        Application.Current.Dispatcher.Invoke(fun () ->
          let guiCtx = System.Threading.SynchronizationContext.Current
          async {
            let win = getWindow ()
            win.Owner <- Application.Current.MainWindow
            win.DataContext <- Application.Current.MainWindow.DataContext
            do! Async.SwitchToContext guiCtx
            win.ShowDialog () |> ignore
          }
        )
      Cmd.OfAsync.attempt showWin () raise

As such, this is all I currently need, so feel free to commit.

But for further evolution, it would be interesting to consider the result of showDialog, with maybe an overload that take continuations depending on the result.
For reference, when creating a modal window, it is possible to determine from xaml what button is for cancelling (property IsCancel = true, dialog is closed and showDialog will return false) and which is for accepting (IsDefault = true). But there is a trick for the default button : it doesn't close the dialog, the user is supposed to set a click event on the button where the window property DialogResult is set to true (or false) or null, depending if the dialog must be close and accept (true), close and cancel (false) or not be closed (null).
So what would be interesting to consider is a showDialog function like this :

let showDialog (getWindow: unit -> #Window) (validate: 'model -> bool option) (onAccept: 'model -> 'msg) (onCancel: 'model -> 'msg)

Again, this is a suggestion for the future, not an immediate need for me, neither the correct signature.

@cmeeren
Copy link
Member

cmeeren commented Jul 30, 2019

I suggest to keep the part where ownership of the dialog is given to the main window.

I am thinking that the window owner should be controlled by the user (using the getWindow function). IIRC, that particular example - setting the owner - is already mentioned in the existing showWindow documentation. That way, the user can set the owner to whatever window they want, or not set an owner at all if that suits their desired behaviour. If we set the window owner, then we force a specific behaviour on the user, which I don't want. What do you think?


Regarding the dialog result stuff: Interesting! I'll have to think carefully on that functionality and signature; it requires careful consideration to ensure it meshes well with Elmish architecture. For example, one use-case I want to enable is clicking Ok and then have validation errors show (and the window not close). I'll certainly consider it.

@JDiLenarda
Copy link
Contributor

I suggest to keep the part where ownership of the dialog is given to the main window.

I am thinking that the window owner should be controlled by the user (using the getWindow function). IIRC, that particular example - setting the owner - is already mentioned in the existing showWindow documentation. That way, the user can set the owner to whatever window they want, or not set an owner at all if that suits their desired behaviour. If we set the window owner, then we force a specific behaviour on the user, which I don't want. What do you think?

I didn't notice that. I modified my code and I'm ok with this.

@cmeeren
Copy link
Member

cmeeren commented Aug 1, 2019

Great! I'll experiment a bit with result-producing windows as per your suggestion before I publish anything.

@cmeeren cmeeren reopened this Aug 1, 2019
@cmeeren
Copy link
Member

cmeeren commented Aug 1, 2019

Hmm, one problem with the proposed result stuff is that AFAIK there is no way to access the current model from a command (only the dispatch if using Cmd.ofSub). Any ideas?

@cmeeren
Copy link
Member

cmeeren commented Aug 1, 2019

Also, I can't immediately think of any smart ways to have a long-lived dialog when using IsDefault/IsCancel/Esc. Clicking these will automatically close the dialog. Regardless of that however, according to this page,

After ShowDialog has returned, a dialog box cannot be reopened. Instead, you need to create a new instance.

Once I get the first result, that's it, ShowDialog has returned. If the user didn't want the dialog to close, the DialogResult must not be set in the first place, i.e., IsDefault and IsCancel can't be supported properly.

Am I right?

@JDiLenarda
Copy link
Contributor

For retreiving the model : since the datacontext is the ViewModel<‘model,’msg>, can’t we retrieve it through either reflection (hackish) or internal function ?

For the button behaviour : I’m confused. For the dialog closing right now on IsCancel, that’s what I’m expecting, but not for IsDefault.

Ah, well, after getting another look at the documentation, I realize I was mistaken about the role of IsDefault. It’s not for the Ok button specifically, but for telling what button must be clicked when the user press enter. So the same button can be IsCancel and IsDefault (For dialogs with only one button, or the ones where the developer want to play it safe). I’m sorry, that was a false lead. So maybe working on the x:Name of the control ?

Look, I’ll try to dig further and come with a proof of concept, but that won’t be before a few weeks. Btw, I’ll make a post on what I have theorized on the subject in the next days.

Anyway, thank you for considering the demand (and for that very good v3, too).

@cmeeren
Copy link
Member

cmeeren commented Aug 2, 2019

Another way to do this which I kinda like, is to have the signature be showDialog: (getWindow: #Window -> unit) : DialogHandle * Cmd<'msg>, and also have a function closeDialog: DialogHandle -> Cmd<'msg>. The DialogHandle could be a simple GUID, or just an obj instance. Then users could save the DialogHandle in the model and use it to close the dialog when they wish from the update function, implementing the button handlers as they normally do (binding to normal Binding.cmds). Elmish.WPF would keep an internal mapping between DialogHandles and Windows.

The downside is that it doesn't feel "pure" to have handles in state. But then again, dialogs like this aren't idiomatic Elm anyway, and static views has its own idiosyncrasies no matter how you look at it, so I'm not too concerned about this.

The upside is that it's very simple, requiring no special continuations or out-of-message-loop stuff, and apart from having a handle in the state, it meshes very well with Elmish architecture.

(To be clear; with this method, we don't use DialogResult at all. Both Ok and Cancel buttons dispatch messages that are handled normally in the update loop.)

@cmeeren
Copy link
Member

cmeeren commented Aug 2, 2019

Try the feature-dialogs branch and check out the rewritten NewWindow sample. Let me know what you think.

@cmeeren
Copy link
Member

cmeeren commented Aug 3, 2019

Okay, I'm making progress with a new approach to windows I like much better: Use Binding stuff to open/hide windows. This has many benefits, most notably that windows are moved away from commands in the update function (which should not have anything to do with UI) and to the UI layer (the bindings are, after all, the Elmish view function).

In effect, this gets rid of most current drawbacks mentioned in the readme:

  • No significant impacts on structure/architecture, since the bindings function is UI-related and therefore free to reference XAML stuff - no navigation service required to separate update from UI
  • Modal windows work just as well as non-modal
  • Result-producing windows? No problem since the Window state (shown/hidden/closed) is controlled from the state (via a binding)

Will push a new branch when I'm ready to show something resembling what I have in mind.

@cmeeren
Copy link
Member

cmeeren commented Aug 4, 2019

All right! I have pushed some new delightful stuff to a new feature-windows-binding branch.

How delightful, you ask?

I'd say the readme update commit pretty much sums it up nicely - from a wall-of-text "yes you can use windows, buuut" to a single-paragraph "sure!".

Check out the NewWindow sample, give the new API a spin, and let me know what you think. I'm pretty sold on this, given how well it works and all the usability problems it solves. IMHO it's in a shape where I'm comfortable releasing, but I'd like a second opinion first.

@JDiLenarda
Copy link
Contributor

Calling windows and dialogs through Binding rather than Cmd is a very good idea. Be sure I'll get a further look at it, I just don't know when.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants