Skip to content

fix(ComboBox): updated selection not scrolled into view #20204

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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 @@ -1410,6 +1410,61 @@ public async Task When_ComboBox_Popup_Dismissed()
Assert.IsTrue(dropDownClosedFired, "DropDownClosed event was not fired");
}

[TestMethod]
public Task When_ComboBox_ScrollIntoView_SelectedItem() => When_ComboBox_ScrollIntoView_Selection(viaIndex: false);

[TestMethod]
public Task When_ComboBox_ScrollIntoView_SelectedIndex() => When_ComboBox_ScrollIntoView_Selection(viaIndex: true);

private async Task When_ComboBox_ScrollIntoView_Selection(bool viaIndex)
{
var source = ( // 12x20=240 items, enough to overflow
from a in "qwerasdfzxcv"
from b in Enumerable.Range(0, 20)
select string.Concat(a, b)
).ToArray();
var sut = new ComboBox
{
ItemsSource = source,
};
await UITestHelper.Load(sut, x => x.IsLoaded);

var popup = sut.FindFirstDescendantOrThrow<Popup>("Popup");

// open the popup
sut.IsDropDownOpen = true;
await UITestHelper.WaitFor(() => popup.IsOpen, timeoutMS: 2000, "timed out waiting on the popup to open");

// wait until the dropdown panel is ready
await UITestHelper.WaitFor(() => sut.ItemsPanelRoot is { }, timeoutMS: 2000, "timed out waiting on the ItemsPanelRoot");

// set selection
if (viaIndex)
{
sut.SelectedIndex = source.Length - 3;
}
else
{
sut.SelectedItem = source[^3];
}
await UITestHelper.WaitForIdle();

// sanity check
Assert.AreEqual(source.Length - 3, sut.SelectedIndex, "SelectedIndex should be the 3rd last");
Assert.AreEqual(source[^3], sut.SelectedItem, "SelectedItem should be the 3rd last");

var sv = sut.ItemsPanelRoot.FindFirstAncestorOrThrow<ScrollViewer>();
var cbi = sut.ContainerFromIndex(sut.SelectedIndex) as FrameworkElement;
Assert.IsNotNull(cbi, "Selected container should not be null");

var cbiAbsRect = new Rect(cbi.ActualOffset.X, cbi.ActualOffset.Y, cbi.ActualWidth, cbi.ActualHeight);
var viewportAbsRect = new Rect(sv.HorizontalOffset, sv.VerticalOffset, sv.ViewportWidth, sv.ViewportHeight);
var intersection = viewportAbsRect;
intersection.Intersect(cbiAbsRect);

Assert.IsTrue(cbiAbsRect == intersection, $"Selected container should be fully within viewport: CBI={PrettyPrint.FormatRect(cbiAbsRect)}, VP={PrettyPrint.FormatRect(viewportAbsRect)}");
}

