Derived collections onRemoved handler #744
Conversation
…n an item is removed
|
onRemoved should probably also be called for each item when the collection resets |
|
Yes, that is handled, since Reset calls internalClear which is one of the places where onRemoved is called. I just forgot to make a test for that case. |
|
Cool, didn't see that, sorry! |
There was a problem hiding this comment.
And here is the reason I wish the code style options were part of the project/solution settings rather than the user settings. I caught some of them, but not all.
|
Oops, ok, I'll fix that |
There was a problem hiding this comment.
We could probably simplify a lot of this code by just writing:
this.onRemoved = onRemoved ?? (() => {});
|
Added a few notes, but this generally looks good |
|
@TheGrandUser Don't close this! Just make more commits |
|
Oh, I was not aware that was how it worked, being a bit new to git. I've made the new commits. |
|
@TheGrandUser No worries, it's a common misconception that you have to close and resubmit the PR :) |
|
Great, thanks @TheGrandUser! |
|
Great, hope to see it on NuGet before too long :) |
|
@TheGrandUser I've got one more major issue to look at (type forwarding for |
This adds an optional Action onRemoved argument to the ReactiveDerivedCollection class, and a matching Action onRemoved each of the CreateDerivedCollection and CreateDerivedBindingList methods.
If the delegate is given, then it will be called whenever a derived item is removed, as well as for each item when the derived collection as a whole is disposed.
The DerviedCollectionShouldHandleItemsRemoved test is also added and passes, and all prior tests pass.
Fixes #743