From 50b0659964baa50b58cff0e3292dfde57a02cbe9 Mon Sep 17 00:00:00 2001 From: Prathamesh Narkhede <55591622+prathameshnarkhede@users.noreply.github.com> Date: Thu, 5 Sep 2024 09:48:38 -0700 Subject: [PATCH] Breaking change: Changed from ListView to CollectionView in BookmarksView which resolves ItemTemplate issue in iOS (#598) * Document known MAUI iOS issue in ItemTemplateChanged * Refactor UI: Replace ListView with CollectionView * Refactor BookmarksView to centralize CollectionView updates - This Fixes a bug with Android adding item couple of times when new Bookmark is added. - Centralize the update logic for the CollectionView in the BookmarksView class by introducing a new `UpdateListView` method. * Refactor: centralize "PresentingView" string usage * Changing name * Refactor event handling in BookmarksView and DataSource This fixes problem in Android from root level. * Removing unnecessary keyword * Refactor BookmarksView for simplicity and performance * Refactor BookmarksView to use class-level CollectionView to maintain sequence of detaching and attaching events * Refactor _presentingView visibility and assignment logic --- .../Samples/BookmarksViewSample.xaml | 6 +- .../BookmarksView/BookmarksView.cs | 83 +++++++------------ .../BookmarksView/BookmarksViewDataSource.cs | 46 ++++++---- 3 files changed, 60 insertions(+), 75 deletions(-) diff --git a/src/Samples/Toolkit.SampleApp.Maui/Samples/BookmarksViewSample.xaml b/src/Samples/Toolkit.SampleApp.Maui/Samples/BookmarksViewSample.xaml index 0ac38ae35..8cdfb7823 100644 --- a/src/Samples/Toolkit.SampleApp.Maui/Samples/BookmarksViewSample.xaml +++ b/src/Samples/Toolkit.SampleApp.Maui/Samples/BookmarksViewSample.xaml @@ -10,12 +10,10 @@ x:Class="Toolkit.SampleApp.Maui.Samples.BookmarksViewSample"> - + - - + diff --git a/src/Toolkit/Toolkit.Maui/BookmarksView/BookmarksView.cs b/src/Toolkit/Toolkit.Maui/BookmarksView/BookmarksView.cs index f91f3a887..fafa741ee 100644 --- a/src/Toolkit/Toolkit.Maui/BookmarksView/BookmarksView.cs +++ b/src/Toolkit/Toolkit.Maui/BookmarksView/BookmarksView.cs @@ -24,8 +24,9 @@ namespace Esri.ArcGISRuntime.Toolkit.Maui; /// public class BookmarksView : TemplatedView { - private ListView? _presentingView; - private BookmarksViewDataSource _dataSource = new BookmarksViewDataSource(); + private readonly BookmarksViewDataSource _dataSource = new(); + private const string _presentingViewName = "PresentingView"; + private CollectionView? _presentingView; private static readonly DataTemplate DefaultDataTemplate; private static readonly ControlTemplate DefaultControlTemplate; @@ -33,20 +34,18 @@ public class BookmarksView : TemplatedView [DynamicDependency(nameof(Bookmark.Name), "Esri.ArcGISRuntime.Mapping.Bookmark", "Esri.ArcGISRuntime")] static BookmarksView() { - DefaultDataTemplate = new DataTemplate(() => - { - var defaultCell = new TextCell(); - defaultCell.SetBinding(TextCell.TextProperty, nameof(Bookmark.Name)); - return defaultCell; - }); - - string template = @" - - - RecycleElement - - - "; + string dataTemplate = + @" + "; + DefaultDataTemplate = Microsoft.Maui.Controls.Xaml.Extensions.LoadFromXaml(new DataTemplate(), dataTemplate); + + string template = + $@" + + "; DefaultControlTemplate = Microsoft.Maui.Controls.Xaml.Extensions.LoadFromXaml(new ControlTemplate(), template); } @@ -56,7 +55,6 @@ static BookmarksView() public BookmarksView() { ItemTemplate = DefaultDataTemplate; - ControlTemplate = DefaultControlTemplate; } @@ -67,17 +65,14 @@ protected override void OnApplyTemplate() { base.OnApplyTemplate(); - if (_presentingView != null) + if (_presentingView is not null) { - _presentingView.ItemSelected -= Internal_bookmarkSelected; + _presentingView.SelectionChanged -= Internal_bookmarkSelected; } - - _presentingView = GetTemplateChild("PresentingView") as ListView; - - if (_presentingView != null) + _presentingView = GetTemplateChild(_presentingViewName) as CollectionView; + if (_presentingView is not null) { - _presentingView.ItemSelected += Internal_bookmarkSelected; - _presentingView.ItemTemplate = ItemTemplate; + _presentingView.SelectionChanged += Internal_bookmarkSelected; _presentingView.ItemsSource = _dataSource; } } @@ -131,7 +126,7 @@ public GeoView? GeoView /// Identifies the bindable property. /// public static readonly BindableProperty ItemTemplateProperty = - BindableProperty.Create(nameof(ItemTemplate), typeof(DataTemplate), typeof(BookmarksView), DefaultDataTemplate, BindingMode.OneWay, null, propertyChanged: ItemTemplateChanged); + BindableProperty.Create(nameof(ItemTemplate), typeof(DataTemplate), typeof(BookmarksView), DefaultDataTemplate, BindingMode.OneWay, null); /// /// Handles property changes for the bindable property. @@ -151,27 +146,6 @@ private static void GeoViewChanged(BindableObject sender, object? oldValue, obje bookmarkView._dataSource.SetGeoView(newValue as GeoView); } - /// - /// Handles property changes for the bindable property. - /// - private static void ItemTemplateChanged(BindableObject sender, object? oldValue, object? newValue) - { - BookmarksView bookmarkView = (BookmarksView)sender; - - if (bookmarkView._presentingView != null) - { - bookmarkView._presentingView.ItemTemplate = newValue as DataTemplate; - -#if WINDOWS - // This workaround addresses an issue with MAUI WinUI. - // Without refreshing the items source of the BookmarksView ListView the change is not reflected in the UI. - var existingItems = bookmarkView._presentingView.ItemsSource; - bookmarkView._presentingView.ItemsSource = null; - bookmarkView._presentingView.ItemsSource = existingItems; -#endif - } - } - /// /// Selects the bookmark and navigates to it in the associated . /// @@ -192,16 +166,15 @@ private void SelectAndNavigateToBookmark(Bookmark bookmark) /// /// Handles selection on the underlying list view. /// - private void Internal_bookmarkSelected(object? sender, SelectedItemChangedEventArgs e) + private void Internal_bookmarkSelected(object? sender, SelectionChangedEventArgs e) { - if (e.SelectedItem is Bookmark bm) - { - SelectAndNavigateToBookmark(bm); - } - - if (e.SelectedItem != null && sender is ListView lv) + if (sender is CollectionView cv) { - lv.SelectedItem = null; + if (cv.SelectedItem is Bookmark item) + { + SelectAndNavigateToBookmark(item); + } + cv.ClearValue(CollectionView.SelectedItemProperty); } } diff --git a/src/Toolkit/Toolkit/UI/Controls/BookmarksView/BookmarksViewDataSource.cs b/src/Toolkit/Toolkit/UI/Controls/BookmarksView/BookmarksViewDataSource.cs index a811939c2..5b91866c7 100644 --- a/src/Toolkit/Toolkit/UI/Controls/BookmarksView/BookmarksViewDataSource.cs +++ b/src/Toolkit/Toolkit/UI/Controls/BookmarksView/BookmarksViewDataSource.cs @@ -38,6 +38,9 @@ namespace Esri.ArcGISRuntime.Toolkit.UI.Controls internal class BookmarksViewDataSource : IList, INotifyCollectionChanged, INotifyPropertyChanged, IList { private GeoView? _geoView; + private WeakEventListener? _geoViewBookmarksListener; + private WeakEventListener? _geoViewLoadListener; + private WeakEventListener? _overrideListListener; private IList? _overrideList; private IList ActiveBookmarkList @@ -93,10 +96,13 @@ public void SetOverrideList(IEnumerable? bookmarks) // Subscribe to events if applicable if (bookmarks is INotifyCollectionChanged iCollectionChanged) { - var listener = new WeakEventListener(this, iCollectionChanged); - listener.OnEventAction = static (instance, source, eventArgs) => instance.HandleOverrideListCollectionChanged(source, eventArgs); - listener.OnDetachAction = static (instance, source, weakEventListener) => source.CollectionChanged -= weakEventListener.OnEvent; - iCollectionChanged.CollectionChanged += listener.OnEvent; + _overrideListListener?.Detach(); + _overrideListListener = new WeakEventListener(this, iCollectionChanged) + { + OnEventAction = static (instance, source, eventArgs) => instance.HandleOverrideListCollectionChanged(source, eventArgs), + OnDetachAction = static (instance, source, weakEventListener) => source.CollectionChanged -= weakEventListener.OnEvent + }; + iCollectionChanged.CollectionChanged += _overrideListListener.OnEvent; } } @@ -188,13 +194,16 @@ private void GeoView_PropertyChanged(object? sender, PropertyChangedEventArgs e) private void GeoViewDocumentChanged(object? sender, object? e) { + _geoViewLoadListener?.Detach(); if (_geoView is MapView mv && mv.Map is ILoadable mapLoadable) { // Listen for load completion - var listener = new WeakEventListener(this, mapLoadable); - listener.OnEventAction = static (instance, source, eventArgs) => instance.Doc_Loaded(source, eventArgs); - listener.OnDetachAction = static (instance, source, weakEventListener) => source.Loaded -= weakEventListener.OnEvent; - mapLoadable.Loaded += listener.OnEvent; + _geoViewLoadListener = new WeakEventListener(this, mapLoadable) + { + OnEventAction = static (instance, source, eventArgs) => instance.Doc_Loaded(source, eventArgs), + OnDetachAction = static (instance, source, weakEventListener) => source.Loaded -= weakEventListener.OnEvent + }; + mapLoadable.Loaded += _geoViewLoadListener.OnEvent; // Ensure event is raised even if already loaded _ = mv.Map.RetryLoadAsync(); @@ -202,10 +211,12 @@ private void GeoViewDocumentChanged(object? sender, object? e) else if (_geoView is SceneView sv && sv.Scene is ILoadable sceneLoadable) { // Listen for load completion - var listener = new WeakEventListener(this, sceneLoadable); - listener.OnEventAction = static (instance, source, eventArgs) => instance.Doc_Loaded(source, eventArgs); - listener.OnDetachAction = static (instance, source, weakEventListener) => source.Loaded -= weakEventListener.OnEvent; - sceneLoadable.Loaded += listener.OnEvent; + _geoViewLoadListener = new WeakEventListener(this, sceneLoadable) + { + OnEventAction = static (instance, source, eventArgs) => instance.Doc_Loaded(source, eventArgs), + OnDetachAction = static (instance, source, weakEventListener) => source.Loaded -= weakEventListener.OnEvent + }; + sceneLoadable.Loaded += _geoViewLoadListener.OnEvent; // Ensure event is raised even if already loaded _ = sv.Scene.RetryLoadAsync(); @@ -240,10 +251,13 @@ private void Doc_Loaded(object? sender, EventArgs e) OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); } - var listener = new WeakEventListener(this, bmCollection); - listener.OnEventAction = static (instance, source, eventArgs) => instance.HandleGeoViewBookmarksCollectionChanged(source, eventArgs); - listener.OnDetachAction = static (instance, source, weakEventListener) => source.CollectionChanged -= weakEventListener.OnEvent; - bmCollection.CollectionChanged += listener.OnEvent; + _geoViewBookmarksListener?.Detach(); + _geoViewBookmarksListener = new WeakEventListener(this, bmCollection) + { + OnEventAction = static (instance, source, eventArgs) => instance.HandleGeoViewBookmarksCollectionChanged(source, eventArgs), + OnDetachAction = static (instance, source, weakEventListener) => source.CollectionChanged -= weakEventListener.OnEvent + }; + bmCollection.CollectionChanged += _geoViewBookmarksListener.OnEvent; } private void HandleGeoViewBookmarksCollectionChanged(object? sender, NotifyCollectionChangedEventArgs e)