From 176188ea0a210591c1fc9edcea3da537ac92d729 Mon Sep 17 00:00:00 2001 From: xiaoy312 Date: Fri, 7 Mar 2025 14:07:04 -0500 Subject: [PATCH 1/2] fix(listview): setting SelectedItem during list refresh leaves blank in items on wasm/skia --- .../Given_ListViewBase.cs | 28 ++++++++++ .../Extensions/ViewExtensions.visual-tree.cs | 2 +- .../VirtualizingPanelLayout.managed.cs | 51 ++++++++++++------- 3 files changed, 61 insertions(+), 20 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs index 3b3e07c3716b..1e71d5d66b22 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs @@ -4868,6 +4868,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(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().Count(), "There should be only 4 materialized container."); +#endif + } } public partial class Given_ListViewBase // data class, data-context, view-model, template-selector diff --git a/src/Uno.UI/Extensions/ViewExtensions.visual-tree.cs b/src/Uno.UI/Extensions/ViewExtensions.visual-tree.cs index 0b8564bc627b..e0ff8269e107 100644 --- a/src/Uno.UI/Extensions/ViewExtensions.visual-tree.cs +++ b/src/Uno.UI/Extensions/ViewExtensions.visual-tree.cs @@ -152,11 +152,11 @@ static IEnumerable 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 diff --git a/src/Uno.UI/UI/Xaml/Controls/ListViewBase/VirtualizingPanelLayout.managed.cs b/src/Uno.UI/UI/Xaml/Controls/ListViewBase/VirtualizingPanelLayout.managed.cs index fc71b42f4098..b4167a613f75 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ListViewBase/VirtualizingPanelLayout.managed.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ListViewBase/VirtualizingPanelLayout.managed.cs @@ -165,10 +165,12 @@ private double ViewportExtent /// private double ViewportEnd => ScrollOffset + ViewportExtent; + internal const double ExtendedViewportScaling = 0.5; + /// /// The additional length in pixels for which to create buffered views. /// - private double ViewportExtension => CacheLength * ViewportExtent * 0.5; + private double ViewportExtension => CacheLength * ViewportExtent * ExtendedViewportScaling; /// /// The start of the 'extended viewport,' the area of the visible viewport plus the buffer area defined by . @@ -459,7 +461,10 @@ private void ArrangeElements(Size finalSize, Size adjustedVisibleWindow) /// /// Update the item container layout by removing no-longer-visible views and adding visible views. /// - /// Adjustment to apply when calculating fillable area. + /// + /// Adjustment to apply when calculating fillable area. + /// Used when a viewport change is not yet committed into the . + /// private void UpdateLayout(double? extentAdjustment, bool isScroll) { ResetReorderingIndex(); @@ -501,7 +506,10 @@ private void UpdateCompleted() /// /// Fill in extended viewport with views. /// - /// Adjustment to apply when calculating fillable area. + /// + /// Adjustment to apply when calculating fillable area. + /// Used when a viewport change is not yet committed into the . + /// 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,27 +535,22 @@ 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; } } } @@ -548,7 +558,10 @@ void FillForward() /// /// Remove views that lie entirely outside extended viewport. /// - /// Adjustment to apply when calculating fillable area. + /// + /// Adjustment to apply when calculating fillable area. + /// Used when a viewport change is not yet committed into the . + /// private void UnfillLayout(double extentAdjustment) { UnfillBackward(); From 20c0e900a8ff1d4ec892c075f7de3e8c5e67af0c Mon Sep 17 00:00:00 2001 From: xiaoy312 Date: Mon, 17 Mar 2025 21:47:47 -0400 Subject: [PATCH 2/2] test: fix broken test --- .../Given_ListViewBase.cs | 59 ++++++++++++------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs index 1e71d5d66b22..2d4a3027a94b 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs @@ -2243,35 +2243,52 @@ public async Task When_LargeExtent_And_Very_Large_List_Scroll_To_End_And_Back_Ha var scroll = list.FindFirstDescendant(); 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