public sealed class TwoWayBindingClearViewModel : IDisposable
{
public enum Themes
Expand Down
13 changes: 13 additions & 0 deletions src/Uno.UI/Extensions/ViewExtensions.visual-tree.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,19 @@ internal static bool TryGetDpValue<T>(object owner, string property, [NotNullWhe
.OfType<T>()
.FirstOrDefault();

/// <summary>
/// Returns the first ancestor of a specified type.
/// </summary>
/// <typeparam name="T">The type of ancestor to find.</typeparam>
/// <param name="reference">Any node of the visual tree</param>
/// <remarks>First Counting from the <paramref name="reference"/> and not from the root of tree.</remarks>
/// <exception cref="Exception">If the specified node could not be found.</exception>
public static T? FindFirstAncestorOrThrow<T>(this _View? reference) => EnumerateAncestors(reference)
.OfType<T>()
.FirstOrDefault() ??
throw new Exception($"Unable to find element: {typeof(T).Name}");


/// <summary>
/// Returns the first ancestor of a specified type that satisfies the <paramref name="predicate"/>.
/// </summary>
Expand Down
68 changes: 68 additions & 0 deletions src/Uno.UI/UI/Xaml/Controls/ComboBox/ComboBox.custom.cs
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,74 @@ internal Brush LightDismissOverlayBackground
set { SetValue(LightDismissOverlayBackgroundProperty, value); }
}

internal void ScrollIntoView(object item, ScrollIntoViewAlignment alignment = ScrollIntoViewAlignment.Default)
{
if (ItemsPanelRoot is null)
{
// Not ready.
return;
}

if (ContainerFromItem(item) is UIElement element)
{
// The container we want to jump to is already materialized, so just jump to it.
// 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)
ScrollIntoViewFastPath(element, alignment);
}
else if (VirtualizingPanel?.GetLayouter() is { } layouter)
{
#if __APPLE_UIKIT__ || __ANDROID__
// TODO
#else
layouter.ScrollIntoView(item, alignment);
#endif
}
}

private void ScrollIntoViewFastPath(UIElement element, ScrollIntoViewAlignment alignment)
{
if (ScrollViewer is { } sv && sv.Presenter is { } presenter)
{
var offsetXY = element.TransformToVisual(presenter).TransformPoint(
#if __SKIA__ // Skia correctly doesn't include the offsets in TransformToVisual
new Point(presenter.HorizontalOffset, presenter.VerticalOffset)
#else
Point.Zero
#endif
);

var orientation = ItemsPanelRoot?.PhysicalOrientation ?? Orientation.Vertical;

var (elementOffset, elementLength, presenterOffset, presenterViewportLength) =
orientation is Orientation.Vertical
? (offsetXY.Y, element.ActualSize.Y, presenter.VerticalOffset, presenter.ViewportHeight)
: (offsetXY.X, element.ActualSize.X, presenter.HorizontalOffset, presenter.ViewportWidth);

if (presenterOffset <= elementOffset && elementOffset + elementLength <= presenterOffset + presenterViewportLength)
{
// if the element is within the visible viewport, do nothing.
return;
}

// If we use the above offset directly, the item we want to jump to will be the start of the viewport, i.e. leading.
// For the default alignment, we move the element to either of the viewport ends (i.e. to the top or the bottom of the
// viewport. To move to the bottom, we scroll one "viewport page" less. This brings the element's start right after the
// viewport's length ends we then scroll again by elementLength so that the end of the element is the end of the viewport.
var newOffset = alignment is ScrollIntoViewAlignment.Default && presenterOffset < elementOffset
? elementOffset - presenterViewportLength + elementLength
: elementOffset;

if (orientation is Orientation.Vertical)
{
sv.ScrollToVerticalOffset(newOffset);
}
else
{
sv.ScrollToHorizontalOffset(newOffset);
}
}
}

internal static DependencyProperty LightDismissOverlayBackgroundProperty { get; } =
DependencyProperty.Register("LightDismissOverlayBackground", typeof(Brush), typeof(ComboBox), new FrameworkPropertyMetadata(null));

Expand Down
48 changes: 31 additions & 17 deletions src/Uno.UI/UI/Xaml/Controls/Primitives/Selector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,9 @@ internal virtual void OnSelectedItemChanged(object oldSelectedItem, object selec
isSelectionUnset = true;
}

var newIndex = -1;
if (!_changingSelectedIndex)
{
newIndex = IndexFromItem(selectedItem);
var newIndex = IndexFromItem(selectedItem);
if (SelectedIndex != newIndex)
{
SelectedIndex = newIndex;
Expand All @@ -191,22 +190,9 @@ internal virtual void OnSelectedItemChanged(object oldSelectedItem, object selec
}

#if !IS_UNIT_TESTS
if (newIndex != -1 && IsInLiveTree)
if (SelectedIndex != -1 && IsInLiveTree)
{
if (this is ListViewBase lvb
#if __APPLE_UIKIT__
// workaround to prevent scrolling when it is not ready
// without this, the ios TabView could render blank if the selection happens too early.
&& ContainerFromIndex(newIndex) is FrameworkElement { IsLoaded: true }
#endif
)
{
#if __APPLE_UIKIT__ || __ANDROID__
lvb.InstantScrollToIndex(newIndex);
#else
lvb.ScrollIntoView(selectedItem);
#endif
}
ScrollSelectionIntoView();
}
#endif

Expand All @@ -222,6 +208,34 @@ internal virtual void OnSelectedItemChanged(object oldSelectedItem, object selec
}
}

private void ScrollSelectionIntoView()
{
if (SelectedIndex == -1 || !IsInLiveTree) return;

if (this is ListViewBase lvb)
{
#if __APPLE_UIKIT__
// workaround to prevent scrolling when it is not ready
// without this, the ios TabView could render blank if the selection happens too early.
if (ContainerFromIndex(SelectedIndex) is not FrameworkElement { IsLoaded: true }) return;
#endif

#if __APPLE_UIKIT__ || __ANDROID__
lvb.InstantScrollToIndex(SelectedIndex);
#else
lvb.ScrollIntoView(SelectedItem);
#endif
}
else if (this is ComboBox cb)
{
#if __APPLE_UIKIT__ || __ANDROID__
// TODO
#else
cb.ScrollIntoView(SelectedItem);
#endif
}
}

internal void TryUpdateSelectorItemIsSelected(object item, bool isSelected)
{
if (item is SelectorItem si)
Expand Down
Loading