From 5ee7b000b7c5242c31ab9a0a9684b5ab854e402c Mon Sep 17 00:00:00 2001
From: Brayan Khosravian <35541212+BrayanKhosravian@users.noreply.github.com>
Date: Thu, 28 Nov 2019 09:12:23 +0100
Subject: [PATCH]
Fix-7167-UWP-ListViewRenderer-ScrollToTop-when-NotifyCollectionChangedAction.Reset
(#7279)
* - created test view in control gallery for issue 7167
- wrote view, vm, and "ImprovedObservableCollection" for testing purpose
- reproduced the bug without using any external nugets like "System.Reactive"
- added onitemselected of listview to test itemselections
- fixed by changing the implementation of Xamarin.Forms.Platform.UWP.ListViewRenderer.ReloadData
- single commit which is useful for cherry picks
- if itemsource and collectionviewsource.source (_collection) are the same the collectionviewsource is not being reinstantioated again
- wrote ui test.
* - changed controlgalery.projitems manually after cherrypick
* - made testcase uitestable
* - fixed uitesting exception
* - made view, vm ui-testable
- (TODO) modified uitest. (assert is missing)
* - created test view in control gallery for issue 7167
- wrote view, vm, and "ImprovedObservableCollection" for testing purpose
- reproduced the bug without using any external nugets like "System.Reactive"
- added onitemselected of listview to test itemselections
- fixed by changing the implementation of Xamarin.Forms.Platform.UWP.ListViewRenderer.ReloadData
- single commit which is useful for cherry picks
- if itemsource and collectionviewsource.source (_collection) are the same the collectionviewsource is not being reinstantioated again
- wrote ui test.
* - changed controlgalery.projitems manually after cherrypick
* - made testcase uitestable
* - fixed uitesting exception
* - made view, vm ui-testable
- (TODO) modified uitest. (assert is missing)
* - wrote uitest, formetted
* - updated comments
* - uitest are only being executed for UWP
* - removed comments (PR feedback)
---
.../Issue7167.xaml | 37 ++++++
.../Issue7167.xaml.cs | 131 +++++++++++++++++++++
.../TestPages/TestPages.cs | 2 +-
.../Xamarin.Forms.Controls.Issues.Shared.projitems | 10 ++
Xamarin.Forms.Platform.UAP/ListViewRenderer.cs | 12 +-
5 files changed, 187 insertions(+), 5 deletions(-)
create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7167.xaml
create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7167.xaml.cs
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7167.xaml b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7167.xaml
new file mode 100644
index 0000000..dbe41fa
--- /dev/null
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7167.xaml
@@ -0,0 +1,37 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
\ No newline at end of file
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7167.xaml.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7167.xaml.cs
new file mode 100644
index 0000000..188aede
--- /dev/null
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue7167.xaml.cs
@@ -0,0 +1,131 @@
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Collections.Specialized;
+using System.Linq;
+using System.Windows.Input;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+using Xamarin.Forms.Xaml;
+#if UITEST && __WINDOWS__
+using Xamarin.UITest;
+using Xamarin.Forms.Core.UITests;
+using NUnit.Framework;
+#endif
+
+
+namespace Xamarin.Forms.Controls.Issues
+{
+#if APP
+ [XamlCompilation(XamlCompilationOptions.Compile)]
+#endif
+
+ [Preserve(AllMembers = true)]
+ [Issue(IssueTracker.Github, 7167,
+ "[Bug] improved observablecollection. a lot of collectionchanges. a reset is sent and listview scrolls to the top", PlatformAffected.UWP)]
+ public partial class Issue7167 : TestContentPage
+ {
+
+ protected override void Init()
+ {
+#if APP
+ InitializeComponent();
+#endif
+ BindingContext = new Issue7167ViewModel();
+ }
+
+ void MyListView_OnItemSelected(object sender, SelectedItemChangedEventArgs e)
+ {
+ var item = e.SelectedItem;
+ var index = e.SelectedItemIndex;
+
+ }
+
+#if UITEST && __WINDOWS__
+ const string ListViewId = "ListViewId";
+ const string AddCommandID = "AddCommandID";
+ const string ClearListCommandId = "ClearListCommandId";
+ const string AddRangeCommandId = "AddRangeCommandId";
+ const string AddRangeWithCleanCommandId = "AddRangeWithCleanCommandId";
+
+ [Test]
+ public void Issue7167Test()
+ {
+ // arrange
+ // add items to the list and scroll down till item "25"
+ RunningApp.Screenshot("Empty ListView");
+ RunningApp.Tap(q => q.Button(AddRangeCommandId));
+ RunningApp.Tap(q => q.Button(AddRangeCommandId));
+ RunningApp.WaitForElement(c => c.Index(25).Property("Enabled", true));
+ RunningApp.Print.Tree();
+ RunningApp.ScrollDownTo(a => a.Marked("25").Property("text").Contains("25"),
+ b => b.Marked(ListViewId), ScrollStrategy.Auto);
+ RunningApp.WaitForElement(x => x.Marked("25"));
+
+ // act
+ // when adding additional items via a addrange and a CollectionChangedEventArgs.Action.Reset is sent
+ // then the listview shouldnt reset or it should not scroll to the top
+ RunningApp.Tap(q => q.Marked(AddRangeCommandId));
+
+ // assert
+ // assert that item "25" is still visible
+ var result = RunningApp.Query(x => x.Marked(ListViewId).Child().Marked("25"));
+ Assert.That(result?.Length <= 0);
+ }
+#endif
+
+
+ }
+
+ [Preserve (AllMembers = true)]
+ internal class Issue7167ViewModel
+ {
+ IEnumerable CreateItems()
+ {
+ var itemCount = Items.Count();
+ return Enumerable.Range(itemCount, 50).Select(num => num.ToString());
+ }
+
+ public ImprovedObservableCollection Items { get; set; } = new ImprovedObservableCollection();
+
+ public ICommand AddCommand => new Command(_ => Items.Add(CreateItems().First()));
+ public ICommand ClearListCommand => new Command(_ => Items.Clear());
+ public ICommand AddRangeCommand => new Command(_ => Items.AddRange(CreateItems()));
+ public ICommand AddRangeWithCleanCommand => new Command(_ =>
+ {
+ Items.Clear();
+ Items.AddRange(CreateItems());
+ });
+
+ }
+
+ [Preserve (AllMembers = true)]
+ internal class ImprovedObservableCollection : ObservableCollection
+ {
+ bool _isActivated = true;
+ public void AddRange(IEnumerable source)
+ {
+ _isActivated = false;
+
+ foreach (var item in source)
+ {
+ base.Items.Add(item);
+ }
+
+ _isActivated = true;
+
+ this.OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
+ }
+
+ protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
+ {
+ if (_isActivated)
+ base.OnCollectionChanged(e);
+ }
+
+ }
+
+
+
+}
+
+
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/TestPages/TestPages.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/TestPages/TestPages.cs
index 46d5e47..7c97476 100644
--- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/TestPages/TestPages.cs
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/TestPages/TestPages.cs
@@ -786,7 +786,7 @@ namespace Xamarin.Forms.Controls.Issues
using System;
using NUnit.Framework;
- // Run setup once for all tests in the Xamarin.Forms.Controls.Issues namespace
+// Run setup once for all tests in the Xamarin.Forms.Controls.Issues namespace
// (instead of once for each test)
[SetUpFixture]
public class IssuesSetup
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
index ceb26ef..829646d 100644
--- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
@@ -596,6 +596,10 @@
Issue3979.xaml
Code
+
+ Issue7167.xaml
+ Code
+
Issue4194.xaml
Code
@@ -1387,6 +1391,12 @@
+
+ Designer
+ MSBuild:UpdateDesignTimeXaml
+
+
+
Designer
MSBuild:UpdateDesignTimeXaml
diff --git a/Xamarin.Forms.Platform.UAP/ListViewRenderer.cs b/Xamarin.Forms.Platform.UAP/ListViewRenderer.cs
index f291726..157da78 100644
--- a/Xamarin.Forms.Platform.UAP/ListViewRenderer.cs
+++ b/Xamarin.Forms.Platform.UAP/ListViewRenderer.cs
@@ -117,13 +117,13 @@ namespace Xamarin.Forms.Platform.UWP
bool IsObservableCollection(object source)
{
- var type = source.GetType();
- return type.GetTypeInfo().IsGenericType &&
- type.GetGenericTypeDefinition() == typeof(ObservableCollection<>);
+ return source is INotifyCollectionChanged && source is IList;
}
void ReloadData()
{
+ var isStillTheSameUnderlyingItemsSource = _collection != null && object.ReferenceEquals(_collection, Element?.ItemsSource);
+
if (Element?.ItemsSource == null)
{
_collection = null;
@@ -131,18 +131,22 @@ namespace Xamarin.Forms.Platform.UWP
else
{
_collectionIsWrapped = !IsObservableCollection(Element.ItemsSource);
+
if (_collectionIsWrapped)
{
_collection = new ObservableCollection