michaellperry / Assisticant

MIT License
36 stars 19 forks source link

Is there a way to force an update on a computed in Assisticant? (order of things happening is important) #31

Closed smasihemami closed 4 years ago

smasihemami commented 4 years ago

Hello Michael, Thanks for your recent fixes. I just pushed an example code to my fork of assisticant (https://github.com/smasihemami/Assisticant/tree/ListSlot) to be able to better discuss my current issue with you. In the example, on changing the sample id definition mode (either defined by first x number of characters or characters preceding the first "_" character in the file name), the list of sample IDs shown in the CheckedComboBox changes. Hence the number of Sample IDs changes. On this change, all the items in the CheckedComboBox should be initially selected. This happens on startup of the Storyboard application, but on changing the Sample ID definition mode, it does not happen, although I re-populate the list of selected items (line 25 in StoryboardParametersModel.cs). As far as I remember from my old way of doing it with Prism, I was setting IsSelectAllActive to false before changing the SelectedItemsOverride observable collection (in this example, it's called SelectedSampleIDs) and back to true right after finishing, so that CheckedComboBox synchronizes its internal state with SelectedItemsOverride collection. But with Assisticant, I'm not sure how to do that. Is there a way to force an update by Assisticant on a specific computed? Or better say, is there a way to control when a computed gets updated?

michaellperry commented 4 years ago

If you set a breakpoint inside of the Remove method of ObservableList, you will see that the control itself is removing the selected items after you have added them. The control is removing the selected items in response to the items being removed from the list, which is happening because the list is updating.

This is a pretty common behavior of controls that I have fought several times. They make no distinction between an update from the view model and an action that the user has taken. The Assisticant philosophy is that input is always from the user, and output is always toward the user. Output must never cause an input side-effect.

Whenever a control violates this philosophy, Assisticant has to protect itself. Take a look at the _updating field in BindingListSlot to see an example. It looks like ListSlot will need something similar. The wrinkle is that the control is modifying the ListSlot because a CollectionSlot bound to a different property is updating. Let me see if I can find a good solution here.

By the way, the way that you are modifying an observable within a subscription is a similar violation. User input should be the only thing that modifies an observable. A subscription is a form of output. A better solution would be to add all sample IDs right after the SampleIDDefinitionModel setters. That would keep all of the observable changes directly attributable to user input and prevent any dependency issues. In this case, I don't think it's causing any problems, but just something to look out for.

michaellperry commented 4 years ago

I think I have a solution. All of the gates currently in the system protect a single property. In most cases, this was sufficient. It protected against controls that would read a value, modify it in some way (for example apply formatting rules), and then write it back. But in this case, we have an interplay between two properties of the same control.

The solution I am considering is to add a gate to the ViewProxy itself. If any property of the proxy is currently notifying its control, then all inputs are ignored. This should cover our situation.

By the way, you can control the order of notification using the NotifyAfter attribute. This might be a necessary part of the solution. Apply [NotifyAfter("AllSampleIDs")] to the SelectedSampleIDs property. I don't think this is sufficient, but it might turn out to be necessary.

michaellperry commented 4 years ago

I created a pull request against your fork. Please try this and see if you can identify any more problems. If not, I'll publish this change so you can have it in the feed.

Thanks.

smasihemami commented 4 years ago

That deserves a huge wow Michael. And seems like it's now working smoothly. I'm learning a lot from you and I am highly grateful of your time and pieces of advice 👍 I'll try to develop more and will open new issues if I identified more problems.

michaellperry commented 4 years ago

The fix is published in version 1.5.6