elmish / Elmish.WPF

Static WPF views for elmish programs
Other
430 stars 71 forks source link

Binding to events? #33

Closed dsyme closed 5 years ago

dsyme commented 6 years ago

Hi all

I'm seeing plenty of examples of Xaml where binding to events via code behind is needed, e.g. see below for a snippet of Xamarin code.

My question is - what's the right way to go about this in Elmish.WPF?

From what I've seen there doesn't seem much alternative to writing the code-behind event handler (apart from massive things like this with very intrusive Xaml) - but how would the event handler capture the Elmish dispatch routine?

Xaml:

        <ListView x:Name="ItemsListView" ItemsSource="{Binding [Items]}" ItemSelected="OnItemSelected" ....  >          

code behind:

    async void OnItemSelected(object sender, SelectedItemChangedEventArgs args)
    {
        ...
    }
2sComplement commented 6 years ago

I haven't yet needed to do this in Elmish.WPF, but in vanilla MVVM you would probably get a reference to the Control's DataContext (its ViewModel) in code-behind and do something with it there. In Elmish.WPF with the ViewModel being a DynamicObject, this should still be possible even if not the cleanest solution (isn't code-behind-to-view-model always a little dirty?).

Stepping back for a second though, in your example you could just hook up a two-way binding to the ListView's SelectedItem no?

dsyme commented 6 years ago

@2sComplement Thanks!

Yes, it looks like I could use SelectedItem, but I'm trying to work out whether Elmish.XamarinForms is complete enough to do everything necessary. There are a lot of events in XamarinForms that don't have a corresponding 2-way binding.

2sComplement commented 6 years ago

@dsyme Ideally I try to keep code-behind limited to view-only types of interactions. I'd be curious to know what other events would be applicable here, where they would need some way of binding to the ViewModel.

dsyme commented 6 years ago

@2sComplement Some of the samples may be sloppy Xaml. Another example:

    <ToolbarItem Text="Save" Clicked="Save_Clicked" />

should be:

    <ToolbarItem Text=Save Command= SaveCommand />

But I expect there is a lot of this kind of thing around in samples, including educational ones. Also, in the samples I'm translating I also see event hookup for ItemAppearing, which AFAIK is not accessible via command. More generally, to take an example, the list of events on Xamarin ListView is:

Both of these make me think that if there's some way to wire up events to commands (i.e. commands stored in the ViewModel) then it would be useful...

2sComplement commented 6 years ago

System.Windows.Interactivity seems to provide a way of doing this via the InvokeCommandAction class (taken from this blog post):

<Window x:Class="Mm.HandlingEventsMVVM.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:i="clr-namespace:System.Windows.Interactivity;assembly=System.Windows.Interactivity"
        Title="MainWindow" Height="350" Width="525">
    <StackPanel>
        <Rectangle Fill="Yellow" Stroke="Black" Width="100" Height="100">
            <i:Interaction.Triggers>
                <i:EventTrigger EventName="MouseEnter" >
                    <i:InvokeCommandAction Command="{Binding MouseEnterCommand}" />
                </i:EventTrigger>
            </i:Interaction.Triggers>
        </Rectangle>
    </StackPanel>
</Window>

Does this help for Xamarin?

dsyme commented 6 years ago

So much xaml :)

We might be able to get Xamarin.Forms to allow

                 ItemSelected=“{Binding [ItemSelected]}”

where the view model property ItemSelected returns a delegate of an appropriate type.

I'm discussing that with the XF people as a possible Xaml extension. I think the implementation might be very simple: just allow value to be a delegate here and just call eventInfo.AddEventHandler(value) directly in that case

dsyme commented 6 years ago

@2sComplement If we use the SelectedItem bindable property we get the nasty:

"SelectedItem"|> Binding.twoWay (fun m -> null) (fun v m -> 
    if not (isNull v) then Device.OpenUri(Uri("http://fsharp.org"))
    Ignore ) 

The return of null seems harmless (if we don't do that we'd have to store the SelectedItem in the elmish model, which seems wrong when all we're trying to do is hook into the event to cause an external action (or a navigation action), i.e. a command)

Have you seen this pattern before, i.e. where a change in a property on a UI element is perceived as a command as far as the application is concerned? I suppose each time this happens it's an indication of a missing Command in the UI framework, e.g. Xamarin.Forms doesn't have an ItemSelectedCommand, just the event instead.

2sComplement commented 6 years ago

Lots of Xaml for sure, but it appears to be the best practice for this specific scenario.

In your example, the event of interest tells you that an item was selected, so to me it makes sense to store that item in your model, and incur any side effects as a result of the selection. If you want to trigger a command from a control inside a ListView, you'd probably create a custom control that has a button or a link or something that actually has a Command dependency property attached to it, and fire off your command from there.

I personally don't run in to this scenario very often; I find that, with a few exceptions, UI events that are not dependency properties are pertinent to view-side code.

dsyme commented 6 years ago

Lots of Xaml for sure, but it appears to be the best practice for this specific scenario.

Yes, agreed. Though best practice in Xaml can also mean unusable :) In any case, the XF guys seem open to making this simpler in their variant of Xaml, either via ItemSelectedCommand or making eventing simpler as I mentioned above

