fsprojects/FSharp.ViewModule

CollectionChanged not triggering the PropertyDependencies of a Factory.CommandSyncChecked

giuliohome opened this issue · 5 comments

Description

I'm trying to detect when an ObservableCollection is not empty and enable a clear command only in that case

Repro steps

I've tried the following

    let clearRecentsCmd = 
        self.Factory.CommandSyncChecked(
            (fun () -> self.ClearRecents()), 
            (fun () -> self.RecentsNotEmpty  ), 
            [ <@@ self.RecentsNotEmpty @@>;  <@@ self.Recents @@>])
    let recents = self.Factory.Backing(<@ self.Recents @>, ObservableCollection<RecentFile>(getRecent())) 
    do
        self.DependencyTracker.AddPropertyDependencies(<@@ self.RecentsNotEmpty @@>, [ <@@ self.Recents @@> ; ])
    member x.Recents with get() = recents.Value and set value = recents.Value <- value
    member x.Add2Recents r = 
        x.Recents.Insert(0,r)
    member x.ClearRecents() =
        x.Recents.Clear()
    member x.RecentsNotEmpty with get() = x.Recents.Count > 0

Expected behavior

x.Recents.Insert(0,r) should raise a Command CanExecuteChanged

Actual behavior

The command doesn't get enabled if I add a new RecentFile to the empty ObservableCollection.

Known workarounds

I'm forced to use the clumsy x.Recents <- ObservableCollection(r :: (x.Recents |> Seq.toList)) instead of x.Recents.Insert(0,r)

Related information

I'm using .Net Framework 4.6.1 and F# 4.4.3

Notice that the issue doesn't get fixed even if I use the simpler

let recents = ObservableCollection<RecentFile>(getRecent()) 
\\...
member x.Recents = recents

Found a better workaround

    let recentCollectionChanged = self.Factory.Backing(<@ self.RecentCollectionChanged @>, false) 
\\...
    do
        recents.CollectionChanged.Add  (fun _ -> 
            self.RecentCollectionChanged <- not self.RecentCollectionChanged )
        self.DependencyTracker.AddPropertyDependencies(<@@ self.RecentsNotEmpty @@>, [ <@@ self.Recents @@> ; <@@ self.RecentCollectionChanged @@> ])
\\...
    member x.RecentCollectionChanged with get() = recentCollectionChanged.Value and set value = recentCollectionChanged.Value <- value

now I can use the x.Recents.Insert(0,r)

I've prepared a github mvce here

it's also enough a simple, old style

        recents.CollectionChanged.Add  (fun _ -> 
           self.RaisePropertyChanged("RecentsNotEmpty") )

but I think that should be accomplished in a new, different way with some sort of self.DependencyTracker.AddPropertyDependencies ?

This is expected behavior. ObservableCollection doesn't raise INotifyPropertyChanged.PropertyChanged when you add/insert - it uses INotifyCollectionChanged instead.

I'd be open to a PR that allowed an "AddCollectionDependency" similar to AddPropertyDependencies which could be made to allow this to work, but without that, the best option is probably something similar to your workaround. However, you could simplify this - there is no need for the RecentCollectionChanged. If you feel this would be very useful, feel free to raise an issue as a suggestion for that to happen and/or submit a PR.

I'm going to close this, but I'll also submit a PR against your MVCE showing how I would recommend approaching it.

I don't think the PR should allow a new "AddCollectionDependency" because the dependent to which we are adding dependencies is still a property. One of the dependencies is indeed a collection but in either case you are subscribing an observable interface to raise the property changed event of the dependent property. The problem is where the observer pattern is publishing notifications. That happens in the backing notifying value for a property dependency. I guess that a similar backing is missing for a collection dependency.
Anyway I'm not even sure how you would be open to a PR if you're closing the underlying issue.

Btw I don't think it is fundamental to add the enhancement and I will reshape my project so that it won't use a Collection changed event. That is because I prefer refreshing the whole collection from the db. But generally speaking one could have different opinions and/or should account for in memory management too...