Merge pull request #19721 from unoplatform/mergify/bp/release/stable/5.6/pr-19654

fix(listview): setting SelectedItem during list refresh leaves blank in items on wasm/skia (backport #19654)

Author: jeromelaban

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs

@@ -2240,35 +2240,52 @@ public async Task When_LargeExtent_And_Very_Large_List_Scroll_To_End_And_Back_Ha
 
 			var scroll = list.FindFirstDescendant<ScrollViewer>();
 			Assert.IsNotNull(scroll);
-			dataContextChanged.Should().BeLessThan(10, $"dataContextChanged {dataContextChanged}");
-
-			ScrollTo(list, ElementHeight);
-
-			await WindowHelper.WaitForIdle();
-
-			materialized.Should().BeLessThan(12, $"materialized {materialized}");
-			dataContextChanged.Should().BeLessThan(11, $"dataContextChanged {dataContextChanged}");
 
-			ScrollTo(list, ElementHeight * 3);
-
-			await WindowHelper.WaitForIdle();
+			int expectedMaterialized = 0, expectedDCChanged = 0;
+			int[] previouslyMaterializedItems = [];
+			async Task ScrollAndValidate(string context, double? scrollTo)
+			{
+				if (scrollTo is { } voffset)
+				{
+					ScrollTo(list, voffset);
+				}
+				await WindowHelper.WaitForIdle();
 
-			materialized.Should().BeLessThan(14, $"materialized {materialized}");
-			dataContextChanged.Should().BeLessThan(13, $"dataContextChanged {dataContextChanged}");
+#if HAS_UNO && !(__IOS__ || __ANDROID__)
+				var evpScaling = (list.ItemsPanelRoot as IVirtualizingPanel).GetLayouter().CacheLength * VirtualizingPanelLayout.ExtendedViewportScaling;
+#else
+				var evpScaling = 0.5;
+#endif
 
-			ScrollTo(list, scroll.ExtentHeight / 2); // Scroll to middle
+				var offset = scroll.VerticalOffset;
+				var max = scroll.ExtentHeight;
+				var vp = scroll.ViewportHeight;
 
-			await WindowHelper.WaitForIdle();
+				var evpStart = Math.Clamp(offset - vp * evpScaling, 0, max);
+				var evpEnd = Math.Clamp(offset + vp + vp * evpScaling, 0, max);
 
-			materialized.Should().BeLessThan(14, $"materialized {materialized}");
-			dataContextChanged.Should().BeLessThan(25, $"dataContextChanged {dataContextChanged}");
+				var firstIndex = (int)Math.Round(evpStart / ElementHeight, 0, MidpointRounding.ToNegativeInfinity);
+				var lastIndex = (int)Math.Round(evpEnd / ElementHeight, 0, MidpointRounding.ToPositiveInfinity) - 1;
+				var itemsInEVP = Enumerable.Range(firstIndex, lastIndex - firstIndex + 1).ToArray();
+				var newItemsInEVP = itemsInEVP.Except(previouslyMaterializedItems).ToArray();
 
-			ScrollTo(list, scroll.ExtentHeight / 4); // Scroll to Quarter
+				// materialized starts with +1 extra, since we use it to determine whether the DataTemplate itself is a self-container
+				// Math.Max to count the historical highest, since "materialization" doesnt "unhappen" (we dont count tear-down).
+				expectedMaterialized = Math.Max(expectedMaterialized, 1 + itemsInEVP.Length);
+				// dc-changed counts the total items prepared and "re-entrancy"(out of effective-viewport and back in).
+				// we just need to add the new items since last time
+				expectedDCChanged += newItemsInEVP.Length;
+				previouslyMaterializedItems = itemsInEVP;
 
-			await WindowHelper.WaitForIdle();
+				materialized.Should().BeLessOrEqualTo(expectedMaterialized, $"[{context}] materialized {materialized}");
+				dataContextChanged.Should().BeLessOrEqualTo(expectedDCChanged, $"[{context}] dataContextChanged {dataContextChanged}");
+			}
 
-			materialized.Should().BeLessThan(14, $"materialized {materialized}");
-			dataContextChanged.Should().BeLessThan(35, $"dataContextChanged {dataContextChanged}");
+			await ScrollAndValidate("initial state", null);
+			await ScrollAndValidate("scrolled past element#0", ElementHeight);
+			await ScrollAndValidate("scrolled past element#2", ElementHeight * 3);
+			await ScrollAndValidate("scrolled to 1/2", scroll.ExtentHeight / 2);
+			await ScrollAndValidate("scrolled back to 1/4", scroll.ExtentHeight / 4);
 		}
 #endif
 