you'd probably create a custom control that has a button or a link or something that actually has a Command dependency property attached to it

Yes, that would be a fair approach

cmeeren commented 5 years ago

I haven't found a good way of doing this, and the "official" approach is the System.Windows.Interactivity method described by @2sComplement in this comment, so I'm closing this.

TysonMN commented 5 years ago

And the System.Windows.Interactivity DLL is in the Expression.Blend.Sdk NuGet package.

cmeeren commented 5 years ago

I have just installed the System.Windows.Interactivity.WPF package.

TysonMN commented 5 years ago

I tried that one first, but I didn't work for me (before I gave up after one minute of trying). Maybe I did something wrong.

This reminds me. I would like to add more samples, and this is one of the things for which I would like to add a sample. Then Elmish.WPF can have a completely official way to solve this problem :)

cmeeren commented 5 years ago

My bad, you also have to install MicrosoftExpressionInteractions. Check out the new EventBindingsAndBehaviors sample I just added. :)

TysonMN commented 5 years ago

Excellent! I didn't know anything about behaviors, so I learned something new there.

I put a breakpoint in OnDetaching but wasn't able to hit it.

Is there some way to cause OnDetaching to be called in this example?

cmeeren commented 5 years ago

You could use Visibility to hide/show it. You'd have to modify the example. (That might not be a bad idea, actually. Add a button or checkbox that toggles the visibility of the auto-focused textbox, to demonstrate the behaviour. Feel free to create a PR!)

TysonMN commented 5 years ago

I added a button to hide/show the text box. I agree; it is a nice addition. The text box regains focus each time it (re)appears. PR coming soon.

However, OnDetaching still isn't called. Seems like others are also unable to get OnDetaching called.

TysonMN commented 5 years ago

I am not convinced about the use of both System.Windows.Interactivity.WPF and MicrosoftExpressionInteractions NuGet packages in order to obtain System.Windows.Interactivity.DLL.

When I build the sample in Visual Studio, I get the following build warning.

Found conflicts between different versions of "System.Windows.Interactivity" that could not be resolved. These reference conflicts are listed in the build log when log verbosity is set to detailed.

Here is the relevant section of the verbose output.

There was a conflict between "System.Windows.Interactivity, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" and "System.Windows.Interactivity, Version=4.5.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35".
    "System.Windows.Interactivity, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" was chosen because it was primary and "System.Windows.Interactivity, Version=4.5.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" was not.
    References which depend on "System.Windows.Interactivity, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" [C:\Users\twilliams\.nuget\packages\system.windows.interactivity.wpf\2.0.20525\lib\net40\System.Windows.Interactivity.dll].
        C:\Users\twilliams\.nuget\packages\system.windows.interactivity.wpf\2.0.20525\lib\net40\System.Windows.Interactivity.dll
          Project file item includes which caused reference "C:\Users\twilliams\.nuget\packages\system.windows.interactivity.wpf\2.0.20525\lib\net40\System.Windows.Interactivity.dll".
            C:\Users\twilliams\.nuget\packages\system.windows.interactivity.wpf\2.0.20525\lib\net40\System.Windows.Interactivity.dll
    References which depend on "System.Windows.Interactivity, Version=4.5.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" [].
        C:\Code\ThirdParty\Elmish.WPF\src\Samples\EventBindingsAndBehaviors.Views\bin\Debug\net471\EventBindingsAndBehaviors.Views.dll
          Project file item includes which caused reference "C:\Code\ThirdParty\Elmish.WPF\src\Samples\EventBindingsAndBehaviors.Views\bin\Debug\net471\EventBindingsAndBehaviors.Views.dll".
            C:\Code\ThirdParty\Elmish.WPF\src\Samples\EventBindingsAndBehaviors.Views\bin\Debug\net471\EventBindingsAndBehaviors.Views.dll
        C:\Users\twilliams\.nuget\packages\microsoftexpressioninteractions\3.0.40218\lib\net45\Microsoft.Expression.Interactions.dll
          Project file item includes which caused reference "C:\Users\twilliams\.nuget\packages\microsoftexpressioninteractions\3.0.40218\lib\net45\Microsoft.Expression.Interactions.dll".
            C:\Users\twilliams\.nuget\packages\microsoftexpressioninteractions\3.0.40218\lib\net45\Microsoft.Expression.Interactions.dll

