search

dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.08k stars 1.17k forks source link

Support Bulk Operations in ListCollectionView via ObservableCollection<T> #1887

Open grubioe opened 5 years ago

grubioe commented 5 years ago

To support corefx 10752, surfaced by corefx issue 38085.

In .NET Core 3 Preview 7, the CoreFX team introduced support for ranges via Collection<T> and ObservableCollection<T>. WPF is not able to handle how the new implementation of AddRange on Collection<T> as the add is done as a bulk insert at the end which changes the behavior of 'ListCollectionView` which does not support bulk operations and throws an https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Data/ListCollectionView.cs#L2521.

The net effect here is that a common helper method that is used in many WPF apps (AddRange to an ObservableCollection<T> no longer binds and the replacement method's behavior has a negative and observable side effect.

Adding issue for .NET 5 consideration

weltkante commented 5 years ago

This is actually a longstanding bug in WPF, throwing from a INotifyCollectionChanged event handler is not allowed, you are consuming the events and are not in control of the origin. Correct implementation would be to fall back to the Reset case when you encounter a situation you cannot handle.

The usual workaround is to put a proxy between WPF and the event source, to translate events WPF doesn't understand into reset events.

vatsan-madhavan commented 5 years ago

/cc @karelz, @robertmclaws

michael-hawker commented 3 years ago

Related to #52

robertmclaws commented 3 years ago

YES! Let's finally get this done for 7.0!!!

SkyeHoefling commented 3 years ago

Should I try and grab the old code I originally PRed from a few years ago and submit a new PR to dotnet/runtime?

legistek commented 2 years ago

I'd just like to say that as much as I'd love if this weren't an issue, as the publisher of a commerical WPF app I'd be a bit apprehensive of the possibility of side effects from this. We saw how easily relatively modest changes to OC broke existing code because of all the extension methods people had implemented over the years to get around this issue; we don't know what other code is out there that assumes the behavior that might be changing if this were implemented across the framework.

I'm thinking generally of the possibility of controls that code assumes will fire multiple events in response to serial collection changes suddenly firing only one such event. SelectionChanged events and things of that nature, for example. But it's the unknown unknowns that I'd be even more concerned about.

Certainly range methods can be added to ObservableCollection - or perhaps better yet a new collection type altogether that fully implements INCC - without creating the same unintended issues the original approach did as long as steps are taken to ensure they aren't inadvertently used with WPF. I would consider the possibility of a more conservative approach before doing something that could potentially impact the behavior of a large section of this 15 year old framework.