@@ -4865,6 +4882,34 @@ void AddItem(string item, bool select = false)
 
 			Assert.AreEqual(sv.ScrollableHeight, sv.VerticalOffset, "ListView is not scrolled to the end.");
 		}
+
+		[TestMethod]
+		public async Task When_SelectionChanged_DuringRefresh()
+		{
+			var source = new ObservableCollection<string>(Enumerable.Range(0, 4).Select(x => $"Item {x}"));
+			var sut = new ListView
+			{
+				Height = source.Count * 29 * 1.5, // give ample room
+				ItemsSource = source,
+				ItemTemplate = FixedSizeItemTemplate, // height=29
+			};
+
+			await UITestHelper.Load(sut, x => x.IsLoaded);
+
+			Assert.IsTrue(Enumerable.Range(0, 4).All(x => sut.ContainerFromIndex(x) is { }), "All containers should be materialized.");
+
+			source.Move(1, 2); // swap: 0[1]23 -> 02[1]3
+			sut.SelectedItem = source[2]; // select "Item 1" (at index 2)
+
+			await UITestHelper.WaitForIdle();
+
+			var tree = sut.TreeGraph();
+			Assert.IsTrue(Enumerable.Range(0, 4).All(x => sut.ContainerFromIndex(x) is { }), "All containers should be materialized.");
+
+#if !(__ANDROID__ || __IOS__ || __MACOS__)
+			Assert.AreEqual(4, sut.ItemsPanelRoot.Children.OfType<ListViewItem>().Count(), "There should be only 4 materialized container.");
+#endif
+		}
 	}
 
 	public partial class Given_ListViewBase // data class, data-context, view-model, template-selector

src/Uno.UI/Extensions/ViewExtensions.visual-tree.cs