This problem persists even after I clear my NuGet cache.

If I use the Expression.Blend.Sdk NuGet package (as I mentioned above) instead of those two NuGet packages, then there are no build warnings and the sample still works. A disadvantage with depending on that NuGet package is that it is a third-party solution hosting Microsoft DLLs.

As of last year, it appears the correct solution is to depend on Microsoft.Xaml.Behaviors.Wpf. This blog post by Microsoft announcing the release of this NuGet package includes migration steps. I tried it, and it worked for me when adding the NuGet package dependency though Visual Studio and also building with Visual Studio. I would create a PR for this change, but I don't know how .paket works (yet).

cmeeren commented 5 years ago

Thanks! Fixed in https://github.com/elmish/Elmish.WPF/commit/5df8514ce4452883a1e417c9f6f479f545339f9e

reinux commented 5 years ago

What about for binding to something like the Window.Closing event? The program has to be able to write to the event args to indicate that it doesn't want to close.

cmeeren commented 5 years ago

I haven't tested, but it seems that it should be possible using the same method. Have you tried?

reinux commented 5 years ago

I've been trying to figure out how it might be done using this mechanism.

Apologies, I'm still new to Elmish.WPF, and I'm having trouble finding specific documentation.

cmeeren commented 5 years ago

Hm, actually, it won't work that way because the mechanism shown above is only for sending messages to the Elmish message loop. There's no way to write to the event args.

I don't have a good solution at the moment. Perhaps you could have the main window be invisible, and simply open a new window using Binding.subModelWin? Then you can control the closing of the window using the Elmish model/messages.

reinux commented 5 years ago

That could work, though I worry that without a reliable way to consume events of all sorts, I'll quickly be roadblocked by some other problem for which there aren't any workarounds.

I've seen this pattern, of having a -ing + -ed (closing, closed etc.) event pair with a mutable Cancel property in the -ing args, in quite a few places.

cmeeren commented 5 years ago

Reopening this issue then. I'm afraid I don't have any solutions at the moment.

TysonMN commented 5 years ago

What about for binding to something like the Window.Closing event? The program has to be able to write to the event args to indicate that it doesn't want to close.

If you want to cancel such an event, why not do it in the code-behind? What does this have to do with the Elmish message loop?

I was considering how to create a text box that only accepts digits. I found a partial solution that involved adding some simple canceling logic to the code behind, which doesn't have anything to do with the Elmish message loop. That seems fine to me since it is preventing state from being created. All (in-process) state would still be stored in the Elmish model.

That could work, though I worry that without a reliable way to consume events of all sorts, I'll quickly be roadblocked by some other problem for which there aren't any workarounds.

I've seen this pattern, of having a -ing + -ed (closing, closed etc.) event pair with a mutable Cancel property in the -ing args, in quite a few places.

While working towards a general solution, it is helpful to consider specific examples that are motivated by actual problems. After solving enough specific cases, it often becomes clear how to solve the general case.

Can you provide a particular -ing + -ed event pair with a mutable Cancel property in the -ing args and a specific problem you are facing with them?

cmeeren commented 5 years ago

If you want to cancel such an event, why not do it in the code-behind? What does this have to do with the Elmish message loop?

I think the core issue is how to use domain logic (i.e. Elmish state) to decide whether to close the window or not. (If one just wants to cancel the closing regardless, code-behind works, but then AFAIK you won't be able to close the app ever.)

I was considering how to create a text box that only accepts digits. I found a partial solution that involved adding some simple canceling logic to the code behind, which doesn't have anything to do with the Elmish message loop. That seems fine to me since it is preventing state from being created. All (in-process) state would still be stored in the Elmish model.

That's one solution, though you could also keep this entirely in your Elmish model. Just have a string field for the raw data, and an update branch for setting this field where any non-digits are stripped away. (The Validation sample may be of inspiration regarding raw/validated data, though it doesn't restrict input in this way.)

While working towards a general solution, it is helpful to consider specific examples that are motivated by actual problems. After solving enough specific cases, it often becomes clear how to solve the general case.

I agree, it would help with examples of several use-cases, including details (such as what decides whether or not to close the window.) I can think of one - if there are unsaved changes, don't close the window, and instead display a warning (which can then be used to close the window).

TysonMN commented 5 years ago

