JordanMarr / ReactiveElmish.Avalonia

Static Avalonia views for Elmish programs
Other
92 stars 8 forks source link

IsAutoUpdating bool not triggering state of subscription? #19

Closed houstonhaynes closed 1 year ago

houstonhaynes commented 1 year ago

I'm in the process of following your suggestion and added "isAutoUpdating" from my earlier PR to a model.IsAutoUpdating bool value. I've done all of the homework down to modifying the logic in the subscription to look at model.IsAutoUpdating and while it compiles and runs the chart doesn't update when the AutoUpdate button is triggered. I'm wondering if this is related to the eagerness of F#'s evaluation - and should this take a different form. Thoughts? Thanks!


type Model = 
    {
        Series: ObservableCollection<ISeries>
        Actions: Action list
        IsAutoUpdateChecked: bool
        IsAutoUpdating: bool
    }

type Msg = 
    | AddItem
    | AddNull
    | RemoveItem
    | UpdateItem
    | ReplaceItem
    | Reset
    | AutoUpdate
    | SetIsAutoUpdateChecked of bool
    | Ok

let rec init() =
    {
        Series = 
            ObservableCollection<ISeries> 
                [ 
                    ColumnSeries<DateTimePoint>(Values = newSeries(None), Name = "Luck By Second")
                    :> ISeries 
                ]
        Actions = [ { Description = "Initialized Chart"; Timestamp = DateTime.Now } ]
        IsAutoUpdateChecked = false
        IsAutoUpdating = false 
    }

let update (msg: Msg) (model: Model) =
    let values = model.Series[0].Values :?> ObservableCollection<DateTimePoint>
    match msg with
   // [ ... ] 
    | AutoUpdate ->
        // toggle the isAutoUpdating to switch the autoUpdateSubscription behavior
        match model.IsAutoUpdating with
            | false ->
                { model with
                    IsAutoUpdating = true
                    Actions = model.Actions @ [ { Description = $"Is Auto Updating: {model.IsAutoUpdating}"; Timestamp = DateTime.Now } ]
                }
            | _ ->
                { model with
                    IsAutoUpdating = false
                    Actions = model.Actions @ [ { Description = $"Is Auto Updating: {model.IsAutoUpdating}"; Timestamp = DateTime.Now } ]
                }

let bindings ()  : Binding<Model, Msg> list = [
    "Actions" |> Binding.oneWay (fun m -> List.rev m.Actions)
    "AddItem" |> Binding.cmd AddItem
    "RemoveItem" |> Binding.cmd RemoveItem
    "UpdateItem" |> Binding.cmd UpdateItem
    "ReplaceItem" |> Binding.cmd ReplaceItem
    "Reset" |> Binding.cmd Reset
    "AutoUpdate" |> Binding.cmd AutoUpdate
    "IsAutoUpdateChecked" |> Binding.twoWay ((fun m -> m.IsAutoUpdateChecked), SetIsAutoUpdateChecked)
    "Series" |> Binding.oneWayLazy ((fun m -> m.Series), (fun _ _ -> true), id)
    "XAxes" |> Binding.oneWayLazy ((fun _ -> XAxes), (fun _ _ -> true), id)
    "Ok" |> Binding.cmd Ok
]

let designVM = ViewModel.designInstance (init()) (bindings())

open System.Timers

let subscriptions (model: Model) : Sub<Msg> =
    let autoUpdateSubscription (dispatch: Msg -> unit) = 
        let timer = new Timer(1000) 
        timer.Elapsed.Add(fun _ -> 
            if model.IsAutoUpdating then
                // similar to newSeries create null entry in 1% of cases
                let randomNull = rnd.Next(0, 99)
                match randomNull with
                | i when i = 0 ->
                    dispatch AddNull
                | _ -> dispatch AddItem
                dispatch RemoveItem
        )
        timer.Start()
        timer :> IDisposable

    [
        [ nameof autoUpdateSubscription ], autoUpdateSubscription
    ]
houstonhaynes commented 1 year ago

Hold the phone - IsAutoUpdating is not being set... let me spend a bit more time with it in the morning with a fresh pair of brain cells. šŸ˜

JordanMarr commented 1 year ago

I think you need to move the condition into the list comprehension:

let subscriptions (model: Model) : Sub<Msg> =
    let autoUpdateSubscription (dispatch: Msg -> unit) = 
        let timer = new Timer(1000) 
        timer.Elapsed.Add(fun _ -> 

            // similar to newSeries create null entry in 1% of cases
            let randomNull = rnd.Next(0, 99)
            match randomNull with
            | i when i = 0 ->
                dispatch AddNull
            | _ -> dispatch AddItem
            dispatch RemoveItem
        )
        timer.Start()
        timer :> IDisposable

    [
        if model.IsAutoUpdating then
            [ nameof autoUpdateSubscription ], autoUpdateSubscription
    ]
houstonhaynes commented 1 year ago

This is brilliant! Thanks. I was a bit sheepish about where that might go - and grokking the role of the list comprehension helps.

Just to be pedantically clear, the original version of this with the let mutable isUpdating the loop was always "running" - i.e. the loop starts as the view initializes. Is this correct?

And if I'm reading this correctly, this has now changed with the if within the list comprehension such that the loop "starts from a cold start" when model.IsAutoUpdating is set to true.

So this in effect actually makes the app comparatively more efficient as well as more "pure" by keeping IsAutoUpdating in the model, correct?

houstonhaynes commented 1 year ago

I'm seeing this "odd" behavior - where the actual behavior of the button and chart are correct - but the state of IsAutoUpdating is remaining "True" after the control is deactivated. I'm missing something in the logic flow here. Maybe a change is needed to the binding type that I don't quite grok?

Screenshot 2023-08-02 at 7 59 02 AM

houstonhaynes commented 1 year ago

This works but I feel like the logic seems "iffy". šŸ˜Š šŸ™„

| AutoUpdate ->
        let autoUpdateStatus = not model.IsAutoUpdating
        { model with
            IsAutoUpdating = autoUpdateStatus
            Actions = model.Actions @ [ { Description = $"Is Auto Updating: {autoUpdateStatus}"; Timestamp = DateTime.Now } ]
        }

Am I reading this right that this is actually the DataGrid getting this update "before" the autoUpdateSubscription is triggered? This likely requires more depth of understanding for AvaloniaProgram.withSubscription than I have right now. I'm honestly not sure how much order-of-operations matters as the "behavior" seems correct.