Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Mar 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2243,35 +2243,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

Expand Down Expand Up @@ -4868,6 +4885,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
Expand Down
2 changes: 1 addition & 1 deletion src/Uno.UI/Extensions/ViewExtensions.visual-tree.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>.
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)
Expand All @@ -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)))
{
Expand All @@ -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();
Expand Down
Loading