Skip to content

Commit f97a070

Browse files
committed
fix(ComboBox): updated selection not scrolled into view
1 parent 6e9b1ff commit f97a070

File tree

4 files changed

+160
-17
lines changed

4 files changed

+160
-17
lines changed

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

+53
Original file line numberDiff line numberDiff line change
@@ -1410,6 +1410,59 @@ public async Task When_ComboBox_Popup_Dismissed()
14101410
Assert.IsTrue(dropDownClosedFired, "DropDownClosed event was not fired");
14111411
}
14121412

1413+
[TestMethod]
1414+
public Task When_ComboBox_ScrollIntoView_SelectedItem() => When_ComboBox_ScrollIntoView_Selection(viaIndex: false);
1415+
1416+
[TestMethod]
1417+
public Task When_ComboBox_ScrollIntoView_SelectedIndex() => When_ComboBox_ScrollIntoView_Selection(viaIndex: true);
1418+
1419+
private async Task When_ComboBox_ScrollIntoView_Selection(bool viaIndex)
1420+
{
1421+
var source = ( // 12x20=240 items, enough to overflow
1422+
from a in "qwerasdfzxcv"
1423+
from b in Enumerable.Range(0, 20)
1424+
select string.Concat(a, b)
1425+
).ToArray();
1426+
var sut = new ComboBox
1427+
{
1428+
ItemsSource = source,
1429+
};
1430+
await UITestHelper.Load(sut, x => x.IsLoaded);
1431+
1432+
var popup = sut.FindFirstDescendantOrThrow<Popup>("Popup");
1433+
1434+
// open the popup
1435+
sut.IsDropDownOpen = true;
1436+
await UITestHelper.WaitFor(() => popup.IsOpen, timeoutMS: 2000, "timed out waiting on the popup to open");
1437+
1438+
// wait until the dropdown panel is ready
1439+
await UITestHelper.WaitFor(() => sut.ItemsPanelRoot is { }, timeoutMS: 2000, "timed out waiting on the ItemsPanelRoot");
1440+
1441+
// set selection
1442+
if (viaIndex)
1443+
{
1444+
sut.SelectedIndex = source.Length - 3;
1445+
}
1446+
else
1447+
{
1448+
sut.SelectedItem = source[^3];
1449+
}
1450+
await UITestHelper.WaitForIdle();
1451+
1452+
// sanity check
1453+
Assert.AreEqual(source.Length - 3, sut.SelectedIndex, "SelectedIndex should be the 3rd last");
1454+
Assert.AreEqual(source[^3], sut.SelectedItem, "SelectedItem should be the 3rd last");
1455+
1456+
var sv = sut.ItemsPanelRoot.FindFirstAncestorOrThrow<ScrollViewer>();
1457+
var cbi = sut.ContainerFromIndex(sut.SelectedIndex) as FrameworkElement;
1458+
Assert.IsNotNull(cbi, "Selected container should not be null");
1459+
1460+
var cbiAbsRect = new Rect(cbi.ActualOffset.X, cbi.ActualOffset.Y, cbi.ActualWidth, cbi.ActualHeight);
1461+
var viewportAbsRect = new Rect(sv.HorizontalOffset, sv.VerticalOffset, sv.ViewportWidth, sv.ViewportHeight);
1462+
var intersection = viewportAbsRect.IntersectWith(cbiAbsRect);
1463+
Assert.IsTrue(cbiAbsRect == intersection, $"Selected container should be fully within viewport: CBI={PrettyPrint.FormatRect(cbiAbsRect)}, VP={PrettyPrint.FormatRect(viewportAbsRect)}");
1464+
}
1465+
14131466
public sealed class TwoWayBindingClearViewModel : IDisposable
14141467
{
14151468
public enum Themes

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

+13
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,19 @@ internal static bool TryGetDpValue<T>(object owner, string property, [NotNullWhe
233233
.OfType<T>()
234234
.FirstOrDefault();
235235

236+
/// <summary>
237+
/// Returns the first ancestor of a specified type.
238+
/// </summary>
239+
/// <typeparam name="T">The type of ancestor to find.</typeparam>
240+
/// <param name="reference">Any node of the visual tree</param>
241+
/// <remarks>First Counting from the <paramref name="reference"/> and not from the root of tree.</remarks>
242+
/// <exception cref="Exception">If the specified node could not be found.</exception>
243+
public static T? FindFirstAncestorOrThrow<T>(this _View? reference) => EnumerateAncestors(reference)
244+
.OfType<T>()
245+
.FirstOrDefault() ??
246+
throw new Exception($"Unable to find element: {typeof(T).Name}");
247+
248+
236249
/// <summary>
237250
/// Returns the first ancestor of a specified type that satisfies the <paramref name="predicate"/>.
238251
/// </summary>

src/Uno.UI/UI/Xaml/Controls/ComboBox/ComboBox.custom.cs

+64
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,70 @@ internal Brush LightDismissOverlayBackground
485485
set { SetValue(LightDismissOverlayBackgroundProperty, value); }
486486
}
487487

488+
internal void ScrollIntoView(object item, ScrollIntoViewAlignment alignment = ScrollIntoViewAlignment.Default)
489+
{
490+
if (ItemsPanelRoot is null)
491+
{
492+
// Not ready.
493+
return;
494+
}
495+
496+
if (ContainerFromItem(item) is UIElement element)
497+
{
498+
// The container we want to jump to is already materialized, so just jump to it.
499+
// This means we're in a non-virtualizing panel or in a virtualizing panel where the container we want is materialized for some reason (e.g. partially in view)
500+
ScrollIntoViewFastPath(element, alignment);
501+
}
502+
else if (VirtualizingPanel?.GetLayouter() is { } layouter)
503+
{
504+
layouter.ScrollIntoView(item, alignment);
505+
}
506+
}
507+
508+
private void ScrollIntoViewFastPath(UIElement element, ScrollIntoViewAlignment alignment)
509+
{
510+
if (ScrollViewer is { } sv && sv.Presenter is { } presenter)
511+
{
512+
var offsetXY = element.TransformToVisual(presenter).TransformPoint(
513+
#if __SKIA__ // Skia correctly doesn't include the offsets in TransformToVisual
514+
new Point(presenter.HorizontalOffset, presenter.VerticalOffset)
515+
#else
516+
Point.Zero
517+
#endif
518+
);
519+
520+
var orientation = ItemsPanelRoot?.PhysicalOrientation ?? Orientation.Vertical;
521+
522+
var (elementOffset, elementLength, presenterOffset, presenterViewportLength) =
523+
orientation is Orientation.Vertical
524+
? (offsetXY.Y, element.ActualSize.Y, presenter.VerticalOffset, presenter.ViewportHeight)
525+
: (offsetXY.X, element.ActualSize.X, presenter.HorizontalOffset, presenter.ViewportWidth);
526+
527+
if (presenterOffset <= elementOffset && elementOffset + elementLength <= presenterOffset + presenterViewportLength)
528+
{
529+
// if the element is within the visible viewport, do nothing.
530+
return;
531+
}
532+
533+
// If we use the above offset directly, the item we want to jump to will be the start of the viewport, i.e. leading.
534+
// For the default alignment, we move the element to either of the viewport ends (i.e. to the top or the bottom of the
535+
// viewport. To move to the bottom, we scroll one "viewport page" less. This brings the element's start right after the
536+
// viewport's length ends we then scroll again by elementLength so that the end of the element is the end of the viewport.
537+
var newOffset = alignment is ScrollIntoViewAlignment.Default && presenterOffset < elementOffset
538+
? elementOffset - presenterViewportLength + elementLength
539+
: elementOffset;
540+
541+
if (orientation is Orientation.Vertical)
542+
{
543+
sv.ScrollToVerticalOffset(newOffset);
544+
}
545+
else
546+
{
547+
sv.ScrollToHorizontalOffset(newOffset);
548+
}
549+
}
550+
}
551+
488552
internal static DependencyProperty LightDismissOverlayBackgroundProperty { get; } =
489553
DependencyProperty.Register("LightDismissOverlayBackground", typeof(Brush), typeof(ComboBox), new FrameworkPropertyMetadata(null));
490554

src/Uno.UI/UI/Xaml/Controls/Primitives/Selector.cs

+30-17
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,9 @@ internal virtual void OnSelectedItemChanged(object oldSelectedItem, object selec
170170
isSelectionUnset = true;
171171
}
172172

173-
var newIndex = -1;
174173
if (!_changingSelectedIndex)
175174
{
176-
newIndex = IndexFromItem(selectedItem);
175+
var newIndex = IndexFromItem(selectedItem);
177176
if (SelectedIndex != newIndex)
178177
{
179178
SelectedIndex = newIndex;
@@ -191,22 +190,9 @@ internal virtual void OnSelectedItemChanged(object oldSelectedItem, object selec
191190
}
192191

193192
#if !IS_UNIT_TESTS
194-
if (newIndex != -1 && IsInLiveTree)
193+
if (SelectedIndex != -1 && IsInLiveTree)
195194
{
196-
if (this is ListViewBase lvb
197-
#if __APPLE_UIKIT__
198-
// workaround to prevent scrolling when it is not ready
199-
// without this, the ios TabView could render blank if the selection happens too early.
200-
&& ContainerFromIndex(newIndex) is FrameworkElement { IsLoaded: true }
201-
#endif
202-
)
203-
{
204-
#if __APPLE_UIKIT__ || __ANDROID__
205-
lvb.InstantScrollToIndex(newIndex);
206-
#else
207-
lvb.ScrollIntoView(selectedItem);
208-
#endif
209-
}
195+
ScrollSelectionIntoView();
210196
}
211197
#endif
212198

@@ -222,6 +208,33 @@ internal virtual void OnSelectedItemChanged(object oldSelectedItem, object selec
222208
}
223209
}
224210

211+
private void ScrollSelectionIntoView()
212+
{
213+
if (SelectedIndex == -1 || !IsInLiveTree) return;
214+
215+
if (this is ListViewBase lvb)
216+
{
217+
#if __APPLE_UIKIT__
218+
// workaround to prevent scrolling when it is not ready
219+
// without this, the ios TabView could render blank if the selection happens too early.
220+
if (ContainerFromIndex(SelectedIndex) is not FrameworkElement { IsLoaded: true }) return;
221+
#endif
222+
223+
#if __APPLE_UIKIT__ || __ANDROID__
224+
lvb.InstantScrollToIndex(SelectedIndex);
225+
#else
226+
lvb.ScrollIntoView(SelectedItem);
227+
#endif
228+
}
229+
#if __APPLE_UIKIT__ || __ANDROID__
230+
#else
231+
else if (this is ComboBox cb)
232+
{
233+
cb.ScrollIntoView(SelectedItem);
234+
}
235+
#endif
236+
}
237+
225238
internal void TryUpdateSelectorItemIsSelected(object item, bool isSelected)
226239
{
227240
if (item is SelectorItem si)

0 commit comments

Comments
 (0)