I was considering how to create a text box that only accepts digits. I found a partial solution that involved adding some simple canceling logic to the code behind, which doesn't have anything to do with the Elmish message loop. That seems fine to me since it is preventing state from being created. All (in-process) state would still be stored in the Elmish model.

That's one solution, though you could also keep this entirely in your Elmish model. Just have a string field for the raw data, and an update branch for setting this field where any non-digits are stripped away. (The Validation sample may be of inspiration regarding raw/validated data, though it doesn't restrict input in this way.)

Indeed. This reminds me of the two ways to turn a partial function (e.g. one that throws an exception) into a total function (e.g. one that does not throw an exception). Either the input can be restricted (e.g. to forbid anything but digits) or the output can be expanded (e.g. to a validation monad). By default, my preference is to restrict the input, but both approaches have their place.

cmeeren commented 5 years ago

By default, my preference is to restrict the input, but both approaches have their place.

I completely agree, though if you want raw input validation to be a part of the business logic and validated as it's entered, there's no way around having the raw (normally string) data somehow in the Elmish model - either in addition to the validated value, or with the validated value being calculated on demand from the raw value. The Validation sample has a code comment about this.

reinux commented 5 years ago

While working towards a general solution, it is helpful to consider specific examples that are motivated by actual problems. After solving enough specific cases, it often becomes clear how to solve the general case.

I agree, it would help with examples of several use-cases, including details (such as what decides whether or not to close the window.) I can think of one - if there are unsaved changes, don't close the window, and instead display a warning (which can then be used to close the window).

I'll keep playing with it and report back with some more concrete examples when I find them. I'm porting some AvalonEdit stuff over from vanilla MVVM. Pretty sure I saw some in there.

But yeah, the "Save changes? y/n/cancel" dialog is the example I was thinking of. I also don't want my application to be closed accidentally in the middle of a presentation, so there's a close confirmation for that as well.

In a similar vein, Application.Current.SessionEnding also has a Cancel argument to prevent Windows from shutting down. Not sure how I feel about using that, but I'm sure there are legitimate uses.

RoutedEventArgs in general has a get/set Handled property. The only use cases I can think of off the top of my head can be handled in the UI layer.

Will keep looking.

TysonMN commented 5 years ago

...if you want raw input validation to be a part of the business logic and validated as it's entered, there's no way around having the raw (normally string) data somehow in the Elmish model - either in addition to the validated value, or with the validated value being calculated on demand from the raw value.

Maybe in complete generality like when access to the model is required (such as validating that two text boxes contain the same text...passwords or email addresses) or when event handlers in the code behind are forbidden. In special cases though, I can see an advantage to restricting the input.

Suppose a business requirement is that the user must create a four-digit pin. I like the idea of having a text box that has a max length of 4 (easy to do because of the MaxLength property...we can store 4 on the model and bind it to this property) and only allows digits as input. I think it would be a better user experience to forbid letters than to allow letters while also giving a validation error message. I wasn't able to enforce that restriction though.

Here is a really nice example of how this improves the user experience. I vaguely recall once having to enter a GUID into some text box. The text box was implemented like the example above in that it only accepted digits and letters a to f. The GUID I pasted into it contained dashes (-) but the text box filtered those out and just accepted the digits and letters a to f. I was so impressed that I tried some other cases. When I pasted in all valid characters but too many characters, then it accepted characters up to its limit then ignored the rest.

