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

Support range notifications #9568

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Mrxx99
Copy link

@Mrxx99 Mrxx99 commented Aug 13, 2024

Fixes #52

Description

This adds support for range notifications coming from INotifyCollectionChanged for Add/Remove/Replace.
I hope I have covered all scenarios where it need to be adjusted. It works with and without AllowsCrossThreadChanges.

Customer Impact

Customers can finally use ObservableCollections that implement range operations. Would also help unblock this: dotnet/runtime#18087

Testing

A test app that hopefully covers all scenarios, see here: https://github.com/Mrxx99/ObservableRangeWpfTestApp
I have run the DRTs successfully.

Risk

I am not aware of any, except of maybe regression bugs.

Microsoft Reviewers: Open in CodeFlow

@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions draft labels Aug 13, 2024
@h3xds1nz
Copy link
Member

h3xds1nz commented Aug 15, 2024

This unfortunately requires way more effort, this is just one out of many things that need to be addressed before the runtime PR would be unblocked in a way where it doesn't implement a new type / or using a switch.

For example, the changes you've made in ListCollectionView, this requires way more work for LiveShaping to work properly (Grouping, Filtering, etc.)

Then there's all the other views that would have to be fixed too.

@Mrxx99
Copy link
Author

Mrxx99 commented Aug 16, 2024

Was thinking about that and had already had some more changes e.g. in the BindingListCollectionView, but I am not sure if all of these really are required. At least I thought that e.g. when using filtering you can't be using a normal ObservalbleCollection, but you would use a different CollectionView(Source). I had forgotten how filtering/grouping works.
Another thing I thought would not need to be adjusted is all the adding functionality that is coming from the view (e.g. adding a new row in a DataGrid) as there is still only one item allowed there.

I already thought it was not complete, that's why I created it as a draft and to also start a discussion to find everything that has to be done. So thanks for your help. Will take a look at filtering next. Never heard about the LiveShapingList (besides seeing the name in the code). I will have to investigate about this.

If you or someone else wants to help, the work could also be maybe be split into multiple pull requests. I will also accept pull requests on my branch and my test app to cover more scenarios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution A label for all community Contributions draft PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved INotifyCollectionChanged Handling
2 participants