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

Binding Collections #5

Closed
gchiaramonte opened this issue Jun 25, 2017 · 36 comments
Closed

Binding Collections #5

gchiaramonte opened this issue Jun 25, 2017 · 36 comments

Comments

@gchiaramonte
Copy link

If my model contains a list and I have Add and Remove messages to add, how would I bind the list to a grid or listview in the view? Thanks.

@2sComplement
Copy link
Collaborator

2sComplement commented Jun 25, 2017

You should be able to simply use Binding.oneWay and return your F# list from your model. The view will be notified of any changes to this list, so you don't need to use ObservableCollection. Not sure why you would bind a list to a Grid but binding to a ListView or ItemsControl should work.

Let me know if you have issues doing it that way. I'll keep this open until I can add an example of this type of binding.

@ReedCopsey
Copy link

Just FYI... that's not practical with large collections, or fast updates. Recreating view for entire collection can be half second+ UI lockups. Spent quite a bit of effort into making it work in gjallarhorn with a custom INotifyCollectionChange mechanism from a normal f# list because of that.

@isaacabraham
Copy link

I had a play around with using ObservableCollection<_> on my model today. Of course doing this means breaking the idea of an immutable model. A custom INotifyCollectionChange mechanism would probably be the preferred way - rebinding a big list repeatedly kills performance in WPF.

@BentTranberg
Copy link

BentTranberg commented Jul 5, 2017

I am having so much fun right now. GUI was never this easy. (Sorry about the bad indentation here.)

<ListView ItemsSource="{Binding Reports}" SelectedItem="{Binding SelectedReport}" SelectionMode="Single" IsSynchronizedWithCurrentItem="True">

and

"Reports" |> Binding.oneWay (fun m -> m.Reports) "SelectedReport" |> Binding.twoWay (fun model -> match model.SelectedReport with Some report -> report | None -> { Name = "" }) (fun report model -> (if report.Name <> "" then Some report else None) |> SelectReport)

@2sComplement
Copy link
Collaborator

👍 🍺

@Kurren123
Copy link

Is this still an unsolved problem? If there's no one having a look at it maybe I can investigate?

@2sComplement
Copy link
Collaborator

Binding to a list or array in your model does work. There are a few things I wanted to track with this issue:

Not sure if there's anything else that I'm missing but feel free to investigate as you like!

@Kurren123
Copy link

Thanks @2sComplement . In terms of performance, I'm guessing to only add/remove the relevant items from the ItemsControl rather than rebuild the whole thing we're going to need to diff the updated collection with the collection before the update. Will this be inefficient with just a standard List, do you think we'll need to use another collection for diffing? I'm still new to the internals of model view update so forgive my lack of knowledge.

@2sComplement
Copy link
Collaborator

@Kurren123 since the model's list is immutable, we'd need to do something behind the scenes to bind to an ObservableCollection, or as mentioned above some custom INotifyCollectionChange implementation. TBH though I have not determined the limitations of the current implementation's performance.

I added the oneWayMap binding when playing with a graphing library where I was plotting data from the model at a relatively high frequency. In this case, the viewBinding map took my model's array and returned the graphing library's Series type (which was mutable), and that seemed to perform quite well. The idea behind oneWayMap is that the collection used for performing a diff does not go through any extra processing than it needs to until there is an actual change.

My goal with the performance sample was to determine whether or not this is a critical problem to fix, so I think that would be the first step before making any changes.

@bcachet
Copy link

bcachet commented Jul 20, 2017

Maybe we can learn from https://github.com/ReedCopsey/Gjallarhorn/blob/master/src/Gjallarhorn.Bindable/CollectionBinding.fs and create something similar.

@2sComplement
Copy link
Collaborator

Definitely Gjallarhorn will be a good reference for this.

@2sComplement
Copy link
Collaborator

Closing this until performance becomes an issue for someone.

@isaacabraham
Copy link

Could I suggest leaving this open with an up-for-grabs label? Then if someone encounters this in the future they'll see it immediately

@2sComplement
Copy link
Collaborator

Sure, will do.

@rchybicki
Copy link

Just a heads-up for anyone needing an editable datagrid, to keep the model immutable and the datagrid editable you need a mutable collection in between (so that wpf allows adding and deleting), on my grid with a few hundred cells with highlighting on validation on a fast computer the reload after an edit takes around 1s to 2s, I'm switching my project to gjallarhorn hoping for faster edits.

@2sComplement
Copy link
Collaborator

Interesting, I haven't tried an editable datagrid so thanks for pointing this out. Will give it a try and let you know if I can make any improvements.

@ReedCopsey
Copy link

In general, I would recommend avoiding datagrid for editing. With immutable data, the schema will already be known, so a normal itemscollection works fine, and let's add/remove be handled via messages.

@TheJayMann
Copy link
Contributor

This has become an issue for me recently. Not a performance issue, as everything appears to load quickly enough. Rather, it is a visual state issue.

I have a collection of items, where each item has a collection, and each of those items has a collection, and is visualized as a treeview. The issue I have is that each time I remove a leaf item, due to immutability, the entire tree is replaced. When this happens, all treeview items are removed and replaced with new treeview items. Again, this happens quickly enough, however, the treeview resets to it's completely collapsed mode, as well as the cursor position is forgotten. Given that the tree is rather large and spread out, it can be a bit time consuming to get back to the place the previous item was, just to then possibly delete another few items closely positioned.

I had an idea for dealing with this similar to the concept of event sourcing. The idea would be to create a list wrapper, which would be a record containing a list representing the current state, as well as a list of actions which have occurred to a list in order to create the current list, such as the list being cleared, items being appended or prepended to the list, an item being inserted at a specific position, or an item being removed from a specific position. Most of the functions available in the List module could be reimplemented for this type, such as filter, append, etc, which would perform same the operation on the list as well as creating the appropriate event items. This type could then be treated specially by the view model code such that it raises the INotifyCollectionChanged.CollectionChanged event. Assuming someone else doesn't start it, I could possibly come up with this new type, with some appropriate functions to use it. Getting it to work properly with an INotifyCollectionChanged will likely require a bit more effort.

@2sComplement
Copy link
Collaborator

I think you will have to track the tree state in your model. For example, each node in the tree would have an isExpanded property. That way when the tree is re-rendered, it retains its visual state. Your idea of a wrapper would work here on the item level rather than the list level. For example:

type Expandable<'a> =
    { isExpanded: bool
      item: 'a }

Then you have a list of Exandables in your model.

@TheJayMann
Copy link
Contributor

The problem with this suggestion is that there is no easy way to transfer the expandedness of the treeview item back into the model except by making it mutable, or possibly hooking into an expanding event allowing it to be updated in the update function.

For now, I will try the mutable expandable wrapper path.

@et1975
Copy link
Member

et1975 commented Apr 17, 2018

We use "optics" for ease of manipulation on immutable hierarchies. Check out aether for example.

To elaborate, the "elmish way" implies that all of your visual state (including cursor position, what's collapsed, highlighted, etc) is captured in your model. Think what it means for something like time-traveling debugger (not that we have one for WPF, but it's possible to add one).

@TheJayMann
Copy link
Contributor

Optics do not solve this issue. Optics (from what I've read) make it easier to make deep change without having to recreate everything back up to the top. I do not need to solve this problem, as I already have that working. The issue is how to get the current collapsed state of the individual treeview item mapped back into the model. There is no real way to use databinding to get the value back into the viewmodel. I can make a binding for the collection itself, and even make it two way. However, I cannot make bindings for the individual items of the collection, and, if I make the collection binding two way, as well as make the binding to the treeview item's IsExpanded property two way, when I attempt to expand a treeview item, I get an exception stating that a two way binding cannot be made to the read only property of IsExpanded, as I expected.

Thus, I do not need to figure out how to update a deeply nested property with a new value for an immutable object. Rather, I need a way to get that new value from WPF so that I can actually update the deeply nested value. Either that, or I need a way for Elmish to notify WPF which items specifically were removed so that WPF can simply maintain it's state without having to reflect it in the application state, which goes back to the initial point of the original topi of this issue.

@rchybicki
Copy link

Had the same problem, had to go with a combination of immutable model and mutable VMs for collections because of performance. The real solution is what Gjallarhorn has, a model collection binding, like the model binding but for collections, that internally supports the CollectionChanged event.

@2sComplement
Copy link
Collaborator

I understand your problem now and agree that we need a new binding type. I will explore some options where ideally the underlying elmish model can stay immutable and idiomatic.

@TheJayMann
Copy link
Contributor

As I was thinking about it, even adding support for differenced collections wouldn't be enough for my situation, as that would really only allow for a top level collection not contained within another collection.
Really what would be needed would be some form of Bindings.modelSeq which would allow further defining view bindings directly for individual items of a collection. This would be required for my case whether I were to use a differencing collection type as I described above, or whether I were to two way bind the IsExpanded property as suggested to me.

@2sComplement
Copy link
Collaborator

Yup that's exactly what I had in mind as well. View bindings all the way down!

@ReedCopsey
Copy link

That's the approach I used in Gjallarhorn. Solves these issues nicely.

@JDiLenarda
Copy link
Contributor

Hi people,

Ignoring this issue and that you are working on it, I wrote a Binding.collectionTwoWay function :

collectionTwoWay (getList: 'model -> Map<string,'a>) (getItem: 'model -> string -> 'a) (setItem: 'model -> string -> 'a -> 'msg) name : ViewBinding<'model,'msg>

First parameter is a function creating a dictionary from model.
Second one get an item for the the given key.
Third one set an item for the given key.

It allows to display a list in 2-way mode, or to access element of a list through its index or key. The list implementation in your model can be whatever you want, you'll just have to provide a function to convert it to a Map<string,obj> (if you want to work with a list, convert its indexes to string). In your XAML file, you have to refer to a Value property (Key is allowed too). On the other hand, no need to work with ObservableCollection, BindingList, INotifyPropertyChanged.

It works by creating some DynamicObject :

  • for each list, one object with GetEnumerator, TryGetIndex, TryGetIndex
  • for each item, one object with TryGetMember, TrySetMember that respond to "Key" and "Value"
  • only Elmish.WPF code is modified.
  • it is fresh, barely tested.

Let me know if you are interested and how to submit my work properly. This is my first attempt to contribute to a Github project, I hope I don't mess up with etiquette.

@TheJayMann
Copy link
Contributor

The first thing you will want to do is find the fork button at the top of the page. This will create a complete copy of this repository into your own list of repositories. Then, you can clone your github repository, and make your changes to that clone. Once you believe you are done, you can push your commits to your repository, then create a pull request. This will create a pull request issue on this repository, were a discussion about the code changes can take place, and, ultimately, the changes will either be merged in to this repository, or they will be cancelled. If you need to make further changes after making the pull request, simply pushing updates into your repository will automatically update the pull request.

@cmeeren
Copy link
Member

cmeeren commented Aug 1, 2018

@JDiLenarda I'd love to see your code, even if you don't make a pull request.

@2sComplement
Copy link
Collaborator

2sComplement commented Aug 2, 2018

@cmeeren You can check it out here: #38

@JDiLenarda
Copy link
Contributor

@cmeeren : there’s a pull request, though to be fair my claims were a bit premature (as much as my initial pull request). I’ve dropped this for performance reason in favor of a model collection that rely on ObservableCOllection. If you want to see this initial work check for earlier commits from the pull request. Give me feedback !

@cmeeren
Copy link
Member

cmeeren commented Aug 8, 2018

Inspired and helped by the work by @JDiLenarda I have a more or less working prototype of a Binding.modelSeq function that seems to work ideally:

  • All ObservableCollection stuff is handled internally
  • The Elmish model is immutable
  • No losing focus on changes
  • All edits to the ObservableCollection are done on the UI thread, meaning user's won't have to think about that

The signature is similar to @JDiLenarda's complexModel, but also takes a getId: 'subModel -> 'a parameter that is used to identify models (which should contain a unique identifier). The value returned by the getId function is naturally also available in the toMsg function so the update function can know which submodel it should update.

There are a last few bugs to iron out and some more testing to do, but I hope to be able to make a PR some time during the week or weekend.

@JDiLenarda
Copy link
Contributor

@cmeeren : something’s happening, at last ! Can’t wait to see your work.

@rchybicki
Copy link

@cmeeren 👍 Same here, good luck!

@cmeeren
Copy link
Member

cmeeren commented Aug 14, 2018

Work finished, let's find a good way forward: #46

@cmeeren cmeeren mentioned this issue Aug 25, 2018
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests