dotnet / maui

.NET MAUI is the .NET Multi-platform App UI, a framework for building native device applications spanning mobile, tablet, and desktop.
https://dot.net/maui
MIT License
22.12k stars 1.73k forks source link

Multiple issues in iOS CollectionView selection synchronization #14535

Open filipnavara opened 1 year ago

filipnavara commented 1 year ago

Description

There are at least three problems in the CollectionView SelectedItem/SelectedItems synchronization in iOS handler code:

https://github.com/dotnet/maui/blob/9fdfc84566c4ad9581459201dc3a10833409b6e1/src/Controls/src/Core/Handlers/Items/iOS/SelectableItemsViewController.cs#L146-L175

  1. The ShouldNotBeSelected method uses reference equality instead of Equals. If the item source returns a different instance of an object then this will fail and deselect items.
  2. The algorithm is actually O(n^2) complexity which is unacceptable. A HashSet could bring this down to more reasonable logarithmic complexity.
  3. If the user selects an item it updates SelectedItem/SelectedItems which in turns runs the selection synchronization unnecessarily:
MailClient.Mobile.UI.Views.ISelectView.IsSelectedPropChangedS (/Users/filipnavara/Projects/emclient-maui/MailClient.Mobile/MailClient.Mobile/UI/Views/ISelectView.cs:54)
Microsoft.Maui.Controls.BindableObject.ClearValue (:0)
Microsoft.Maui.Controls.BindableObject.ClearValue (:0)
Microsoft.Maui.Controls.Setter.UnApply (:0)
Microsoft.Maui.Controls.VisualStateManager.GoToState (:0)
Microsoft.Maui.Controls.Handlers.Items.TemplatedCell.UpdateVisualStates (:0)
Microsoft.Maui.Controls.Handlers.Items.TemplatedCell.set_Selected (:0)
ObjCRuntime.Messaging.void_objc_msgSend_NativeHandle_bool (:0)
UIKit.UICollectionView.DeselectItem (:0)
Microsoft.Maui.Controls.Handlers.Items.SelectableItemsViewController<Microsoft.Maui.Controls.ReorderableItemsView>.SynchronizePlatformSelectionWithSelectedItems (:0)
Microsoft.Maui.Controls.Handlers.Items.SelectableItemsViewController<Microsoft.Maui.Controls.ReorderableItemsView>.UpdatePlatformSelection (:0)
Microsoft.Maui.Controls.Handlers.Items.SelectableItemsViewHandler<Microsoft.Maui.Controls.ReorderableItemsView>.MapSelectedItems (:0)
Microsoft.Maui.PropertyMapper<Microsoft.Maui.Controls.CollectionView,Microsoft.Maui.Controls.Handlers.Items.CollectionViewHandler>. (:0)
Microsoft.Maui.PropertyMapper.UpdatePropertyCore (:0)
Microsoft.Maui.PropertyMapper.UpdateProperty (:0)
Microsoft.Maui.Handlers.ElementHandler.UpdateValue (:0)
Microsoft.Maui.Controls.Element.OnPropertyChanged (:0)
MailClient.Mobile.UI.Views.CollectionViewEx.OnPropertyChanged (/Users/filipnavara/Projects/emclient-maui/MailClient.Mobile/MailClient.Mobile/UI/Views/CollectionViewEx.cs:76)
Microsoft.Maui.Controls.SelectableItemsView.SelectedItemsPropertyChanged (:0)
Microsoft.Maui.Controls.SelectionList.Add (:0)
Microsoft.Maui.Controls.Handlers.Items.SelectableItemsViewController<Microsoft.Maui.Controls.ReorderableItemsView>.FormsSelectItem (:0)
Microsoft.Maui.Controls.Handlers.Items.SelectableItemsViewController<Microsoft.Maui.Controls.ReorderableItemsView>.ItemSelected (:0)
Microsoft.Maui.Controls.Handlers.Items.SelectableItemsViewDelegator<Microsoft.Maui.Controls.ReorderableItemsView,MailClient.Mobile.iOS.Handlers.ReorderableItemsViewController<Microsoft.Maui.Controls.ReorderableItemsView>>.ItemSelected (:0)
ObjCRuntime.Messaging.void_objc_msgSendSuper_NativeHandle_NativeHandle (:0)
UIKit.UIResponder.TouchesEnded (:0)
Microsoft.Maui.Platform.MauiSwipeView.TouchesEnded (:0)
UIKit.UIApplication.xamarin_UIApplicationMain (:0)
UIKit.UIApplication.UIApplicationMain (:0)
UIKit.UIApplication.Main (:0)
MailClient.Mobile.iOS.Application.Main (/Users/filipnavara/Projects/emclient-maui/MailClient.Mobile/MailClient.Mobile.iOS/Main.cs:17)

Steps to Reproduce

TBD

Link to public reproduction project repository

https://github.com/filipnavara/CollectionViewIsBroken

Version with bug

7.0 (current)

Last version that worked well

Unknown/Other

Affected platforms

iOS

Affected platform versions

iOS 16.4

Did you find any workaround?

No, short of copying the whole handler code and fixing it myself.

Relevant log output

No response

drasticactions commented 1 year ago

For notes:

https://github.com/xamarin/Xamarin.Forms/blob/5.0.0/Xamarin.Forms.Platform.iOS/CollectionView/SelectableItemsViewController.cs#L144-L175

Checking Forms, it's the same logic there, copied to MAUI.

filipnavara commented 1 year ago

(I'm working on pushing a reduced sample; my machine is in a bit of disrepair after yesterday's updates)

filipnavara commented 1 year ago

Checking Forms, it's the same logic there, copied to MAUI.

I wonder if XF was avoiding the problem number 3 because the mapping was done differently? I only noticed the first two because of the third one.

filipnavara commented 1 year ago

Updated the sample: https://github.com/filipnavara/CollectionViewIsBroken

Notice that the user selection on tap doesn't "stick". Putting a breakpoint here shows that the item gets instantly deselected: https://github.com/filipnavara/CollectionViewIsBroken/blob/84e9d406ce13d16a84e4ad9b3a5a744a3f585dd3/CollectionViewIsBroken/MainPage.xaml.cs#L86

ghost commented 1 year ago

We've added this issue to our backlog, and we will work to address it as time and resources allow. If you have any additional information or questions about this issue, please leave a comment. For additional info about issue management, please read our Triage Process.