From 75d0dc3009efcb7c469295fc735679113dba0307 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 8 Mar 2023 10:16:01 -0500 Subject: [PATCH 1/4] fix: Selection operator might fail when entity tracking is wrongly implemented --- src/Uno.Extensions.Reactive/Core/Axes/SelectionInfo.cs | 2 +- .../Presentation/Bindings/BindableImmutableList.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Uno.Extensions.Reactive/Core/Axes/SelectionInfo.cs b/src/Uno.Extensions.Reactive/Core/Axes/SelectionInfo.cs index 07ddd71e2a..061eec890b 100644 --- a/src/Uno.Extensions.Reactive/Core/Axes/SelectionInfo.cs +++ b/src/Uno.Extensions.Reactive/Core/Axes/SelectionInfo.cs @@ -136,7 +136,7 @@ internal bool TryGetSelectedItem(IImmutableList items, [NotNullWhen(true)] } selectedItem = default!; - return true; + return false; } internal bool TryGetSelectedIndex(IImmutableList items, [NotNullWhen(true)] out uint? selectedIndex, bool failIfOutOfRange = true, bool failIfMultiple = false) diff --git a/src/Uno.Extensions.Reactive/Presentation/Bindings/BindableImmutableList.cs b/src/Uno.Extensions.Reactive/Presentation/Bindings/BindableImmutableList.cs index 91af8a26c5..9489ed9b60 100644 --- a/src/Uno.Extensions.Reactive/Presentation/Bindings/BindableImmutableList.cs +++ b/src/Uno.Extensions.Reactive/Presentation/Bindings/BindableImmutableList.cs @@ -47,7 +47,7 @@ private protected override CollectionChangeSet GetChanges(IImmutableList< // '_analyzer' might be null when the base.ctor subscribe to the 'property' and invokes the 'OnOwnerUpdated' // That's should not happen since the `AutoInit` has been disable, but for safety we fallback on ListFeed.DefaultAnalyzer. // This is valid as in that case the 'previous' will be null/empty anyway. - => (_analyzer ?? ListFeed.DefaultAnalyzer).GetChanges(previous ?? ImmutableList.Empty, current); + => (_analyzer ?? ListFeed.DefaultAnalyzer).GetChanges(previous ?? ImmutableList.Empty, current ?? ImmutableList.Empty); private protected override IImmutableList Replace(IImmutableList? items, TItem oldItem, TItem newItem) { From 58b3dc454054d78a2b4a7191ea0762f221d0c4b9 Mon Sep 17 00:00:00 2001 From: David Date: Tue, 21 Mar 2023 16:11:26 -0400 Subject: [PATCH 2/4] feat: Add ability to Move items in the DiffImmutableList --- .../Differential/DifferentialImmutableList.cs | 70 ++++++++++++------- .../Facades/Differential/MoveNode.cs | 17 +++++ 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/src/Uno.Extensions.Reactive/Collections/Facades/Differential/DifferentialImmutableList.cs b/src/Uno.Extensions.Reactive/Collections/Facades/Differential/DifferentialImmutableList.cs index 84fa5f7dc9..5c2ced6d3a 100644 --- a/src/Uno.Extensions.Reactive/Collections/Facades/Differential/DifferentialImmutableList.cs +++ b/src/Uno.Extensions.Reactive/Collections/Facades/Differential/DifferentialImmutableList.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics.Contracts; using System.Linq; using System.Runtime.CompilerServices; using System.Text; @@ -49,39 +50,39 @@ object IList.this[int index] } /// - public int IndexOf(T item, int index, int count, IEqualityComparer equalityComparer) + [Pure] public int IndexOf(T item, int index, int count, IEqualityComparer equalityComparer) => Head.IndexOf(item, index, equalityComparer?.ToEqualityComparer()); /// - public int LastIndexOf(T item, int index, int count, IEqualityComparer equalityComparer) + [Pure] public int LastIndexOf(T item, int index, int count, IEqualityComparer equalityComparer) => throw new NotSupportedException(); /// - public DifferentialImmutableList Add(T value) => new (new AddNode(Head, value!, Count)); - IImmutableList IImmutableList.Add(T value) => Add(value); + [Pure] public DifferentialImmutableList Add(T value) => new (new AddNode(Head, value!, Count)); + [Pure] IImmutableList IImmutableList.Add(T value) => Add(value); /// - public DifferentialImmutableList AddRange(IImmutableList items) => new(new AddNode(Head, items.AsUntypedList(), Count)); - IImmutableList IImmutableList.AddRange(IEnumerable items) => AddRange(items.ToImmutableList()); + [Pure] public DifferentialImmutableList AddRange(IImmutableList items) => new(new AddNode(Head, items.AsUntypedList(), Count)); + [Pure] IImmutableList IImmutableList.AddRange(IEnumerable items) => AddRange(items.ToImmutableList()); /// - public DifferentialImmutableList Insert(int index, T element) => new(new AddNode(Head, element!, index)); - IImmutableList IImmutableList.Insert(int index, T element) => Insert(index, element); + [Pure] public DifferentialImmutableList Insert(int index, T element) => new(new AddNode(Head, element!, index)); + [Pure] IImmutableList IImmutableList.Insert(int index, T element) => Insert(index, element); /// - public DifferentialImmutableList InsertRange(int index, IImmutableList items) => new(new AddNode(Head, items.AsUntypedList(), index)); - IImmutableList IImmutableList.InsertRange(int index, IEnumerable items) => InsertRange(index, items.ToImmutableList()); + [Pure] public DifferentialImmutableList InsertRange(int index, IImmutableList items) => new(new AddNode(Head, items.AsUntypedList(), index)); + [Pure] IImmutableList IImmutableList.InsertRange(int index, IEnumerable items) => InsertRange(index, items.ToImmutableList()); /// - public DifferentialImmutableList Remove(T value, IEqualityComparer equalityComparer) + [Pure] public DifferentialImmutableList Remove(T value, IEqualityComparer equalityComparer) { var index = Head.IndexOf(value, 0, equalityComparer?.ToEqualityComparer()); return new(new RemoveNode(Head, value, index)); } - IImmutableList IImmutableList.Remove(T value, IEqualityComparer equalityComparer) => Remove(value, equalityComparer); + [Pure] IImmutableList IImmutableList.Remove(T value, IEqualityComparer equalityComparer) => Remove(value, equalityComparer); /// - public DifferentialImmutableList RemoveAll(Predicate match) + [Pure] public DifferentialImmutableList RemoveAll(Predicate match) { var head = Head; for (var i = Count - 1; i >= 0; i--) @@ -95,32 +96,49 @@ public DifferentialImmutableList RemoveAll(Predicate match) return new(head); } - IImmutableList IImmutableList.RemoveAll(Predicate match) => RemoveAll(match); + [Pure] IImmutableList IImmutableList.RemoveAll(Predicate match) => RemoveAll(match); /// //public DifferentialImmutableList RemoveRange(IEnumerable items, IEqualityComparer equalityComparer) - IImmutableList IImmutableList.RemoveRange(IEnumerable items, IEqualityComparer equalityComparer) => throw new NotSupportedException(); + [Pure] IImmutableList IImmutableList.RemoveRange(IEnumerable items, IEqualityComparer equalityComparer) => throw new NotSupportedException(); /// - public DifferentialImmutableList RemoveRange(int index, int count) => new(new RemoveNode(Head, index, count)); - IImmutableList IImmutableList.RemoveRange(int index, int count) => RemoveRange(index, count); + [Pure] public DifferentialImmutableList RemoveRange(int index, int count) => new(new RemoveNode(Head, index, count)); + [Pure] IImmutableList IImmutableList.RemoveRange(int index, int count) => RemoveRange(index, count); /// - public DifferentialImmutableList RemoveAt(int index) => new(new RemoveNode(Head, index, 1)); - IImmutableList IImmutableList.RemoveAt(int index) => RemoveAt(index); + [Pure] public DifferentialImmutableList RemoveAt(int index) => new(new RemoveNode(Head, index, 1)); + [Pure] IImmutableList IImmutableList.RemoveAt(int index) => RemoveAt(index); /// - public DifferentialImmutableList Clear() => new(new EmptyNode()); - IImmutableList IImmutableList.Clear() => Clear(); + [Pure] public DifferentialImmutableList Clear() => new(new EmptyNode()); + [Pure] IImmutableList IImmutableList.Clear() => Clear(); /// - public DifferentialImmutableList SetItem(int index, T value) => new(new ReplaceNode(Head, value, index)); - IImmutableList IImmutableList.SetItem(int index, T value) => SetItem(index, value); + [Pure] public DifferentialImmutableList SetItem(int index, T value) => new(new ReplaceNode(Head, value, index)); + [Pure] IImmutableList IImmutableList.SetItem(int index, T value) => SetItem(index, value); /// //public DifferentialImmutableList Replace(T oldValue, T newValue, IEqualityComparer equalityComparer) => throw new NotSupportedException(); - IImmutableList IImmutableList.Replace(T oldValue, T newValue, IEqualityComparer equalityComparer) => throw new NotSupportedException(); + [Pure] IImmutableList IImmutableList.Replace(T oldValue, T newValue, IEqualityComparer equalityComparer) => throw new NotSupportedException(); + + /// + /// Moves a range of items within the collection + /// + /// The original index of moved items + /// The target index of moved items + /// The count of moved items + /// A new instance of immutable list where items has been moved. + [Pure] public DifferentialImmutableList MoveRange(int fromIndex, int toIndex, int count) => new(new MoveNode(Head, fromIndex, toIndex, count)); + + /// + /// Moves a range of items within the collection + /// + /// Index of the element that is being replaced. + /// The new value + /// A new instance of immutable list where item has been replaced. + [Pure] public DifferentialImmutableList ReplaceAt(int index, T newValue) => new(new ReplaceNode(Head, Head.ElementAt(index), newValue, index)); /// bool IList.Contains(object value) => ((IList)this).IndexOf(value) >= 0; @@ -144,10 +162,10 @@ public DifferentialImmutableList RemoveAll(Predicate match) void IList.Clear() => throw NotSupported(); /// - public IEnumerator GetEnumerator() => new Enumerator(Head); + [Pure] public IEnumerator GetEnumerator() => new Enumerator(Head); /// - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + [Pure] IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); /// public void CopyTo(Array array, int index) diff --git a/src/Uno.Extensions.Reactive/Collections/Facades/Differential/MoveNode.cs b/src/Uno.Extensions.Reactive/Collections/Facades/Differential/MoveNode.cs index 206a363f9e..dabedf922c 100644 --- a/src/Uno.Extensions.Reactive/Collections/Facades/Differential/MoveNode.cs +++ b/src/Uno.Extensions.Reactive/Collections/Facades/Differential/MoveNode.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Specialized; using System.Linq; +using Uno.Extensions.Reactive.Utils; namespace Uno.Extensions.Collections.Facades.Differential; @@ -30,6 +31,22 @@ public MoveNode(IDifferentialCollectionNode previous, NotifyCollectionChangedEve _isBackward = _oldFromIndex > _newFromIndex; } + public MoveNode(IDifferentialCollectionNode previous, int fromIndex, int toIndex, int count) + { + Previous = previous; + Count = previous.Count; + + _moved = previous.AsList().Slice(fromIndex, count); + _movedCount = count; + + _oldFromIndex = fromIndex; + _oldToIndex = _oldFromIndex + _movedCount; + _newFromIndex = toIndex; + _newToIndex = _newFromIndex + _movedCount; + + _isBackward = _oldFromIndex > _newFromIndex; + } + /// public IDifferentialCollectionNode Previous { get; } From cca89ad57f11e992cea5c607ff48fd46327edf27 Mon Sep 17 00:00:00 2001 From: David Date: Tue, 21 Mar 2023 16:14:46 -0400 Subject: [PATCH 3/4] chore: Improve CollectionAnalyzer to flag replace as simple entity update --- .../ConstraintParts/Items.cs | 8 ++--- .../ConstraintParts/ItemsChanged.cs | 8 ++--- .../RichNotifyCollectionChangedEventArgs.cs | 35 +++++++++++++++---- .../CollectionAnalyzer.ChangesBuffer.cs | 8 ++--- .../Tracking/CollectionAnalyzer.Core.cs | 19 +++++----- .../Tracking/CollectionAnalyzer._Replace.cs | 10 ++++-- 6 files changed, 57 insertions(+), 31 deletions(-) diff --git a/src/Uno.Extensions.Reactive.Testing/ConstraintParts/Items.cs b/src/Uno.Extensions.Reactive.Testing/ConstraintParts/Items.cs index 6656bdac6f..db9cf70737 100644 --- a/src/Uno.Extensions.Reactive.Testing/ConstraintParts/Items.cs +++ b/src/Uno.Extensions.Reactive.Testing/ConstraintParts/Items.cs @@ -35,11 +35,11 @@ public static ItemsChanged Remove(int at, params T[] items) public static ItemsChanged Remove(int at, IEnumerable items) => ItemsChanged.Remove(at, items); - public static ItemsChanged Replace(int at, IEnumerable oldItems, IEnumerable newItems) - => ItemsChanged.Replace(at, oldItems, newItems); + public static ItemsChanged Replace(int at, IEnumerable oldItems, IEnumerable newItems, bool isReplaceOfSameEntities = true) + => ItemsChanged.Replace(at, oldItems, newItems, isReplaceOfSameEntities); - public static ItemsChanged Replace(int at, T oldItem, T newItem) - => ItemsChanged.Replace(at, oldItem, newItem); + public static ItemsChanged Replace(int at, T oldItem, T newItem, bool isReplaceOfSameEntity = true) + => ItemsChanged.Replace(at, oldItem, newItem, isReplaceOfSameEntity); public static ItemsChanged Move(int from, int to, params T[] items) => ItemsChanged.Move(from, to, items); diff --git a/src/Uno.Extensions.Reactive.Testing/ConstraintParts/ItemsChanged.cs b/src/Uno.Extensions.Reactive.Testing/ConstraintParts/ItemsChanged.cs index 2abfc54faa..ac06b0ba8f 100644 --- a/src/Uno.Extensions.Reactive.Testing/ConstraintParts/ItemsChanged.cs +++ b/src/Uno.Extensions.Reactive.Testing/ConstraintParts/ItemsChanged.cs @@ -27,11 +27,11 @@ public static ItemsChanged Remove(int index, params T[] items) public static ItemsChanged Remove(int index, IEnumerable items) => new(RichNotifyCollectionChangedEventArgs.RemoveSome(items.ToList(), index)); - public static ItemsChanged Replace(int index, IEnumerable oldItems, IEnumerable newItems) - => new(RichNotifyCollectionChangedEventArgs.ReplaceSome(oldItems.ToList(), newItems.ToList(), index)); + public static ItemsChanged Replace(int index, IEnumerable oldItems, IEnumerable newItems, bool isReplaceOfSameEntities) + => new(RichNotifyCollectionChangedEventArgs.ReplaceSome(oldItems.ToList(), newItems.ToList(), index, isReplaceOfSameEntities)); - public static ItemsChanged Replace(int index, T oldItem, T newItem) - => new(RichNotifyCollectionChangedEventArgs.Replace(oldItem, newItem, index)); + public static ItemsChanged Replace(int index, T oldItem, T newItem, bool isReplaceOfSameEntity) + => new(RichNotifyCollectionChangedEventArgs.Replace(oldItem, newItem, index, isReplaceOfSameEntity)); public static ItemsChanged Move(int oldIndex, int newIndex, params T[] items) => new(RichNotifyCollectionChangedEventArgs.MoveSome(items.ToList(), oldIndex, newIndex)); diff --git a/src/Uno.Extensions.Reactive/Collections/RichNotifyCollectionChangedEventArgs.cs b/src/Uno.Extensions.Reactive/Collections/RichNotifyCollectionChangedEventArgs.cs index 2a03c7cd67..f58650a1c4 100644 --- a/src/Uno.Extensions.Reactive/Collections/RichNotifyCollectionChangedEventArgs.cs +++ b/src/Uno.Extensions.Reactive/Collections/RichNotifyCollectionChangedEventArgs.cs @@ -61,26 +61,40 @@ public static RichNotifyCollectionChangedEventArgs RemoveSome(IList items, /// /// Creates a collection changed event args /// - public static RichNotifyCollectionChangedEventArgs Replace(object? oldItem, object? newItem, int index) + public static RichNotifyCollectionChangedEventArgs Replace(object? oldItem, object? newItem, int index, bool isReplaceOfSameEntity = false) => new(NotifyCollectionChangedAction.Replace, new[] { newItem }, new[] { oldItem }, index); /// /// Creates a collection changed event args /// - public static RichNotifyCollectionChangedEventArgs Replace(T oldItem, T newItem, int index) - => new(NotifyCollectionChangedAction.Replace, new[] { newItem }, new[] { oldItem }, index); + public static RichNotifyCollectionChangedEventArgs Replace(T oldItem, T newItem, int index, bool isReplaceOfSameEntity = false) + => new(NotifyCollectionChangedAction.Replace, new[] { newItem }, new[] { oldItem }, index) { IsReplaceOfSameEntities = isReplaceOfSameEntity}; /// /// Creates a collection changed event args /// - public static RichNotifyCollectionChangedEventArgs ReplaceSome(IList oldItems, IList newItems, int index) - => new(NotifyCollectionChangedAction.Replace, newItems, oldItems, index); + public static RichNotifyCollectionChangedEventArgs ReplaceSome(IList oldItems, IList newItems, int index, bool isReplaceOfSameEntities = false) + { + if (isReplaceOfSameEntities && oldItems.Count != newItems.Count) + { + throw new InvalidOperationException("For a replace flagged with isReplaceOfSameEntities (a.k.a. update), the number of oldItems must be the same as newItems."); + } + + return new(NotifyCollectionChangedAction.Replace, newItems, oldItems, index) { IsReplaceOfSameEntities = isReplaceOfSameEntities }; + } /// /// Creates a collection changed event args /// - public static RichNotifyCollectionChangedEventArgs ReplaceSome(IList oldItems, IList newItems, int index) - => new(NotifyCollectionChangedAction.Replace, newItems.AsUntypedList(), oldItems.AsUntypedList(), index); + public static RichNotifyCollectionChangedEventArgs ReplaceSome(IList oldItems, IList newItems, int index, bool isReplaceOfSameEntities = false) + { + if (isReplaceOfSameEntities && oldItems.Count != newItems.Count) + { + throw new InvalidOperationException("For a replace flagged with isReplaceOfSameEntities (a.k.a. update), the number of oldItems must be the same as newItems."); + } + + return new(NotifyCollectionChangedAction.Replace, newItems.AsUntypedList(), oldItems.AsUntypedList(), index) { IsReplaceOfSameEntities = isReplaceOfSameEntities }; + } /// /// Creates a collection changed event args @@ -191,6 +205,13 @@ private RichNotifyCollectionChangedEventArgs(NotifyCollectionChangedAction actio { } + /// + /// For a Replace change, indicates if entities are actually new versions of the same (same Id / Key, cf. KeyEquality) entities. + /// If true, this change should be considered as an "update" instead of a real "replace". + /// (I.e. entities have same IDs but have some fields that has been updated). + /// + public bool IsReplaceOfSameEntities { get; private set; } + /// /// Gets the old items for a reset change /// diff --git a/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer.ChangesBuffer.cs b/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer.ChangesBuffer.cs index 5eb6ad2962..efa3dd5d44 100644 --- a/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer.ChangesBuffer.cs +++ b/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer.ChangesBuffer.cs @@ -95,19 +95,19 @@ static void SeekToTail(ref Change? tail) } } - public void Update(T oldItem, T newItem, int index) + public void Keep(T oldItem, T newItem, int index) { // As they don't impact the 'result' index, replace-instance are buffered separately and will be inserted at the top of the changes collection. // Note: We use a single '_Same' node for all updates - UpdateOrReplace(ref _sameHead, oldItem, newItem, index, (i, o) => new _Same(i, o)); ; + KeepOrReplace(ref _sameHead, oldItem, newItem, index, (i, o) => new _Same(i, o)); ; } public void Replace(T oldItem, T newItem, int index) { // As they don't impact the 'result' index, replaces are buffered separately and will be inserted at the top of the changes collection. - UpdateOrReplace(ref _replaceHead, oldItem, newItem, index, (i, o) => new _Replace(i, o)); + KeepOrReplace(ref _replaceHead, oldItem, newItem, index, (i, o) => new _Replace(i, o, isReplaceOfSameEntities: true)); } public void Add(T item, int at, int max) @@ -137,7 +137,7 @@ public void Remove(T item, int at) remove.Append(item); } - private void UpdateOrReplace(ref TNode? head, T oldItem, T newItem, int index, Func factory) + private void KeepOrReplace(ref TNode? head, T oldItem, T newItem, int index, Func factory) where TNode : EntityChange { if (head is null) diff --git a/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer.Core.cs b/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer.Core.cs index 80cb70859e..17be7b204f 100644 --- a/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer.Core.cs +++ b/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer.Core.cs @@ -164,22 +164,23 @@ internal static CollectionUpdater ToUpdater(Change? changed, ICollectionUp * (we already determined that they have the same key using the 'itemComparer'). * * If the 'itemVersionComparer' is 'null' we assume that there is no - * notion of version of an item and we rely only on the `itemComparer` to check equality. + * notion of version of an item and we rely only on the `itemComparer` to check (true/full) equality. * Note: in this case we don't raise any 'Replace'. * - * If the 'itemVersionComparer' returns 'true' (or if it's 'null') that means that they are not only KeyEquals but also Equals. + * If the 'itemVersionComparer' returns 'true' that means that they are not only KeyEquals but also Equals. * So as we are usually working with immutable objects, we can ignore that a new instance is available and - * we don't raise any event ('changesBuffer.Update'). Note: We still have to notify the visitor! + * we don't raise any event ('changesBuffer.Update'). Note: We still have to notify the visitor! * - * If the 'itemVersionComparer' returns 'false' that means items are only key equals, but not same version. - * So we have to raise a 'Replace' event. + * If the 'itemVersionComparer' returns 'false' that means items are only key equals, but not same version. + * So we have to raise a 'Replace' event. + * Note: The replace event is flagged as "SameEntity" so the view can raise some "INPC.Item[]" instead of real "INCC.Replace" */ void UpdateInstanceFromOld(T oldItem, int oldIndex, int newIndex) { if (versionComparer is null) { - buffer.Update(oldItem, oldItem, oldIndex); + buffer.Keep(oldItem, oldItem, oldIndex); return; } @@ -187,7 +188,7 @@ void UpdateInstanceFromOld(T oldItem, int oldIndex, int newIndex) var newItem = newItems.ElementAt(newIndex); if (versionComparer(oldItem, newItem)) { - buffer.Update(oldItem, newItem, oldIndex); + buffer.Keep(oldItem, newItem, oldIndex); } else { @@ -199,7 +200,7 @@ void UpdateInstanceFromNew(T newItem, int oldIndex) { if (versionComparer is null) { - buffer.Update(newItem, newItem, oldIndex); + buffer.Keep(newItem, newItem, oldIndex); return; } @@ -207,7 +208,7 @@ void UpdateInstanceFromNew(T newItem, int oldIndex) var oldItem = oldItems.ElementAt(oldIndex); if (versionComparer(oldItem, newItem)) { - buffer.Update(oldItem, newItem, oldIndex); + buffer.Keep(oldItem, newItem, oldIndex); } else { diff --git a/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer._Replace.cs b/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer._Replace.cs index d4c0aff751..9790645906 100644 --- a/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer._Replace.cs +++ b/src/Uno.Extensions.Reactive/Collections/Tracking/CollectionAnalyzer._Replace.cs @@ -11,14 +11,17 @@ partial class CollectionAnalyzer { private sealed class _Replace : EntityChange { + private readonly bool _isReplaceOfSameEntities; + /// - public _Replace(int at, int indexOffset) + public _Replace(int at, int indexOffset, bool isReplaceOfSameEntities = true) : base(at, indexOffset) { + _isReplaceOfSameEntities = isReplaceOfSameEntities; } public override RichNotifyCollectionChangedEventArgs ToEvent() - => RichNotifyCollectionChangedEventArgs.ReplaceSome(_oldItems, _newItems, Starts + _indexOffset); + => RichNotifyCollectionChangedEventArgs.ReplaceSome(_oldItems, _newItems, Starts + _indexOffset, _isReplaceOfSameEntities); /// protected internal override void Visit(ICollectionChangeSetVisitor visitor) @@ -87,7 +90,8 @@ private RichNotifyCollectionChangedEventArgs CreateEvent(int from, int count) => RichNotifyCollectionChangedEventArgs.ReplaceSome( _oldItems.Slice(from, count), _newItems.Slice(from, count), - Starts + _indexOffset + from); + Starts + _indexOffset + from, + _isReplaceOfSameEntities); /// public override string ToString() From b4cd152346e14655cff5e1e62acbf16fd55df72d Mon Sep 17 00:00:00 2001 From: David Date: Fri, 24 Mar 2023 10:28:12 -0400 Subject: [PATCH 4/4] fix(mvux): Fix multi selection support --- src/Directory.Build.props | 5 + .../Given_BindableCollection_Selection.cs | 44 +- .../Presentation/Bindings/BindableListFeed.cs | 33 +- .../Collections/Data/BranchStrategy.cs | 2 +- .../Bindings/Collections/Data/LeafStrategy.cs | 2 +- .../Collections/Facets/SelectionFacet.cs | 417 ++++++++++++------ .../Collections/Services/SelectionService.cs | 1 - .../Utils/SelectionHelper.cs | 2 +- 8 files changed, 339 insertions(+), 167 deletions(-) diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 874cd9187f..0ceb736485 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -107,8 +107,13 @@ + $(DefineConstants);__WINDOWS__ 10.0.19041.0 + + + $(DefineConstants);__WINDOWS__ + False diff --git a/src/Uno.Extensions.Reactive.UI.Tests/Presentation/Bindings/Collections/Given_BindableCollection_Selection.cs b/src/Uno.Extensions.Reactive.UI.Tests/Presentation/Bindings/Collections/Given_BindableCollection_Selection.cs index ca8ed56d9b..5e32efa0ad 100644 --- a/src/Uno.Extensions.Reactive.UI.Tests/Presentation/Bindings/Collections/Given_BindableCollection_Selection.cs +++ b/src/Uno.Extensions.Reactive.UI.Tests/Presentation/Bindings/Collections/Given_BindableCollection_Selection.cs @@ -1,4 +1,6 @@ -using System; +#define __SKIA__ + +using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -7,7 +9,9 @@ using Windows.Foundation; using Microsoft.UI.Xaml.Automation.Peers; using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Data; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Uno.Extensions.Equality; using Uno.Extensions.Reactive.Testing; using Uno.UI.RuntimeTests; @@ -19,9 +23,11 @@ public partial class Given_BindableCollection_Selection : FeedTests { public partial class Given_BindableCollection_Selection_Model { - public IListState Items => ListState.Value(this, () => ImmutableList.Create(41, 42, 43)); + public IListState Items => ListState.Value(this, () => ImmutableList.Create(new(41), new(42), new(43))); } + public partial record MyItem([property:Key] int Value, int Version = 1); + [TestMethod] [InjectedPointer(PointerDeviceType.Mouse)] #if !__SKIA__ @@ -33,7 +39,7 @@ public async Task When_SelectSingleFromView_ListView() InputInjectorHelper.Current.Tap(items[1]); - await UIHelper.WaitFor(async ct => await vm.Items.GetSelectedItem(ct) == 42, CT); + await UIHelper.WaitFor(async ct => (await vm.Items.GetSelectedItem(ct))?.Value == 42, CT); } [TestMethod] @@ -50,7 +56,37 @@ public async Task When_SelectMultipleFromView_ListView() await UIHelper.WaitFor(async ct => { - return (await vm.Items.GetSelectedItems(ct)).SequenceEqual(new[] { 41, 42 }); + return (await vm.Items.GetSelectedItems(ct)).SequenceEqual(new MyItem[] { new(41), new(42) }); + }, CT); + } + + [TestMethod] + [InjectedPointer(PointerDeviceType.Mouse)] +#if !__SKIA__ + [Ignore("Pointer injection not supported yet on this platform")] +#endif + public async Task When_EditSingleSelectedItem_Then_SelectionPreserved() + { + var (vm, lv, items) = await SetupListView(ListViewSelectionMode.Single); + + InputInjectorHelper.Current.Tap(items[1]); + + await UIHelper.WaitFor(async ct => + { + // Selection is preserved on ... + return lv.SelectedItem is MyItem { Value: 42 } // ... the ListView ... + && ((ISelectionInfo)lv.ItemsSource).IsSelected(1) // ... the BindableCollection ... + && await vm.Items.GetSelectedItem(ct) is { Value: 42 }; // ... and the Feed! + }, CT); + + await vm.Model.Items.Update(items => items.Replace(items[1], items[1] with { Version = 2 }), CT); + + await UIHelper.WaitFor(async ct => + { + // Selection is preserved on ... + return lv.SelectedItem is MyItem { Value: 42, Version: 2 } // ... the ListView ... + && ((ISelectionInfo)lv.ItemsSource).IsSelected(1) // ... the BindableCollection ... + && await vm.Items.GetSelectedItem(ct) is { Value: 42, Version: 2 }; // ... and the Feed! }, CT); } diff --git a/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/BindableListFeed.cs b/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/BindableListFeed.cs index ed12d33c06..180e9d3e16 100644 --- a/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/BindableListFeed.cs +++ b/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/BindableListFeed.cs @@ -182,23 +182,24 @@ async ValueTask SetSelected(SelectionInfo info, CancellationToken ct) => await state.UpdateMessage(msg => msg.Selected(info).Set(BindableViewModelBase.BindingSource, collection), ct); async ValueTask Edit(Func change, CancellationToken ct) - => await state.UpdateMessage(msg => - { - // Note: The change might have been computed on an older version of the collection - // We are NOT validating this. It means that we cou;d get an out-of-range for remove for instance. - - var currentItems = msg.CurrentData.SomeOrDefault(); - var currentHead = currentItems switch + => await state.UpdateMessage( + msg => { - IDifferentialCollection diffCollection => diffCollection.Head, - null => new EmptyNode(), - _ => new ResetNode(currentItems) - }; - var newHead = change(currentHead); - var newItems = new DifferentialImmutableList(newHead) as IImmutableList; - - msg.Data(newItems).Set(BindableViewModelBase.BindingSource, collection); - }, + // Note: The change might have been computed on an older version of the collection + // We are NOT validating this. It means that we cou;d get an out-of-range for remove for instance. + + var currentItems = msg.CurrentData.SomeOrDefault(); + var currentHead = currentItems switch + { + IDifferentialCollection diffCollection => diffCollection.Head, + null => new EmptyNode(), + _ => new ResetNode(currentItems) + }; + var newHead = change(currentHead); + var newItems = new DifferentialImmutableList(newHead) as IImmutableList; + + msg.Data(newItems).Set(BindableViewModelBase.BindingSource, collection); + }, ct); return collection; diff --git a/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Data/BranchStrategy.cs b/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Data/BranchStrategy.cs index 3f816794b8..bb14c55218 100644 --- a/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Data/BranchStrategy.cs +++ b/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Data/BranchStrategy.cs @@ -84,7 +84,7 @@ public BranchStrategy(DataStructure dataStructure, uint dataLevel, CollectionAna // Init the flat tracking view (ie. the flatten view of the groups that can be consumed directly by the ICollectionView properties) var flatCollectionChanged = new FlatCollectionChangedFacet(() => view ?? throw new InvalidOperationException("The owner provider must be resolved lazily!")); - var flatSelectionFacet = new SelectionFacet(source, () => view ?? throw new InvalidOperationException("The owner provider must be resolved lazily!")); + var flatSelectionFacet = new SelectionFacet(source, flatCollectionChanged, () => view ?? throw new InvalidOperationException("The owner provider must be resolved lazily!")); var flatPaginationFacet = new PaginationFacet(source, flatCollectionChanged, extendedPropertiesFacet); // Init the groups tracking diff --git a/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Data/LeafStrategy.cs b/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Data/LeafStrategy.cs index 408c8865d8..fcd0c6a004 100644 --- a/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Data/LeafStrategy.cs +++ b/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Data/LeafStrategy.cs @@ -35,7 +35,7 @@ public LeafStrategy(DataStructure dataStructure, CollectionAnalyzer diffAnalyzer if (_isRoot) // I.e. Collection is not grouped { var paginationFacet = new PaginationFacet(source, collectionChangedFacet, extendedPropertiesFacet); - var selectionFacet = new SelectionFacet(source, () => view ?? throw new InvalidOperationException("The owner provider must be resolved lazily!")); + var selectionFacet = new SelectionFacet(source, collectionChangedFacet, () => view ?? throw new InvalidOperationException("The owner provider must be resolved lazily!")); var editionFacet = new EditionFacet(source, collectionFacet); view = new BasicView(collectionFacet, collectionChangedFacet, extendedPropertiesFacet, selectionFacet, paginationFacet, editionFacet); diff --git a/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Facets/SelectionFacet.cs b/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Facets/SelectionFacet.cs index 933951fcf6..0ca46f614e 100644 --- a/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Facets/SelectionFacet.cs +++ b/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Facets/SelectionFacet.cs @@ -2,72 +2,85 @@ using System; using System.Collections.Generic; +using System.Collections.Specialized; +using System.Diagnostics; using System.Linq; using System.Threading; using Windows.Foundation.Collections; +using Uno.Extensions.Collections; using Uno.Extensions.Reactive.Bindings.Collections.Services; using Uno.Extensions.Reactive.Dispatching; -namespace Uno.Extensions.Reactive.Bindings.Collections._BindableCollection.Facets +namespace Uno.Extensions.Reactive.Bindings.Collections._BindableCollection.Facets; + +/// +/// The selection facet of the ICollectionView +/// +internal class SelectionFacet : IDisposable, ISelectionInfo { - /// - /// The selection facet of the ICollectionView - /// - internal class SelectionFacet : IDisposable, ISelectionInfo + /* + * Note: The selection is sync between the ListView and the CollectionView only when SelectionMode is 'Single' + * + * For modes 'Multiple' and 'Extended' the ListView notifies using the `ISelectionInfo` interface. + * + * When we cycle between the selection modes, only the selection made in 'Single' is kept + * (ie. the selection made in other modes is flushed when mode change, and the 'None' doesn't alter the Current on the collection View) + * + * When we unselect the 'CurrentItem', list view calls 'MoveCurrentTo(null)'. + */ + + private readonly EventRegistrationTokenTable _currentChanged = new(); + private readonly EventRegistrationTokenTable _currentChanging = new(); + private readonly ISelectionService? _service; + private readonly Lazy> _target; + private readonly IDispatcher? _dispatcher; + private readonly CollectionChangedFacet _collectionChangedFacet; + + private bool _isInit; + + public SelectionFacet(IBindableCollectionViewSource source, CollectionChangedFacet collectionChangedFacet, Func < IObservableVector> target) { - /* - * Note: The selection is sync beetween the ListView and the CollectionView only when SelectionMode is 'Single' - * - * For modes 'Multiple'and 'Extended' the ListView doesn't notify the CollectionView in any way. - * - * When we cycle between the selection modes, only the selection made in 'Single' is kept - * (ie. the selection made in other modes is flushed when mode change, and the 'None' doesn't alter the Current on the collection View) - * - * When we unselect the 'CurrentItem', list view calls 'MoveCurrentTo(null)'. - * - */ + _service = source.GetService(typeof(ISelectionService)) as ISelectionService; + _target = new Lazy>(target, LazyThreadSafetyMode.None); + _dispatcher = source.Dispatcher; + _collectionChangedFacet = collectionChangedFacet; + } - private readonly EventRegistrationTokenTable _currentChanged = new(); - private readonly EventRegistrationTokenTable _currentChanging = new(); - private readonly ISelectionService? _service; - private readonly Lazy> _target; - private readonly IDispatcher? _dispatcher; + private void Init() + { + // Note: As the OnServiceStateChanged might cause a SetCurrent, which will try to resolve the _target, + // we must keep this Init lazy. - private bool _isInit; + if (_isInit) + { + return; + } + _isInit = true; - public SelectionFacet(IBindableCollectionViewSource source, Func> target) + if (_service is not null) { - _service = source.GetService(typeof(ISelectionService)) as ISelectionService; - _target = new Lazy>(target, LazyThreadSafetyMode.None); - _dispatcher = source.Dispatcher; + _service.StateChanged += OnServiceStateChanged; + OnServiceStateChanged(_service, EventArgs.Empty); } + _collectionChangedFacet.AddCollectionChangedHandler(RestoreCurrentItemOnSourceChanged, lowPriority: true); + } - private void Init() + private void OnServiceStateChanged(object? snd, EventArgs args) + { + if (_service!.IsSelected(CurrentPosition)) { - // Note: As the OnServiceStateChanged might cause a SetCurrent, which will try to resolve the _target, - // we must keep this Init lazy. + return; + } - if (_isInit) - { - return; - } - _isInit = true; + var ranges = _service.GetSelectedRanges(); - if (_service is not null) - { - _service.StateChanged += OnServiceStateChanged; - OnServiceStateChanged(_service, EventArgs.Empty); - } - } +#if __WINDOWS__ + UpdateLocalSelection(ranges); +#endif - private void OnServiceStateChanged(object? snd, EventArgs args) + if (!_service.IsSelected(CurrentPosition)) { - if (_service!.IsSelected(CurrentPosition)) - { - return; - } - - var index = _service.GetSelectedRanges().FirstOrDefault()?.FirstIndex ?? -1; + var index = ranges.FirstOrDefault()?.FirstIndex ?? -1; if (_dispatcher is null or { HasThreadAccess: true }) { MoveCurrentToPosition(index); @@ -77,161 +90,279 @@ private void OnServiceStateChanged(object? snd, EventArgs args) _dispatcher.TryEnqueue(() => MoveCurrentToPosition(index)); } } + } - #region ICollectionView.Current - public EventRegistrationToken AddCurrentChangedHandler(CurrentChangedEventHandler value) + private void RestoreCurrentItemOnSourceChanged(object? sender, NotifyCollectionChangedEventArgs args) + { + // Note about the usage of the CurrentPosition here: + // The ListView won't sync the Current is the source implements ISelectionRange + // BUT it will still listen to CurrentChanged event. + // We keep it in sync on our side (cf. SelectRange and DeselectRange), so if the SelectedItem is updated by the VM (Replace with IsReplaceOfSameEntities flag), + // we are be able to properly restore it here. + // Note: This will work only for the first selected item, i.e. only for SelectionMode.Single which is the main case we are interested in (master-details). + + // Other actions (like Remove and Reset) will be pushed by the ListView itself. + + if (args is RichNotifyCollectionChangedEventArgs { Action: NotifyCollectionChangedAction.Replace, IsReplaceOfSameEntities: true } + && CurrentPosition >= args.OldStartingIndex + && CurrentPosition < args.OldStartingIndex + args.OldItems!.Count) { - Init(); - return _currentChanged.AddEventHandler(value); + MoveCurrentToPosition(CurrentPosition, isCancelable: false, isSelectionRangeUpdate: true); } + } + + #region ICollectionView.Current (Single selection mode) + public EventRegistrationToken AddCurrentChangedHandler(CurrentChangedEventHandler value) + { + Init(); + return _currentChanged.AddEventHandler(value); + } #if USE_EVENT_TOKEN - public void RemoveCurrentChangedHandler(EventRegistrationToken value) - { - Init(); - _currentChanged.RemoveEventHandler(value); - } + public void RemoveCurrentChangedHandler(EventRegistrationToken value) + { + Init(); + _currentChanged.RemoveEventHandler(value); + } #endif - public void RemoveCurrentChangedHandler(CurrentChangedEventHandler value) + public void RemoveCurrentChangedHandler(CurrentChangedEventHandler value) + { + Init(); + _currentChanged.RemoveEventHandler(value); + } + + public EventRegistrationToken AddCurrentChangingHandler(CurrentChangingEventHandler value) + { + Init(); + return _currentChanging.AddEventHandler(value); + } + +#if USE_EVENT_TOKEN + public void RemoveCurrentChangingHandler(EventRegistrationToken value) + { + Init(); + _currentChanging.RemoveEventHandler(value); + } +#endif + + public void RemoveCurrentChangingHandler(CurrentChangingEventHandler value) + { + Init(); + _currentChanging.RemoveEventHandler(value); + } + + public object? CurrentItem { get; private set; } + + public int CurrentPosition { get; private set; } = -1; // -1 means nothing is selected + + public bool IsCurrentAfterLast => false; + + public bool IsCurrentBeforeFirst + { + get { Init(); - _currentChanged.RemoveEventHandler(value); + return CurrentPosition < 0; } + } - public EventRegistrationToken AddCurrentChangingHandler(CurrentChangingEventHandler value) + public bool MoveCurrentTo(object item) + { + Init(); + + if (item == null) { - Init(); - return _currentChanging.AddEventHandler(value); + return SetCurrent(-1, null); } - -#if USE_EVENT_TOKEN - public void RemoveCurrentChangingHandler(EventRegistrationToken value) + else { - Init(); - _currentChanging.RemoveEventHandler(value); + var index = _target.Value.IndexOf(item); + + return index >= 0 && SetCurrent(index, item); } -#endif + } + + public bool MoveCurrentToPosition(int index) + => MoveCurrentToPosition(index, isCancelable: true, isSelectionRangeUpdate: false); - public void RemoveCurrentChangingHandler(CurrentChangingEventHandler value) + private bool MoveCurrentToPosition(int index, bool isCancelable, bool isSelectionRangeUpdate) + { + Init(); + + if (index < 0) { - Init(); - _currentChanging.RemoveEventHandler(value); + return SetCurrent(-1, null, isCancelable, isSelectionRangeUpdate); + } + else + { + return index < _target.Value.Count && SetCurrent(index, _target.Value[index], isCancelable, isSelectionRangeUpdate); } + } - public object? CurrentItem { get; private set; } + public bool MoveCurrentToFirst() => MoveCurrentToPosition(0); // No needs to Init: are not using any Current*** - public int CurrentPosition { get; private set; } = -1; // -1 means nothing is selected + public bool MoveCurrentToLast() => MoveCurrentToPosition(_target.Value.Count - 1); // No needs to Init: are not using any Current*** - public bool IsCurrentAfterLast => false; + public bool MoveCurrentToNext() + { + Init(); + return CurrentPosition + 1 < _target.Value.Count && MoveCurrentToPosition(CurrentPosition + 1); + } - public bool IsCurrentBeforeFirst + public bool MoveCurrentToPrevious() + { + Init(); + return CurrentPosition > 0 && MoveCurrentToPosition(CurrentPosition - 1); + } + + private bool SetCurrent(int index, object? value, bool isCancelable = true, bool isSelectionRangeUpdate = false) + { + if (CurrentPosition == index && EqualityComparer.Default.Equals(CurrentItem, value)) { - get - { - Init(); - return CurrentPosition < 0; - } + // Current is already up to date, do not raise events for nothing! + return true; } - public bool MoveCurrentTo(object item) + var changing = _currentChanging.InvocationList; + if (changing != null) { - Init(); - - if (item == null) + var args = new CurrentChangingEventArgs(isCancelable); + changing.Invoke(this, args); + if (isCancelable && args.Cancel) { - return SetCurrent(-1, null); - } - else - { - var index = _target.Value.IndexOf(item); - - return index >= 0 && SetCurrent(index, item); + return false; } } - public bool MoveCurrentToPosition(int index) - { - Init(); + var oldPosition = CurrentPosition; + + CurrentPosition = index; + CurrentItem = value; - if (index < 0) + if (!isSelectionRangeUpdate) + { + if (index >= 0) { - return SetCurrent(-1, null); + SelectRange(new ItemIndexRange(index, 1)); } - else + else if (oldPosition >= 0) { - return index < _target.Value.Count && SetCurrent(index, _target.Value[index]); + // Note: we unselect only if the new position is -1 (and old was not -1), otherwise we would prevent multi-selection! + DeselectRange(new ItemIndexRange(oldPosition, 1)); } } - public bool MoveCurrentToFirst() => MoveCurrentToPosition(0); // No needs to Init: are not using any Current*** + _currentChanged.InvocationList?.Invoke(this, CurrentItem); - public bool MoveCurrentToLast() => MoveCurrentToPosition(_target.Value.Count - 1); // No needs to Init: are not using any Current*** + return true; + } + #endregion - public bool MoveCurrentToNext() + #region ISelectionInfo (Multi selection modes) + /// + public void SelectRange(ItemIndexRange itemIndexRange) + { + if (_service is null) { - Init(); - return CurrentPosition + 1 < _target.Value.Count && MoveCurrentToPosition(CurrentPosition + 1); + return; } - public bool MoveCurrentToPrevious() +#if __WINDOWS__ + Debug.Assert(itemIndexRange.Length is 1); // Required for local coercing + _localSelection.Add(itemIndexRange); +#endif + _service.SelectRange(itemIndexRange); + + if (_service.GetSelectedRanges() is { Count: 1 } selection) { - Init(); - return CurrentPosition > 0 && MoveCurrentToPosition(CurrentPosition - 1); + MoveCurrentToPosition(selection.First().FirstIndex, isCancelable: false, isSelectionRangeUpdate: true); } + } - private bool SetCurrent(int index, object? value, bool isCancelable = true) + /// + public void DeselectRange(ItemIndexRange itemIndexRange) + { + if (_service is null) { - if (CurrentPosition == index && EqualityComparer.Default.Equals(CurrentItem, value)) - { - // Current is already up to date, do not raise events for nothing! - return true; - } + return; + } - var changing = _currentChanging.InvocationList; - if (changing != null) - { - var args = new CurrentChangingEventArgs(isCancelable); - changing.Invoke(this, args); - if (isCancelable && args.Cancel) - { - return false; - } - } +#if __WINDOWS__ + _localSelection.Remove(itemIndexRange); +#endif + _service.DeselectRange(itemIndexRange); - CurrentPosition = index; - CurrentItem = value; + if (CurrentPosition >= itemIndexRange.FirstIndex + && CurrentPosition <= itemIndexRange.LastIndex + && _service.GetSelectedRanges() is { Count: >= 1 } selection) + { + MoveCurrentToPosition(selection.First().FirstIndex, isCancelable: false, isSelectionRangeUpdate: true); + } + } - SelectRange(new ItemIndexRange(index, 1)); + /// + public bool IsSelected(int index) + => _service?.IsSelected(index) ?? false; - _currentChanged.InvocationList?.Invoke(this, CurrentItem); +#if __WINDOWS__ + // On Windows the ListView does not accept to coerce the ranges. + // Instead we try to keep the range instances provided by the ListView. + private readonly List _localSelection = new(); - return true; - } - #endregion + /// + public IReadOnlyList GetSelectedRanges() + => _localSelection; - /// - public void SelectRange(ItemIndexRange itemIndexRange) - => _service?.SelectRange(itemIndexRange); + private void UpdateLocalSelection(IReadOnlyList svcRanges) + { + // Here we make sure that 'ranges' we are receiving from the _service are the same as _localSelection + // Note: We assume that the ListView on window creates only range of 1 item. - /// - public void DeselectRange(ItemIndexRange itemIndexRange) - => _service?.DeselectRange(itemIndexRange); + if (_localSelection is { Count: 0 }) + { + _localSelection.AddRange(svcRanges); + return; + } - /// - public bool IsSelected(int index) - => _service?.IsSelected(index) ?? false; + var localIndexes = _localSelection + .SelectMany(r => Enumerable.Range(r.FirstIndex, (int)r.Length)) + .ToList(); - /// - public IReadOnlyList GetSelectedRanges() - => _service?.GetSelectedRanges() ?? Array.Empty(); + var serviceIndexes = svcRanges + .SelectMany(r => Enumerable.Range(r.FirstIndex, (int)r.Length)) + .ToList(); - public void Dispose() + foreach (var index in serviceIndexes) { - if (_service is not null) + if (!localIndexes.Contains(index)) { - _service.StateChanged -= OnServiceStateChanged; + _localSelection.Add(new ItemIndexRange(index, 1)); + } + } + + foreach (var index in localIndexes) + { + if (!serviceIndexes.Contains(index)) + { + _localSelection.RemoveAll(r => r.FirstIndex == index); } } } +#else + /// + public IReadOnlyList GetSelectedRanges() + => _service?.GetSelectedRanges() ?? Array.Empty(); +#endif + + #endregion + + public void Dispose() + { + if (_service is not null) + { + _service.StateChanged -= OnServiceStateChanged; + } + _collectionChangedFacet.RemoveCollectionChangedHandler(RestoreCurrentItemOnSourceChanged, lowPriority: true); + } } diff --git a/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Services/SelectionService.cs b/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Services/SelectionService.cs index 9647987e99..ad6c76bfae 100644 --- a/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Services/SelectionService.cs +++ b/src/Uno.Extensions.Reactive.UI/Presentation/Bindings/Collections/Services/SelectionService.cs @@ -27,7 +27,6 @@ public SelectionService(AsyncAction setSelectionFromView) _setSelectionFromView = setSelectionFromView; } - #region Change selection from source public void SetFromSource(SelectionInfo selection) { diff --git a/src/Uno.Extensions.Reactive/Utils/SelectionHelper.cs b/src/Uno.Extensions.Reactive/Utils/SelectionHelper.cs index e9ac172a6c..487d51c8cc 100644 --- a/src/Uno.Extensions.Reactive/Utils/SelectionHelper.cs +++ b/src/Uno.Extensions.Reactive/Utils/SelectionHelper.cs @@ -13,7 +13,7 @@ public static IReadOnlyList Coerce(IReadOnlyList range.Length).GetEnumerator(); + using var enumerator = ranges.OrderBy(range => range.FirstIndex).GetEnumerator(); if (!enumerator.MoveNext()) { return Array.Empty();