CommunityToolkit / WindowsCommunityToolkit

The Windows Community Toolkit is a collection of helpers, extensions, and custom controls. It simplifies and demonstrates common developer tasks building .NET apps with UWP and the Windows App SDK / WinUI 3 for Windows 10 and Windows 11. The toolkit is part of the .NET Foundation.
https://docs.microsoft.com/windows/communitytoolkit/
Other
5.86k stars 1.37k forks source link

`DataGrid` event handling bug. #4969

Open davidxuang opened 7 months ago

davidxuang commented 7 months ago

Describe the bug

When DataGrid handles a remove event when resort happens, it gets the removed item from the sender:

                        _owner.RemoveRowAt(index, sender[index]);

However, since the item is already removed from the sender, it will get the wrong item. This can be verified by the source of AdvancedCollectionView

                _view.RemoveAt(oldIndex);
                var targetIndex = _view.BinarySearch(item, this);
                if (targetIndex < 0)
                {
                    targetIndex = ~targetIndex;
                }

                // Only trigger expensive UI updates if the index really changed:
                if (targetIndex != oldIndex)
                {
                    OnVectorChanged(new VectorChangedEventArgs(CollectionChange.ItemRemoved, oldIndex, item));

Particularly, if an item is removed from the end, the index will be out of the bound of the source list, thus an ArgumentOutOfRangeException will be thrown by the AdvancedCollectionView, with such stack trace:

    System.Private.CoreLib.dll!System.ThrowHelper.ThrowArgumentOutOfRange_IndexMustBeLessException() Line 118   C#
    System.Private.CoreLib.dll!System.Collections.Generic.List<object>.this[int].get(int index) Line 152    C#
    CommunityToolkit.WinUI.UI.dll!CommunityToolkit.WinUI.UI.AdvancedCollectionView.this[int].get(int index) Line 117    C#
    CommunityToolkit.WinUI.UI.Controls.DataGrid.dll!CommunityToolkit.WinUI.UI.Controls.DataGridInternals.DataGridDataConnection.NotifyingDataSource_VectorChanged(Windows.Foundation.Collections.IObservableVector<object> sender, Windows.Foundation.Collections.IVectorChangedEventArgs e) Line 579   C#
    CommunityToolkit.WinUI.UI.Controls.DataGrid.dll!CommunityToolkit.WinUI.UI.Controls.DataGridInternals.DataGridDataConnection.WireEvents.AnonymousMethod__74_2(CommunityToolkit.WinUI.UI.Controls.DataGridInternals.DataGridDataConnection instance, object source, Windows.Foundation.Collections.IVectorChangedEventArgs eventArgs) Line 451    C#
    CommunityToolkit.WinUI.UI.Controls.DataGrid.dll!CommunityToolkit.WinUI.Utilities.WeakEventListener<CommunityToolkit.WinUI.UI.Controls.DataGridInternals.DataGridDataConnection, object, Windows.Foundation.Collections.IVectorChangedEventArgs>.OnEvent(object source, Windows.Foundation.Collections.IVectorChangedEventArgs eventArgs) Line 32    C#
    CommunityToolkit.WinUI.UI.dll!CommunityToolkit.WinUI.UI.AdvancedCollectionView.OnVectorChanged(Windows.Foundation.Collections.IVectorChangedEventArgs e) Line 702   C#

Regression

No response

Reproducible in sample app?

Steps to reproduce

Build a `DataGrid` based on `AdvancedCollectionView` which enables sorting. An exception will be thrown in `AdvancedCollectionView.ItemOnPropertyChanged()` if the tail element is removed from `AdvancedCollectionView._view`.

Expected behavior

The DataGrid should get the right removed item when handling event.

Screenshots

图片 图片

Windows Build Number

Other Windows Build number

No response

App minimum and target SDK version

Other SDK version

No response

Visual Studio Version

2022

Visual Studio Build Number

No response

Device form factor

Desktop

Nuget packages

communitytoolkit.winui.ui.controls 7.1.2 communitytoolkit.winui.ui.controls.datagrid 7.1.2

Additional context

No response

Help us help you

Yes, I'd like to be assigned to work on this item.

ghost commented 7 months ago

Hello davidxuang, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

ghost commented 6 months ago

This issue has been marked as "needs attention 👋" due to no activity for 15 days. Please triage the issue so the fix can be established.

ghost commented 6 months ago

This issue has been marked as "needs attention 👋" due to no activity for 15 days. Please triage the issue so the fix can be established.

ghost commented 5 months ago

This issue has been marked as "needs attention 👋" due to no activity for 15 days. Please triage the issue so the fix can be established.