Skip to content

Commit

Permalink
Breaking change: Changed from ListView to CollectionView in Bookmarks…
Browse files Browse the repository at this point in the history
…View 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
  • Loading branch information
prathameshnarkhede authored Sep 5, 2024
1 parent d06f4e1 commit 50b0659
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@
x:Class="Toolkit.SampleApp.Maui.Samples.BookmarksViewSample">
<ContentPage.Resources>
<DataTemplate x:Key="ItemTemplateOne">
<TextCell Text="{Binding Name}" TextColor="Red" />
<Label Text="{Binding Name}" TextColor="Red" />
</DataTemplate>
<DataTemplate x:Key="ItemTemplateTwo">
<ViewCell>
<Label BackgroundColor="Red" TextColor="White" Text="{Binding Name}" />
</ViewCell>
<Label BackgroundColor="Red" TextColor="White" Text="{Binding Name}" />
</DataTemplate>
</ContentPage.Resources>
<ContentPage.Content>
Expand Down
83 changes: 28 additions & 55 deletions src/Toolkit/Toolkit.Maui/BookmarksView/BookmarksView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,28 @@ namespace Esri.ArcGISRuntime.Toolkit.Maui;
/// </summary>
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;

[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 = @"<ControlTemplate xmlns=""http://schemas.microsoft.com/dotnet/2021/maui"" xmlns:x=""http://schemas.microsoft.com/winfx/2009/xaml"" xmlns:esriTK=""clr-namespace:Esri.ArcGISRuntime.Toolkit.Maui"">
<ListView x:Name=""PresentingView"" HorizontalOptions=""FillAndExpand"" VerticalOptions=""FillAndExpand"">
<x:Arguments>
<ListViewCachingStrategy>RecycleElement</ListViewCachingStrategy>
</x:Arguments>
</ListView>
</ControlTemplate>";
string dataTemplate =
@"<DataTemplate xmlns=""http://schemas.microsoft.com/dotnet/2021/maui"">
<Label Text=""{Binding Name}"" />
</DataTemplate>";
DefaultDataTemplate = Microsoft.Maui.Controls.Xaml.Extensions.LoadFromXaml(new DataTemplate(), dataTemplate);

string template =
$@"<ControlTemplate xmlns=""http://schemas.microsoft.com/dotnet/2021/maui""
xmlns:x=""http://schemas.microsoft.com/winfx/2009/xaml""
xmlns:esriTK=""clr-namespace:Esri.ArcGISRuntime.Toolkit.Maui"">
<CollectionView x:Name=""{_presentingViewName}"" SelectionMode=""Single"" ItemTemplate=""{{TemplateBinding ItemTemplate}}"" />
</ControlTemplate>";
DefaultControlTemplate = Microsoft.Maui.Controls.Xaml.Extensions.LoadFromXaml(new ControlTemplate(), template);
}

Expand All @@ -56,7 +55,6 @@ static BookmarksView()
public BookmarksView()
{
ItemTemplate = DefaultDataTemplate;

ControlTemplate = DefaultControlTemplate;
}

Expand All @@ -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;
}
}
Expand Down Expand Up @@ -131,7 +126,7 @@ public GeoView? GeoView
/// Identifies the <see cref="ItemTemplate" /> bindable property.
/// </summary>
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);

/// <summary>
/// Handles property changes for the <see cref="BookmarksOverride" /> bindable property.
Expand All @@ -151,27 +146,6 @@ private static void GeoViewChanged(BindableObject sender, object? oldValue, obje
bookmarkView._dataSource.SetGeoView(newValue as GeoView);
}

/// <summary>
/// Handles property changes for the <see cref="ItemTemplate" /> bindable property.
/// </summary>
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
}
}

/// <summary>
/// Selects the bookmark and navigates to it in the associated <see cref="GeoView" />.
/// </summary>
Expand All @@ -192,16 +166,15 @@ private void SelectAndNavigateToBookmark(Bookmark bookmark)
/// <summary>
/// Handles selection on the underlying list view.
/// </summary>
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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ namespace Esri.ArcGISRuntime.Toolkit.UI.Controls
internal class BookmarksViewDataSource : IList<Bookmark>, INotifyCollectionChanged, INotifyPropertyChanged, IList
{
private GeoView? _geoView;
private WeakEventListener<BookmarksViewDataSource, INotifyCollectionChanged, object?, NotifyCollectionChangedEventArgs>? _geoViewBookmarksListener;
private WeakEventListener<BookmarksViewDataSource, ILoadable, object?, EventArgs>? _geoViewLoadListener;
private WeakEventListener<BookmarksViewDataSource, INotifyCollectionChanged, object?, NotifyCollectionChangedEventArgs>? _overrideListListener;
private IList<Bookmark>? _overrideList;

private IList<Bookmark> ActiveBookmarkList
Expand Down Expand Up @@ -93,10 +96,13 @@ public void SetOverrideList(IEnumerable<Bookmark>? bookmarks)
// Subscribe to events if applicable
if (bookmarks is INotifyCollectionChanged iCollectionChanged)
{
var listener = new WeakEventListener<BookmarksViewDataSource, INotifyCollectionChanged, object?, NotifyCollectionChangedEventArgs>(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<BookmarksViewDataSource, INotifyCollectionChanged, object?, NotifyCollectionChangedEventArgs>(this, iCollectionChanged)
{
OnEventAction = static (instance, source, eventArgs) => instance.HandleOverrideListCollectionChanged(source, eventArgs),
OnDetachAction = static (instance, source, weakEventListener) => source.CollectionChanged -= weakEventListener.OnEvent
};
iCollectionChanged.CollectionChanged += _overrideListListener.OnEvent;
}
}

Expand Down Expand Up @@ -188,24 +194,29 @@ 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<BookmarksViewDataSource, ILoadable, object?, EventArgs>(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<BookmarksViewDataSource, ILoadable, object?, EventArgs>(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();
}
else if (_geoView is SceneView sv && sv.Scene is ILoadable sceneLoadable)
{
// Listen for load completion
var listener = new WeakEventListener<BookmarksViewDataSource, ILoadable, object?, EventArgs>(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<BookmarksViewDataSource, ILoadable, object?, EventArgs>(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();
Expand Down Expand Up @@ -240,10 +251,13 @@ private void Doc_Loaded(object? sender, EventArgs e)
OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
}

var listener = new WeakEventListener<BookmarksViewDataSource, INotifyCollectionChanged, object?, NotifyCollectionChangedEventArgs>(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<BookmarksViewDataSource, INotifyCollectionChanged, object?, NotifyCollectionChangedEventArgs>(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)
Expand Down

0 comments on commit 50b0659

Please sign in to comment.