Of course both of these no-op style behaviors could be implemented by doing a no-op in update, but then we run the (race condition) risk of overwriting what the user is typing. (I recall seeing an issue about that, but I can't find it now.) I would prefer to avoid creating the potential for race conditions. If no-op is often the desired behavior, I would prefer if it were actually a no-op instead of only modeling it as a no-op (cf https://github.com/elmish/elmish/issues/189)

TysonMN commented 5 years ago

@reinux, I haven't created any modal boxes in Elmish.WPF yet. I am considering whether to use MaterialDesignInXamlToolkit, and I hope their modal dialogs are compatible with Elmish.WPF. They are not implemented as separate windows, so it seems promising.

I expect to try this within the next two weeks. I will report back here with my findings.

JDiLenarda commented 5 years ago

That could work, though I worry that without a reliable way to consume events of all sorts, I'll quickly be roadblocked by some other problem for which there aren't any workarounds.

I've seen this pattern, of having a -ing + -ed (closing, closed etc.) event pair with a mutable Cancel property in the -ing args, in quite a few places.

Here’s an idea I can’t test right now :

reinux commented 5 years ago

Here’s an idea I can’t test right now :

* Subscribe to your window `Closing` event with `Program.withSubscription`

* Make it dispatch a message with a `TaskCompletionSource<TResult>` (from `System.Threading`) that it awaits. Depending of its result, you’ll validate or cancel.

* Then set its result in your update function (through a `Cmd` preferably).

I could be doing something wrong, but it hangs in a deadlock waiting for update, which never gets called because Elmish calls update through the WPF message pump as well (I think).

let update msg m =
    match msg with
    | Closing(t) ->     // never makes it to here
        printfn "Update: Closing"
        t.SetResult true
        m

[<EntryPoint; STAThread>]
let main argv = 
    let w = MainEditorWindow()
    Program.mkSimpleWpf init update bindings
    |> Program.withSubscription(fun m -> Cmd.ofSub(fun dispatch -> w.Closing.Add(fun e ->
            let tcs = TaskCompletionSource()
            let t = Task.Factory.StartNew(fun _ -> dispatch <| Closing tcs)
            printfn "Result: %A" t.Result   // hangs here
        )))
    |> Program.runWindow w

OTOH, I think this workaround illustrates the nature of the problem.

In a more realistic example, where we need to know whether or not we even want to display the Save Changes dialog to begin with, we need a dependency property that tells the Window up front:

        public static readonly DependencyProperty PromptToSaveProperty =
            DependencyProperty.Register(
            "PromptToSave", typeof(Boolean),
            typeof(MainEditorWindow)
            );
        public bool PromptToSave
        {
            get { return (bool)GetValue(PromptToSaveProperty); }
            set {
                Console.WriteLine("Set PromptToSave: " + value);
                    SetValue(PromptToSaveProperty, value); }
        }

Bind to it in the Window constructor:

            Binding b = new Binding("PromptToSave");
            this.SetBinding(PromptToSaveProperty, b);

Publish another event for when the user actually wants a file to be saved:

        public event EventHandler RequestSaveChanges;

Handle the Closing event in code-behind to display the dialog box:

        private void MainEditorWindow_Closing(object sender, System.ComponentModel.CancelEventArgs e)
        {
            if(PromptToSave)
                switch(MessageBox.Show(this, "Save changes?", "Closing application", MessageBoxButton.YesNoCancel))
                {
                    case MessageBoxResult.Yes:
                        RequestSaveChanges(this, e);
                        e.Cancel = false;
                        break;
                    case MessageBoxResult.No:
                        e.Cancel = false;
                        break;
                    case MessageBoxResult.Cancel:
                        e.Cancel = true;
                        break;
                }
        }

Bind PromptToSave in the MVU bindings:

let bindings() = [
    "PromptToSave" |> Binding.oneWay(fun m -> m.file.changed)
    ]

Subscribe to RequestSaveChanges in main:

    |> Program.withSubscription(fun m ->
        Cmd.ofSub(fun dispatch ->
            w.RequestSaveChanges.Add(fun _ -> dispatch Save)))

And handle saving in the update function:

let update msg m =
    match msg with
    | Save ->
        save m.document
        m

It's pretty messy, and it relies on the somewhat tenuous justification that displaying the dialog box is a UI activity and thus something that can appropriately be done in code-behind.


Thinking out loud here...

Stripped down to just the core problem, the only downside of this approach is that, as with @dsyme 's original issue, the event handler has to be in code-behind, and that the Window in turn has to publish an event to which the Program can subscribe.

What the dependency property demonstrates, though, is that because it can't wait on the update loop to decide whether or not to cancel, all the parameters needed to make that decision have to be communicated to the Window/control to begin with, and the plumbing to make that happen can get really zig-zaggy, as well as inefficient if calculating those parameters is expensive.

One solution I can think of, and I don't know if this is even possible with Elmish architecture, is to provide a version of Cmd.ofSub which takes a version of Sub<'msg> which is Sub<'msg, 'model'> = 'model -> 'msg -> unit. That would mean at least that there's less in code-behind and more in the program, though it still breaks the single-source-of-truth benefit of MVU.

A cleaner solution, though it would require abandoning the assumption that update is called at most once per WPF message pump iteration, might be to have a version of Cmd.ofSub that offers a blocking version of dispatch that would call update immediately and return the new model, which then could be used to assign to Cancel appropriately. This shouldn't be an issue, since update is supposed to be ~pure~ well-behaved.

cmeeren commented 5 years ago

I am considering whether to use MaterialDesignInXamlToolkit, and I hope their modal dialogs are compatible with Elmish.WPF.

I can confirm they absolutely are. I have used them myself. By the very nature of it, anything normally controlled using view models is compatible. (In any case, Elmish.WPF allows you to control new windows and dialogs now.)

Of course both of these no-op style behaviors could be implemented by doing a no-op in update, but then we run the (race condition) risk of overwriting what the user is typing. (I recall seeing an issue about that, but I can't find it now.) I would prefer to avoid creating the potential for race conditions.

That's no longer an issue in Elmish.WPF as of https://github.com/elmish/elmish/pull/160. So restricting input in domain objects / during update works perfectly.

If no-op is often the desired behavior, I would prefer if it were actually a no-op instead of only modeling it as a no-op (cf elmish/elmish#189)

I'm not sure what throttlig has to do with this, but as long as we're talking about input validation/parsing: If you don't model it in the domain, it's a false sense of security. If you have int in your model, then it can be any int. If at some point or another a value comes from any other place than your length-limited text box, you might have a bug. However, if you define a domain type as type Pin = private Pin of int with a "constructor" (function or static member) that accepts wither int or string and spits out either Pin option or Result<Pin, whatever> which only gives you a pin if it is actually valid (contains four digits), then there is no way to have an invalid Pin instance.

You'd need the model to have the raw input (probably a string), since otherwise the user can't enter anything other than a complete, valid PIN, which is impossible when typing one digit at a time. Then you can have another field that has e.g. Result<Pin, string>, containing a (guaranteed valid) Pin or an error message to display. (Or the Result<Pin, string> could be calculated from the raw value whenever it's needed.)

Designing with types like this and making illegal states unrepresentable is one of the great strengths of the F# type system. I can't recommend strongly enough Scott Wlaschin's series Designing with types. Parts 6 and 7 is of particular relevance here. Indeed, his whole blog has tons of great tips, and his book Domain Modeling Made Functional, which among many other things has even more information on constraining types, is perhaps the best programming book I have read.

Now, you can of course combine this with constraining the raw input. Doing it in the update allows you full flexibility: If you want to allow arbitrary input with error messages, that's fine. If you want to not allow inputs of length greater than 4, just have update return the previous value if the new value is too long. If you want to parse a GUID and remove any slashes, that's simple. On the other hand, doing it in the UI/XAML means you're restricted to what's available with that technology (admittedly anything, but far, far more verbose using behaviors, custom controls, etc.).

JDiLenarda commented 5 years ago

@reinux : I think your problem is the way you use TaskCompletionSource. This works :

type Msg = | Closing of TaskCompletionSource<bool>

let init () = (), Cmd.none

let update (Closing tsc) model =
    let cmd _ =
        async {
            match MessageBox.Show ("Want to close ?", "?", MessageBoxButton.YesNo) with
            | MessageBoxResult.Yes -> tsc.SetResult false
            | _ -> tsc.SetResult true
        } |> Async.StartImmediate

    model, Cmd.ofSub cmd

let bindings () = []

[<EntryPoint;STAThread>]
let main argv = 
    let window = ClosingWindow.Views.MainWindow ()

    let subscription model =
        Cmd.ofSub (fun dispatch ->
            window.Closing.Add (fun e ->
                let tsc = TaskCompletionSource<bool> ()
                dispatch (Closing tsc)
                e.Cancel <- tsc.Task.Result 
            )
        )

    Program.mkProgramWpf init update bindings
    |> Program.withSubscription subscription
    |> Program.runWindow window

All in all, it can be a whole alternative to the use of System.Windows.Interactivity. If it's more satisfying is anyone's opinion, but that may be some ground to work with later.

TysonMN commented 5 years ago

Of course both of these no-op style behaviors could be implemented by doing a no-op in update, but then we run the (race condition) risk of overwriting what the user is typing. (I recall seeing an issue about that, but I can't find it now.) I would prefer to avoid creating the potential for race conditions.

That's no longer an issue in Elmish.WPF as of elmish/elmish#160. So restricting input in domain objects / during update works perfectly. ... I'm not sure what throttling has to do with this...

The issue I couldn't remember was #40 (that you created).

Oh, great. I think I understand now, which I (somewhat) summarized in #112. Of course I knew that #40 had been closed, but I didn't understand (until today) how the race condition you described had been fixed. One of your concerns in #40 was if update takes too long to execute. That is the connection to throttling: if update is taking too long (in aggregate), then one solution is to execute it less often via throttling.

If you don't model it in the domain, it's a false sense of security. If you have int in your model, then it can be any int. If at some point or another a value comes from any other place than your length-limited text box, you might have a bug. ... Now, you can of course combine this with constraining the raw input.

Yes, I completely agree. This reminds me of the fundamental divide that exists between the server and client sides of a web application. There should be validation in the UI to improve the user experience but this validation needs to be repeated on the server because the server can be sent data from someplace other than the standard client. My desire to restrict the characters the user can enter does not replace back-end validation; it is meant to compliment it. As a first order approximation, don't trust anything unless the compiler can prove it! ;)

if you define a domain type as type Pin = private Pin of int with a "constructor" (function or static member) that accepts either int or string and spits out either Pin option or Result<Pin, whatever> which only gives you a pin if it is actually valid (contains four digits), then there is no way to have an invalid Pin instance.

Yes, I also like this design. I first encountered it in Functional Programming in C# (pages 76-7 of section 3.4.5) where author Enrico Buonanno calls it the "Smart Constructor Pattern". I like that name.

I can't recommend strongly enough Scott Wlaschin's series Designing with types. Parts 6 and 7 is of particular relevance here. Indeed, his whole blog has tons of great tips, and his book Domain Modeling Made Functional, which among many other things has even more information on constraining types, is perhaps the best programming book I have read.

Completely agree again! When I started programming professionally in F# a few months back, I immediately bought his book. I am confident that switching to F# is the correct long term move, but I had some anxiety about loss of efficiency in the short term. Change is always hard. When I saw his F# implementation of the smart constructor pattern (on pages 104-106 at the beginning of chapter 6) using a single case discriminated union with a private constructor, I was relieved.

You'd need the model to have the raw input (probably a string), since otherwise the user can't enter anything other than a complete, valid PIN, which is impossible when typing one digit at a time.

Completely agree. A very bad user experience indeed. The user could enter it by pasting it though. :P

Now, you can of course combine this with constraining the raw input. Doing it in the update allows you full flexibility...

Except in performance. To be clear, I will be the first one to say to optimize last (after achieving correctness and clean code). At the same time and especially as the technical lead of a team, I need to be confident that we will have options available to us when we reach the optimization phase of the development cycle. That is why I created the issue https://github.com/elmish/elmish/issues/189 about throttling and am interested to know what will happen when withSyncDispatch is removed #112. Part of my work this sprint is to investigate what performance issues we might have and be aware of some potential solutions.

cmeeren commented 5 years ago

As a first order approximation, don't trust anything unless the compiler can prove it! ;)

👍

where author Enrico Buonanno calls it the "Smart Constructor Pattern". I like that name.

Yep, that's the name I have heard Scott Wlaschin use, too.

At the same time and especially as the technical lead of a team, I need to be confident that we will have options available to us when we reach the optimization phase of the development cycle.

As for Elmish in general and Elmish.WPF in particular: Memoization of domain functions and update/binding helpers can work wonders, as can lazy bindings. Together they allow you to avoid potentially significant amounts of work, making even frequent updates feasible.

And as you say, don't optimize before you need it. Here's a Fable demo I created demonstrating movable boxes, where the mouse position is part of the model (i.e., very frequent updates). Granted, that's React, not WPF, but it shows that the update loop might perform better than you'd expect.

cmeeren commented 5 years ago

For reference, here's a memoization implementation I've used. Perfect for immutable data.

/// Memoizes the last return value by comparing the input using reference equality.
let memoizeOneRef (f: 'a -> 'b) =
  let mutable value = ValueNone
  fun x ->
    match value with
    | ValueSome (arg, ret) when Object.ReferenceEquals(arg, x) -> ret
    | _ ->
        let ret = f x
        value <- ValueSome (x, ret)
        ret

Usage:

// Original, non-memoized function
let myDomainFun x = x + 1

// Memoized version (must be declared in a stable location, i.e. not inline)
let myDomainFunMemoized = memoizeOneRef myDomainFun

// Note - do not do this, since memoizeOneRef will be applied separately
// each time it's called
// let myDomainFunMemoized x = memoizeOneRef myDomainFun x

// Use the memoized function just like you would the non-memoized one
let x = myDomainFunMemoized 2

If you need multiple parameters, either use tuples or write separate higher-arity memoization functions. Note however that memoization functions can be incorrectly used - you can pass a multi-parameter function to memoizeOneRef without compilation errors, and end up with a memoized function return value that memoizes based on the first argument, which for the most part would be incorrect. (In that case, the 'b in 'a -> 'b would simply be some function type.)

reinux commented 5 years ago

@JDiLenarda

I think your problem is the way you use TaskCompletionSource. This works :

Ah ha! Thanks, that does indeed work.

Goes to show how little I know about Elmish still.

reinux commented 5 years ago

Couple thoughts...

cmeeren commented 5 years ago

@reinux Could you share some examples of how you imagine the usage to be according to your suggestions?

reinux commented 5 years ago

For the first one:

"wstate" |> Binding.subModelWin (
    (fun m -> m.wstate),
    bindings,
    (fun dispatch ->    // Pass in a dispatch function -- can this be done?
        let w = Window ()
        w.MouseWheel.Add (fun e ->
            dispatch <| VolumeUpDown (e.Delta)
        )
        w
    )
)

Being able to see the model from that function right after init could be useful too, though I'm not sure if that might be too conceptually polluting.

The second one:

<b:Interaction.Triggers>
    <b:EventTrigger EventName="MouseWheel">
        <b:CallMethodAction MethodName="MouseWheel" />
    </b:EventTrigger>
</b:Interaction.Triggers>
let bindings () = [
    "MouseWheel" |> Binding.method (fun e m ->
        VolumeUpDown ((e :?> MouseWheelEventArgs).Delta)
]

The usage would be more or less the same as cmdParam and <b:InvokeCommandAction />, but it would allow the actual EventArgs to come through rather than something bound via CommandParameter.

Part of the reason I'm interested in this is that Program.withSubscription is easy enough to work with for the initial window, but after that, there isn't a very straightforward way to bind more handlers.

cmeeren commented 5 years ago

Great suggestions!

My immediate thought on having getWindow accept both dispatch and the current model is that it seems simple, both conceptually and from an implementation standpoint. I'll need to think on it some more.

As for the CallMethodAction stuff: It might be possible, but it would help to have more use-cases that can't be easily solved other ways. Do you have some (more) examples where you can't use e.g. cmdParam?

reinux commented 5 years ago

Do you have some (more) examples where you can't use e.g. cmdParam?

If there's a way to bind the EventArgs of an event to CommandParameter in Microsoft.Xaml.Behaviors, preferably without having to tinker in code-behind, I think that would suffice. There's a good chance I'm just not familiar enough with WPF Commands to know, though I suspect it would at least be a convoluted solution.

Of course, if subModelWin can dispatch messages, that works just as well too, from a practical standpoint.

In general, I think any event that has numeric parameters that need to be processed along with the model (i.e. in update) can benefit from having the EventArgs being exposed to bindings -- so retrieving the position of a mouse click/tap would be another one, say, in a drawing app.

Will get back to you if I can think of more.

cmeeren commented 5 years ago

If there's a way to bind the EventArgs of an event to CommandParameter in Microsoft.Xaml.Behaviors, preferably without having to tinker in code-behind, I think that would suffice. There's a good chance I'm just not familiar enough with WPF Commands to know, though I suspect it would at least be a convoluted solution.

As far as some cursory searching reveals, there is no built-in way to do this, but it might be possible to create a reusable trigger that passes the event args as command parameters. See e.g. MVVM Light's EventToCommand.

A significant drawback of this method is that you'll have to do unsafe casting of the event args.

Of course, if subModelWin can dispatch messages, that works just as well too, from a practical standpoint.

Yes, I think that would work best, because everything would be strongly typed. Your windows could surface the controls using x:Name, and getWindow could subscribe to the relevant events and dispatch necessary messages.

reinux commented 5 years ago

See e.g. MVVM Light's EventToCommand.

Cool, will check that out :D

For now, I've resorted to an unholy workaround, specifying the ID associated with a Window by passing it via a global mutable (with the assumption that init and bindings are both called from the UI thread), and having another global function that relays messages to Program.withSubscription.

Nasty, and a little precarious, but I think it'll do for now.

cmeeren commented 5 years ago

@reinux can you give the subModelWin-args branch a try? I have added model and dispatch as parameters to getWindow. Would be great to see if it solves your problems before I release anything.

reinux commented 5 years ago

@cmeeren Works great! Thanks for taking the time to do this.

One thing I noticed: I haven't come across a situation where this is an issue yet, but Program.withSubscription doesn't have access to the initial model the way subModelWin does, so there's a bit of a disparity there. A simple workaround would be to memoize init and have that in higher scope, e.g. in main (since init is a function, it won't necessarily return the same instance).

Scratch that, not sure why I didn't notice withSubscription passes in a 'model.

By the way, this branch reacts slightly differently to what I think is a bug: with the latest nuget package, if you return a Cmd.ofMsg from init to update some stuff, the model reverts back to the initial state before getting updated again.

I've been trying to repro it with a simpler example, but I haven't been able to.

With this branch, trying to do the same thing causes a null reference exception trying to access Application.Current, which kind of makes more sense and is more informative, since I have some stuff in init that probably needs access to Application. Newing an Application first solved the problem.

cmeeren commented 5 years ago

What a strange bug. AFAIK there should be no difference in that regard between the current nuget and the branch. Particularly since nothing in the Program module hasn't been changed in a while. Have you tried thoroughly cleaning between trying the nuget and trying the branch? (Including deleting the .vs folder.)

Please let me know if you can arrive at a somewhat minimal repro, or figure out what it is.

Also, may I ask what you're doing in init that requires Application? Just curious.