Skip to content

Minimal support for ObservableCollection<T> range actions #10845

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -384,9 +384,16 @@ protected override void ProcessCollectionChanged(NotifyCollectionChangedEventArg
{
ValidateCollectionChangedEventArgs(args);

bool isRangeAction = IsCollectionChangedRangeAction(args);

bool moveCurrencyOffDeletedElement = false;

switch (args.Action)
// If we have a range operation, treat it as a refresh for now
NotifyCollectionChangedAction effectiveAction = isRangeAction
? NotifyCollectionChangedAction.Reset
: args.Action;

switch (effectiveAction)
{
case NotifyCollectionChangedAction.Add:
case NotifyCollectionChangedAction.Remove:
Expand Down Expand Up @@ -608,7 +615,7 @@ protected override void ProcessCollectionChanged(NotifyCollectionChangedEventArg

case NotifyCollectionChangedAction.Reset:
{
_traceLog?.Add("ProcessCollectionChanged action = {0}", args.Action);
_traceLog?.Add("ProcessCollectionChanged action = {0}", effectiveAction);

if (_collection.Count != 0)
{
Expand Down Expand Up @@ -1430,23 +1437,23 @@ private void ValidateCollectionChangedEventArgs(NotifyCollectionChangedEventArgs
switch (e.Action)
{
case NotifyCollectionChangedAction.Add:
if (e.NewItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
break;

case NotifyCollectionChangedAction.Remove:
if (e.OldItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.OldItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
break;

case NotifyCollectionChangedAction.Replace:
if (e.NewItems.Count != 1 || e.OldItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1 || e.OldItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
break;

case NotifyCollectionChangedAction.Move:
if (e.NewItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
if (e.NewStartingIndex < 0)
throw new InvalidOperationException(SR.CannotMoveToUnknownPosition);
break;
Expand All @@ -1459,6 +1466,15 @@ private void ValidateCollectionChangedEventArgs(NotifyCollectionChangedEventArgs
}
}

private bool IsCollectionChangedRangeAction(NotifyCollectionChangedEventArgs e)
{
return
(e.Action == NotifyCollectionChangedAction.Add && e.NewItems.Count > 1) ||
(e.Action == NotifyCollectionChangedAction.Remove && e.OldItems.Count > 1) ||
(e.Action == NotifyCollectionChangedAction.Replace && (e.OldItems.Count > 1 || e.NewItems.Count > 1)) ||
(e.Action == NotifyCollectionChangedAction.Move && (e.OldItems.Count > 1 || e.NewItems.Count > 1));
}

/// <summary>
Comment on lines +1469 to 1478
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are creating 4x the same method. Cant this be made static and moved into a helper class or something like that instead of duplicating code all over?

Copy link
Member

@h3xds1nz h3xds1nz May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All it needs is private protected on CollectionView as its a base class for all of them, Selector can live with dupl imho.

/// Helper to raise a PropertyChanged event />).
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2388,35 +2388,46 @@ private void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs
switch (args.Action)
{
case NotifyCollectionChangedAction.Add:

// Treat range operations as refresh operations, for minimum support.
// This can be expanded later with a better implementation, if needed.
if (args.NewItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
OnItemAdded(args.NewItems[0], args.NewStartingIndex);
OnRefresh();
else
OnItemAdded(args.NewItems[0], args.NewStartingIndex);
break;

case NotifyCollectionChangedAction.Remove:
if (args.OldItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
OnItemRemoved(args.OldItems[0], args.OldStartingIndex);
OnRefresh();
else
OnItemRemoved(args.OldItems[0], args.OldStartingIndex);
break;

case NotifyCollectionChangedAction.Replace:
// Don't check arguments if app targets 4.0, for compat ( 726682)
// Don't check arguments if app targets 4.0, for compat (726682)
if (!FrameworkCompatibilityPreferences.TargetsDesktop_V4_0)
{
if (args.OldItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
OnRefresh();
else
OnItemReplaced(args.OldItems[0], args.NewItems[0], args.NewStartingIndex);
}
OnItemReplaced(args.OldItems[0], args.NewItems[0], args.NewStartingIndex);
else
OnItemReplaced(args.OldItems[0], args.NewItems[0], args.NewStartingIndex);
break;

case NotifyCollectionChangedAction.Move:
// Don't check arguments if app targets 4.0, for compat ( 726682)
// Don't check arguments if app targets 4.0, for compat (726682)
if (!FrameworkCompatibilityPreferences.TargetsDesktop_V4_0)
{
if (args.OldItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
OnRefresh();
else
OnItemMoved(args.OldItems[0], args.OldStartingIndex, args.NewStartingIndex);
}
OnItemMoved(args.OldItems[0], args.OldStartingIndex, args.NewStartingIndex);
else
OnItemMoved(args.OldItems[0], args.OldStartingIndex, args.NewStartingIndex);
break;

case NotifyCollectionChangedAction.Reset:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -862,11 +862,16 @@ private void OnSelectedItemsCollectionChanged(object sender, NotifyCollectionCha
throw new InvalidOperationException(SR.ChangingCollectionNotSupported);
}

// If we have a range operation, treat it as a refresh for now
NotifyCollectionChangedAction effectiveAction = IsCollectionChangedRangeAction(e)
? NotifyCollectionChangedAction.Reset
: e.Action;

SelectionChange.Begin();
bool succeeded=false;
try
{
switch (e.Action)
switch (effectiveAction)
{
case NotifyCollectionChangedAction.Add:
if (e.NewItems.Count != 1)
Expand Down Expand Up @@ -924,6 +929,15 @@ private void OnSelectedItemsCollectionChanged(object sender, NotifyCollectionCha
}
}

private bool IsCollectionChangedRangeAction(NotifyCollectionChangedEventArgs e)
{
return
(e.Action == NotifyCollectionChangedAction.Add && e.NewItems.Count > 1) ||
(e.Action == NotifyCollectionChangedAction.Remove && e.OldItems.Count > 1) ||
(e.Action == NotifyCollectionChangedAction.Replace && (e.OldItems.Count > 1 || e.NewItems.Count > 1)) ||
(e.Action == NotifyCollectionChangedAction.Move && (e.OldItems.Count > 1 || e.NewItems.Count > 1));
}

#endregion


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1493,7 +1493,12 @@ protected override void ProcessCollectionChanged(NotifyCollectionChangedEventArg
bool oldIsCurrentBeforeFirst = IsCurrentBeforeFirst;
bool moveCurrency = false;

switch (args.Action)
// If we have a range operation, treat it as a refresh for now
NotifyCollectionChangedAction effectiveAction = IsCollectionChangedRangeAction(args)
? NotifyCollectionChangedAction.Reset
: args.Action;

switch (effectiveAction)
{
case NotifyCollectionChangedAction.Add:
if (_newItemIndex == -2)
Expand Down Expand Up @@ -1806,6 +1811,12 @@ private IEnumerator InternalGetEnumerator()
// Data Collection immediately after the CollectionChangeEvent
private void AdjustShadowCopy(NotifyCollectionChangedEventArgs e)
{
// No change needed for range operations, as we just reset the collection
if (IsCollectionChangedRangeAction(e))
{
return;
}

switch (e.Action)
{
case NotifyCollectionChangedAction.Add:
Expand Down Expand Up @@ -2430,23 +2441,23 @@ private void ValidateCollectionChangedEventArgs(NotifyCollectionChangedEventArgs
switch (e.Action)
{
case NotifyCollectionChangedAction.Add:
if (e.NewItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
break;

case NotifyCollectionChangedAction.Remove:
if (e.OldItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.OldItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
break;

case NotifyCollectionChangedAction.Replace:
if (e.NewItems.Count != 1 || e.OldItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1 || e.OldItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
break;

case NotifyCollectionChangedAction.Move:
if (e.NewItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
if (e.NewStartingIndex < 0)
throw new InvalidOperationException(SR.CannotMoveToUnknownPosition);
break;
Expand All @@ -2459,6 +2470,15 @@ private void ValidateCollectionChangedEventArgs(NotifyCollectionChangedEventArgs
}
}

private bool IsCollectionChangedRangeAction(NotifyCollectionChangedEventArgs e)
{
return
(e.Action == NotifyCollectionChangedAction.Add && e.NewItems.Count > 1) ||
(e.Action == NotifyCollectionChangedAction.Remove && e.OldItems.Count > 1) ||
(e.Action == NotifyCollectionChangedAction.Replace && (e.OldItems.Count > 1 || e.NewItems.Count > 1)) ||
(e.Action == NotifyCollectionChangedAction.Move && (e.OldItems.Count > 1 || e.NewItems.Count > 1));
}

/// <summary>
/// Helper to raise a PropertyChanged event />).
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,12 @@ protected virtual void ProcessCollectionChanged(NotifyCollectionChangedEventArgs
int oldCurrentPosition = _currentPosition;
bool raiseChanged = false;

switch (args.Action)
// If we have a range operation, treat it as a refresh for now
NotifyCollectionChangedAction effectiveAction = IsCollectionChangedRangeAction(args)
? NotifyCollectionChangedAction.Reset
: args.Action;

switch (effectiveAction)
{
case NotifyCollectionChangedAction.Add:
if (PassesFilter(args.NewItems[0]))
Expand Down Expand Up @@ -1948,25 +1953,25 @@ private void ValidateCollectionChangedEventArgs(NotifyCollectionChangedEventArgs
switch (e.Action)
{
case NotifyCollectionChangedAction.Add:
if (e.NewItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
break;

case NotifyCollectionChangedAction.Remove:
if (e.OldItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.OldItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
if (e.OldStartingIndex < 0)
throw new InvalidOperationException(SR.RemovedItemNotFound);
break;

case NotifyCollectionChangedAction.Replace:
if (e.NewItems.Count != 1 || e.OldItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1 || e.OldItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
break;

case NotifyCollectionChangedAction.Move:
if (e.NewItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
if (e.NewStartingIndex < 0)
throw new InvalidOperationException(SR.CannotMoveToUnknownPosition);
break;
Expand All @@ -1979,6 +1984,15 @@ private void ValidateCollectionChangedEventArgs(NotifyCollectionChangedEventArgs
}
}

private bool IsCollectionChangedRangeAction(NotifyCollectionChangedEventArgs e)
{
return
(e.Action == NotifyCollectionChangedAction.Add && e.NewItems.Count > 1) ||
(e.Action == NotifyCollectionChangedAction.Remove && e.OldItems.Count > 1) ||
(e.Action == NotifyCollectionChangedAction.Replace && (e.OldItems.Count > 1 || e.NewItems.Count > 1)) ||
(e.Action == NotifyCollectionChangedAction.Move && (e.OldItems.Count > 1 || e.NewItems.Count > 1));
}

// fix up CurrentPosition and CurrentItem after a collection change
private void AdjustCurrencyForAdd(int index)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1657,6 +1657,8 @@ protected override void ProcessCollectionChanged(NotifyCollectionChangedEventArg

ValidateCollectionChangedEventArgs(args);

bool isRangeAction = IsCollectionChangedRangeAction(args);

// adding or replacing an item can change CanAddNew, by providing a
// non-null representative
if (!_isItemConstructorValid)
Expand All @@ -1677,7 +1679,8 @@ protected override void ProcessCollectionChanged(NotifyCollectionChangedEventArg
// apply the change to the shadow copy
if (AllowsCrossThreadChanges)
{
if (args.Action != NotifyCollectionChangedAction.Reset)
// ignore when we have a range action, as we'll just reset the collection
if (args.Action != NotifyCollectionChangedAction.Reset && !isRangeAction)
{
if (args.Action != NotifyCollectionChangedAction.Remove && args.NewStartingIndex < 0
|| args.Action != NotifyCollectionChangedAction.Add && args.OldStartingIndex < 0)
Expand All @@ -1693,7 +1696,8 @@ protected override void ProcessCollectionChanged(NotifyCollectionChangedEventArg
}

// If the Action is Reset then we do a Refresh.
if (args.Action == NotifyCollectionChangedAction.Reset)
// Also treat range actions as Reset, for now.
if (args.Action == NotifyCollectionChangedAction.Reset || isRangeAction)
{
// implicitly cancel EditItem transactions
if (IsEditingItem)
Expand Down Expand Up @@ -2497,23 +2501,23 @@ private void ValidateCollectionChangedEventArgs(NotifyCollectionChangedEventArgs
switch (e.Action)
{
case NotifyCollectionChangedAction.Add:
if (e.NewItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
break;

case NotifyCollectionChangedAction.Remove:
if (e.OldItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.OldItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
break;

case NotifyCollectionChangedAction.Replace:
if (e.NewItems.Count != 1 || e.OldItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1 || e.OldItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
break;

case NotifyCollectionChangedAction.Move:
if (e.NewItems.Count != 1)
throw new NotSupportedException(SR.RangeActionsNotSupported);
if (e.NewItems.Count < 1)
throw new NotSupportedException(SR.UnexpectedCollectionChangeAction);
if (e.NewStartingIndex < 0)
throw new InvalidOperationException(SR.CannotMoveToUnknownPosition);
break;
Expand All @@ -2526,6 +2530,15 @@ private void ValidateCollectionChangedEventArgs(NotifyCollectionChangedEventArgs
}
}

private bool IsCollectionChangedRangeAction(NotifyCollectionChangedEventArgs e)
{
return
(e.Action == NotifyCollectionChangedAction.Add && e.NewItems.Count > 1) ||
(e.Action == NotifyCollectionChangedAction.Remove && e.OldItems.Count > 1) ||
(e.Action == NotifyCollectionChangedAction.Replace && (e.OldItems.Count > 1 || e.NewItems.Count > 1)) ||
(e.Action == NotifyCollectionChangedAction.Move && (e.OldItems.Count > 1 || e.NewItems.Count > 1));
}


/// <summary>
/// Create, filter and sort the local index array.
Expand Down