JordanMarr / ReactiveElmish.Avalonia

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

Binding/update issue with ReactiveElmishViewModel #41

Closed TeaDrivenDev closed 8 months ago

TeaDrivenDev commented 8 months ago

Using setters of ReactiveUI-style notifying properties in a class deriving from ReactiveElmishViewModel does not cause UI updates.

member this.Progress
    with get () = progress
    and set value =
        this.RaiseAndSetIfChanged(&progress, value) |> ignore

Changing the class to inherit from ReactiveObject instead makes the updates work as expected; thus my suspicion is that this may have do with the fact hat ReactiveElmishViewModel implements INPC by itself, rather than using the existing implementation in the ReactiveObject base class.

Code demonstrating the issue is at https://github.com/TeaDrivenDev/Shoo/blob/ReactiveElmishViewModel_BindingIssue/src/Shoo/ViewModels/MainWindowViewModel.fs

JordanMarr commented 8 months ago

You are correct. I did that’s as a workaround in order to implement OnPropertyChanged, even though I knew it was a bit of a hack, but was just trying to get it done.

JordanMarr commented 8 months ago

That turned out to be an easy fix. Don't know how I missed that the first time through.

v1.0.2 should fix this so it uses ReactiveObject INPC instead.

JordanMarr commented 8 months ago

It looks like you are fully embracing ReactiveUI, which is nice to have the option.

I did update the BindSourceCache method to have two optional map and sortBy arguments. I wonder if that would work for your solution?

See the Todos in the sample project:

type TodoListViewModel() =
    inherit ReactiveElmishViewModel()

    let store = 
        Program.mkAvaloniaProgram init update
        |> Program.mkStore

    member this.Todos = 
        this.BindSourceCache(
            store.Model.Todos
            , map = fun todo -> TodoViewModel(store, todo)
            , sortBy = fun todo -> todo.Completed, todo.Description
        )
    member this.AddTodo() = store.Dispatch AddTodo
    member this.Clear() = store.Dispatch Clear

    static member DesignVM = new TodoListViewModel()
TeaDrivenDev commented 8 months ago

I haven't used those here because I need access to the intermediary IObservable<IChangeSet<_, _>> to also feed the move queue. I think I know a way to do both; I'll have to have a look tomorrow at how I like that.

TeaDrivenDev commented 8 months ago

I did update the BindSourceCache method to have two optional map and sortBy arguments. I wonder if that would work for your solution?

Turns out they're too high level in this case, as the map uses .Transform(), but I need .TransformWithInlineUpdate(). So what I'd need I suppose is a more general transformation IObservable<IChangeSet<'TValue, 'TKey>> -> IObservable<IChangeSet<'TTransformedValue, 'TKey>> that plugs in between the .Connect() and .Bind() (or .Sort()) and gives more control over the process.

JordanMarr commented 8 months ago

Btw, did v1.0.2 resolve your issue with regards to the INotifyPropertyChanged implementation?

TeaDrivenDev commented 8 months ago

It did, thanks!

Closing this; I'll open a discussion about support for more advanced DynamicData/Rx sometime.