@@ -152,11 +152,11 @@ static IEnumerable<string> GetDetails(object x)
 #if true // framework layout properties
 			if (x is FrameworkElement fe)
 			{
-				yield return $"Actual={FormatSize(fe.ActualWidth, fe.ActualHeight)}";
 				if (fe.Parent is FrameworkElement parent)
 				{
 					yield return $"XY={FormatPoint(fe.TransformToVisual(parent).TransformPoint(default))}";
 				}
+				yield return $"Actual={FormatSize(fe.ActualWidth, fe.ActualHeight)}";
 #if HAS_UNO
 				//yield return $"Available={FormatSize(fe.m_previousAvailableSize)}";
 #endif

src/Uno.UI/UI/Xaml/Controls/ListViewBase/VirtualizingPanelLayout.managed.cs

@@ -165,10 +165,12 @@ private double ViewportExtent
 		/// </summary>
 		private double ViewportEnd => ScrollOffset + ViewportExtent;
 
+		internal const double ExtendedViewportScaling = 0.5;
+
 		/// <summary>
 		/// The additional length in pixels for which to create buffered views.
 		/// </summary>
-		private double ViewportExtension => CacheLength * ViewportExtent * 0.5;
+		private double ViewportExtension => CacheLength * ViewportExtent * ExtendedViewportScaling;
 
 		/// <summary>
 		/// The start of the 'extended viewport,' the area of the visible viewport plus the buffer area defined by <see cref="CacheLength"/>.
@@ -459,7 +461,10 @@ private void ArrangeElements(Size finalSize, Size adjustedVisibleWindow)
 		/// <summary>
 		/// Update the item container layout by removing no-longer-visible views and adding visible views.
 		/// </summary>
-		/// <param name="extentAdjustment">Adjustment to apply when calculating fillable area.</param>
+		/// <param name="extentAdjustment">
+		/// Adjustment to apply when calculating fillable area.
+		/// Used when a viewport change is not yet committed into the <see cref="ScrollOffset"/>.
+		/// </param>
 		private void UpdateLayout(double? extentAdjustment, bool isScroll)
 		{
 			ResetReorderingIndex();
@@ -501,7 +506,10 @@ private void UpdateCompleted()
 		/// <summary>
 		/// Fill in extended viewport with views.
 		/// </summary>
-		/// <param name="extentAdjustment">Adjustment to apply when calculating fillable area.</param>
+		/// <param name="extentAdjustment">
+		/// Adjustment to apply when calculating fillable area.
+		/// Used when a viewport change is not yet committed into the <see cref="ScrollOffset"/>.
+		/// </param>
 		private void FillLayout(double extentAdjustment)
 		{
 			// Don't fill backward if we're doing a light rebuild (since we are starting from nearest previously-visible item)
@@ -512,6 +520,13 @@ private void FillLayout(double extentAdjustment)
 
 			FillForward();
 
+			if (_dynamicSeedStart.HasValue && GetItemsEnd() <= ExtendedViewportEnd + extentAdjustment)
+			{
+				// if the FillForward didnt fully fill the current viewport,
+				// we may still need to a FillBackward, otherwise we risk having a leading blank space
+				FillBackward();
+			}
+
 			// Make sure that the reorder item has been rendered
 			if (GetAndUpdateReorderingIndex() is { } reorderIndex && _materializedLines.None(line => line.Contains(reorderIndex)))
 			{
@@ -520,35 +535,33 @@ private void FillLayout(double extentAdjustment)
 
 			void FillBackward()
 			{
-				if (GetItemsStart() > ExtendedViewportStart + extentAdjustment)
+				while (
+					GetItemsStart() is { } start && start > ExtendedViewportStart + extentAdjustment &&
+					GetNextUnmaterializedItem(Backward, GetFirstMaterializedIndexPath()) is { } nextItem)
 				{
-					var nextItem = GetNextUnmaterializedItem(Backward, GetFirstMaterializedIndexPath());
-					while (nextItem != null && GetItemsStart() > ExtendedViewportStart + extentAdjustment)
-					{
-						AddLine(Backward, nextItem.Value);
-						nextItem = GetNextUnmaterializedItem(Backward, GetFirstMaterializedIndexPath());
-					}
+					AddLine(Backward, nextItem);
 				}
 			}
-
 			void FillForward()
 			{
-				if ((GetItemsEnd() ?? 0) < ExtendedViewportEnd + extentAdjustment)
+				var seed = _dynamicSeedIndex;
+				while (
+					GetItemsEnd() is var end && (end ?? 0) < ExtendedViewportEnd + extentAdjustment &&
+					GetNextUnmaterializedItem(Forward, seed ?? GetLastMaterializedIndexPath()) is { } nextItem)
 				{
-					var nextItem = GetNextUnmaterializedItem(Forward, _dynamicSeedIndex ?? GetLastMaterializedIndexPath());
-					while (nextItem != null && (GetItemsEnd() ?? 0) < ExtendedViewportEnd + extentAdjustment)
-					{
-						AddLine(Forward, nextItem.Value);
-						nextItem = GetNextUnmaterializedItem(Forward, GetLastMaterializedIndexPath());
-					}
+					AddLine(Forward, nextItem);
+					seed = null;
 				}
 			}
 		}
 
 		/// <summary>
 		/// Remove views that lie entirely outside extended viewport.
 		/// </summary>
-		/// <param name="extentAdjustment">Adjustment to apply when calculating fillable area.</param>
+		/// <param name="extentAdjustment">
+		/// Adjustment to apply when calculating fillable area.
+		/// Used when a viewport change is not yet committed into the <see cref="ScrollOffset"/>.
+		/// </param>
 		private void UnfillLayout(double extentAdjustment)
 		{
 			UnfillBackward();