Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix ObservableCollection Reset notification handling #185

Merged
merged 4 commits into from
Aug 11, 2020

Conversation

ryanvs
Copy link
Contributor

@ryanvs ryanvs commented Aug 11, 2020

This is the fix for #184 where the previous handler assumed the collection was cleared when the NotifyCollectionChangedAction.Reset action was received. The new handler iterates through the collection to find which items were removed and then only removes those items.

I added a test for the Reset handling in the AutomationTest AvalonDockTest project. It creates several Anchorables and Documents and uses a CustomObservableCollection that raises the Reset notification. I also modified that project (AvalonDockTest) to the new csproj format because the reference to AvalonDock since it had the new file format. There was an issue with the Apartment STA, so I referenced a nuget project to help with that.

Finally, I added a new test project CaliburnDockTestApp that is a simplified version of my original test project AvalonDockTestIssue.

@Dirkster99
Copy link
Owner

Wow

Wow and

Super Wow again :-) I tried to convert the unit test project before but failed (I don't remember why but I wasted about 2 days on it) but you did a pretty superb job here 🥇 I did not know you can actually do unit tests for .Net and .netcore in paralell - awesome.

Do you have a guide or reference for creating the unit test project when I want to look at this if I need to it for some other project?

I've seen a coverlet.collector in the unit test project - its probably not relevant in this project but is this the thing one can use to display the coverage via GitHub marketplace extension?

Thanks a lot for this contribution, I'll have to sit down and study a few new things here :-)

@Dirkster99 Dirkster99 merged commit 714c65b into Dirkster99:master Aug 11, 2020
@ryanvs ryanvs deleted the reset-collection-fix branch August 12, 2020 15:36
@ryanvs
Copy link
Contributor Author

ryanvs commented Aug 12, 2020

Thanks @Dirkster99 - I had recently converted several unit test projects to core so I just repeated the process. I just used the "trial and error" technique of creating a new core unit test project and then adding pieces into it until it worked. I think the "new project wizard" added coverlet.collector and I left it in place.

I'm glad I could help!

@Dirkster99 Dirkster99 added the bug label Aug 14, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants