From 70f45986ab1ab89fcaafc87c6837ef69c48cf573 Mon Sep 17 00:00:00 2001 From: vatsashah45 Date: Mon, 26 Aug 2024 17:05:18 -0400 Subject: [PATCH] fix: Refactored LoggingSelectionInfo --- .../Given_ListView.cs | 10 +- .../LoggingSelectionInfo.cs | 313 +++++++++++++----- .../Controls/ListViewBase/ListViewBase.cs | 3 +- 3 files changed, 228 insertions(+), 98 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListView.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListView.cs index db89e2acf13d..a4c2c7557e80 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListView.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListView.cs @@ -6,7 +6,6 @@ using Windows.Foundation; using Windows.UI.Input.Preview.Injection; using Microsoft.UI.Xaml.Controls; -using Microsoft.UI.Xaml.Controls.Primitives; using System.Linq; namespace Uno.UI.RuntimeTests.Tests.Microsoft_UI_Xaml_Controls @@ -17,22 +16,20 @@ public class Given_ListView [TestMethod] [RunsOnUIThread] #if !HAS_INPUT_INJECTOR - [Ignore("InputInjector is only supported on Skia #14948")] + [Ignore("InputInjector is not supported on this platform.")] #endif public async Task When_ItemClicked_SelectsCorrectIndex() { var items = new object[] { "Item 1", "Item 2", "Item 3" }; - var loggingSelectionInfo = new LoggingSelectionInfo(items, false, null); + var loggingSelectionInfo = new LoggingSelectionInfo(items); var listViewBase = new ListView { - ItemsSource = items, + ItemsSource = loggingSelectionInfo, }; TestServices.WindowHelper.WindowContent = listViewBase; await TestServices.WindowHelper.WaitForLoaded(listViewBase); - loggingSelectionInfo.SafeMoveCurrentToPosition(0); - // We don't use ActualWidth because of https://github.com/unoplatform/uno/issues/15982 var tapTarget = listViewBase.TransformToVisual(null).TransformPoint(new Point(112 * 0.9, listViewBase.ActualHeight / 2)); var injector = InputInjector.TryCreate() ?? throw new InvalidOperationException("Failed to init the InputInjector"); @@ -42,6 +39,7 @@ public async Task When_ItemClicked_SelectsCorrectIndex() finger.Release(); Assert.AreEqual(0, listViewBase.SelectedIndex); + Assert.IsTrue(loggingSelectionInfo.IsSelected(0), "The item at index 0 should be selected."); Assert.IsTrue(loggingSelectionInfo.MethodLog.All(m => !m.Contains("MoveCurrent", StringComparison.OrdinalIgnoreCase))); } } diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/LoggingSelectionInfo.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/LoggingSelectionInfo.cs index 8730ef614b79..08efb0c690d6 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/LoggingSelectionInfo.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/LoggingSelectionInfo.cs @@ -1,156 +1,287 @@ #if HAS_UNO +using System; +using System.Collections; using System.Collections.Generic; +using System.Collections.Specialized; +using System.ComponentModel; using Microsoft.UI.Xaml.Data; +using Windows.Foundation.Collections; using Windows.Foundation; +using System.Runtime.CompilerServices; using System.Threading.Tasks; -using System; -using System.Collections; -using Microsoft.UI.Xaml; -internal class LoggingSelectionInfo : CollectionView, ISelectionInfo +internal class LoggingSelectionInfo : ICollectionView, ISelectionInfo, IList, INotifyCollectionChanged, INotifyPropertyChanged, IObservableVector { - public List MethodLog { get; } = new List(); + private readonly List _collection = new List(); + private readonly HashSet _selectedIndices = new HashSet(); + private object _currentItem; + private int _currentPosition = -1; - public LoggingSelectionInfo(IEnumerable collection, bool isGrouped, PropertyPath itemsPath) - : base(collection, isGrouped, itemsPath) + public LoggingSelectionInfo(IEnumerable items) { + _collection.AddRange(items); + if (_collection.Count > 0) + { + _currentItem = _collection[0]; + _currentPosition = 0; + } } - public void SelectRange(ItemIndexRange itemIndexRange) - { - MethodLog.Add($"SelectRange({itemIndexRange.FirstIndex}, {itemIndexRange.Length})"); - } + #region ICollectionView Implementation - public void DeselectRange(ItemIndexRange itemIndexRange) + public event CurrentChangingEventHandler CurrentChanging; + public event EventHandler CurrentChanged; + + public object CurrentItem => _currentItem; + + public int CurrentPosition => _currentPosition; + + public bool IsCurrentAfterLast => _currentPosition >= _collection.Count; + + public bool IsCurrentBeforeFirst => _currentPosition < 0; + + public bool MoveCurrentTo(object item) { - MethodLog.Add($"DeselectRange({itemIndexRange.FirstIndex}, {itemIndexRange.Length})"); + int index = _collection.IndexOf(item); + return MoveCurrentToPosition(index); } - public bool IsSelected(int index) + public bool MoveCurrentToPosition(int index) { - MethodLog.Add($"IsSelected({index})"); + if (index >= 0 && index < _collection.Count) + { + if (_currentPosition != index) + { + OnCurrentChanging(); + _currentItem = _collection[index]; + _currentPosition = index; + OnCurrentChanged(); + } + return true; + } return false; } - public IReadOnlyList GetSelectedRanges() - { - MethodLog.Add("GetSelectedRanges()"); - return new List(); - } + public bool MoveCurrentToFirst() => MoveCurrentToPosition(0); + + public bool MoveCurrentToLast() => MoveCurrentToPosition(_collection.Count - 1); + + public bool MoveCurrentToNext() => MoveCurrentToPosition(_currentPosition + 1); + + public bool MoveCurrentToPrevious() => MoveCurrentToPosition(_currentPosition - 1); + + public Predicate Filter { get; set; } + + public bool CanFilter => Filter != null; - public void LogAndMoveCurrentToPosition(int index) + public bool Contains(object item) => _collection.Contains(item); + + public IComparer SortDescriptions { get; set; } + + public bool CanSort => SortDescriptions != null; + + public IObservableVector CollectionGroups { get; } = new ObservableVector(); + + public bool CanGroup => false; + + public object GetItemAt(int index) => _collection[index]; + + public int IndexOf(object item) => _collection.IndexOf(item); + + public bool HasMoreItems => false; + + public IAsyncOperation LoadMoreItemsAsync(uint count) { - MethodLog.Add($"MoveCurrentToPosition({index})"); - base.MoveCurrentToPosition(index); + throw new NotImplementedException("LoadMoreItemsAsync is not implemented."); } - public void LogAndMoveCurrentTo(object item) + protected virtual void OnCurrentChanged() { - MethodLog.Add($"MoveCurrentTo({item})"); - base.MoveCurrentTo(item); + CurrentChanged?.Invoke(this, EventArgs.Empty); } - public void LogAndMoveCurrentToFirst() + protected virtual void OnCurrentChanging() { - MethodLog.Add("MoveCurrentToFirst()"); - base.MoveCurrentToFirst(); + CurrentChanging?.Invoke(this, new CurrentChangingEventArgs(false)); } - public void LogAndMoveCurrentToLast() + #endregion + + #region ISelectionInfo Implementation + + public void SelectRange(ItemIndexRange itemIndexRange) { - MethodLog.Add("MoveCurrentToLast()"); - base.MoveCurrentToLast(); + for (int i = itemIndexRange.FirstIndex; i < itemIndexRange.FirstIndex + itemIndexRange.Length; i++) + { + if (i >= 0 && i < _collection.Count) + { + _selectedIndices.Add(i); + } + } + OnSelectionChanged(); } - public void LogAndMoveCurrentToNext() + public void DeselectRange(ItemIndexRange itemIndexRange) { - MethodLog.Add("MoveCurrentToNext()"); - base.MoveCurrentToNext(); + for (int i = itemIndexRange.FirstIndex; i < itemIndexRange.FirstIndex + itemIndexRange.Length; i++) + { + _selectedIndices.Remove(i); + } + OnSelectionChanged(); } - public void LogAndMoveCurrentToPrevious() + public bool IsSelected(int index) { - MethodLog.Add("MoveCurrentToPrevious()"); - base.MoveCurrentToPrevious(); + return _selectedIndices.Contains(index); } - private bool IsUsingISelectionInfo => true; - - public void SafeMoveCurrentTo(object item) + public IReadOnlyList GetSelectedRanges() { - if (IsUsingISelectionInfo) + if (_selectedIndices.Count == 0) { - MethodLog.Add("MoveCurrentTo() - Skipped due to ISelectionInfo usage"); + return new List(); } - else + + List ranges = new List(); + List sortedIndices = new List(_selectedIndices); + sortedIndices.Sort(); + + int rangeStart = sortedIndices[0]; + int rangeLength = 1; + + for (int i = 1; i < sortedIndices.Count; i++) { - LogAndMoveCurrentTo(item); + if (sortedIndices[i] == sortedIndices[i - 1] + 1) + { + rangeLength++; + } + else + { + ranges.Add(new ItemIndexRange(rangeStart, (uint)rangeLength)); + rangeStart = sortedIndices[i]; + rangeLength = 1; + } } + + ranges.Add(new ItemIndexRange(rangeStart, (uint)rangeLength)); + return ranges; } - public void SafeMoveCurrentToPosition(int index) + protected virtual void OnSelectionChanged() { - if (index == -1) - { - MethodLog.Add("MoveCurrentToPosition() - Skipped due to invalid index (-1)"); - return; - } - if (IsUsingISelectionInfo) - { - MethodLog.Add("MoveCurrentToPosition() - Skipped due to ISelectionInfo usage"); - } - else - { - LogAndMoveCurrentToPosition(index); - } + // Raise collection changed event if necessary + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); } - public void SafeMoveCurrentToFirst() + #endregion + + #region IList and ICollection Implementation + + public int Count => _collection.Count; + + public bool IsReadOnly => false; + + public object this[int index] { - if (IsUsingISelectionInfo) + get => _collection[index]; + set { - MethodLog.Add("MoveCurrentToFirst() - Skipped due to ISelectionInfo usage"); - } - else - { - LogAndMoveCurrentToFirst(); + if (_collection[index] != value) + { + _collection[index] = value; + OnPropertyChanged($"Item[{index}]"); + } } } - public void SafeMoveCurrentToLast() + public void Add(object item) { - if (IsUsingISelectionInfo) - { - MethodLog.Add("MoveCurrentToLast() - Skipped due to ISelectionInfo usage"); - } - else - { - LogAndMoveCurrentToLast(); - } + _collection.Add(item); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item)); + OnVectorChanged(CollectionChange.ItemInserted, _collection.Count - 1); } - public void SafeMoveCurrentToNext() + public void Insert(int index, object item) { - if (IsUsingISelectionInfo) - { - MethodLog.Add("MoveCurrentToNext() - Skipped due to ISelectionInfo usage"); - } - else - { - LogAndMoveCurrentToNext(); - } + _collection.Insert(index, item); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index)); + OnVectorChanged(CollectionChange.ItemInserted, index); } - public void SafeMoveCurrentToPrevious() + public void RemoveAt(int index) { - if (IsUsingISelectionInfo) - { - MethodLog.Add("MoveCurrentToPrevious() - Skipped due to ISelectionInfo usage"); - } - else + var removedItem = _collection[index]; + _collection.RemoveAt(index); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, removedItem, index)); + OnVectorChanged(CollectionChange.ItemRemoved, index); + } + + public void Clear() + { + _collection.Clear(); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); + OnVectorChanged(CollectionChange.Reset, 0); + } + + public void CopyTo(object[] array, int arrayIndex) + { + _collection.CopyTo(array, arrayIndex); + } + + public bool Remove(object item) + { + var index = _collection.IndexOf(item); + if (index >= 0) { - LogAndMoveCurrentToPrevious(); + _collection.RemoveAt(index); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, item, index)); + OnVectorChanged(CollectionChange.ItemRemoved, index); + return true; } + return false; + } + + #endregion + + #region IEnumerable Implementation + + public IEnumerator GetEnumerator() => _collection.GetEnumerator(); + + IEnumerator IEnumerable.GetEnumerator() => _collection.GetEnumerator(); + + #endregion + + #region INotifyCollectionChanged Implementation + + public event NotifyCollectionChangedEventHandler CollectionChanged; + + protected virtual void OnCollectionChanged(NotifyCollectionChangedEventArgs e) + { + CollectionChanged?.Invoke(this, e); } + + #endregion + + #region INotifyPropertyChanged Implementation + + public event PropertyChangedEventHandler PropertyChanged; + + protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null) + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); + } + + #endregion + + #region IObservableVector Implementation + + public event VectorChangedEventHandler VectorChanged; + + protected virtual void OnVectorChanged(CollectionChange change, int index) + { + VectorChanged?.Invoke(this, new VectorChangedEventArgs(change, (uint)index)); + } + + #endregion } #endif diff --git a/src/Uno.UI/UI/Xaml/Controls/ListViewBase/ListViewBase.cs b/src/Uno.UI/UI/Xaml/Controls/ListViewBase/ListViewBase.cs index b6fbe597f0f0..1cdecf75ee48 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ListViewBase/ListViewBase.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ListViewBase/ListViewBase.cs @@ -609,7 +609,8 @@ internal override void OnItemClicked(int clickedIndex, VirtualKeyModifiers modif void SingleSelectionCase() { - //Making sure that the ItemsSource is a CollectionView and not ISelectionInfo as it breaks the selection logic + //The `ISelectionInfo` handled directly by the `ItemsSource` has precedence over the `ICollectionView`. + //If an object implements both interfaces, as WinUI, we make sure to use only the `ISelectionInfo` to avoid conflicting interaction. if (ItemsSource is ICollectionView collectionView and not ISelectionInfo) { //NOTE: Windows seems to call MoveCurrentTo(item); we set position instead to have expected behavior when you have duplicate items in the list.