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

feat: nested selection host support for IR-extension #618

Open
wants to merge 2 commits into
base: main
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
12 changes: 12 additions & 0 deletions doc/helpers/itemsrepeater-extensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ Provides selection support for `ItemsRepeater`.
## Properties
Property|Type|Description
-|-|-
IsSelectionHost|bool|Used to mark an element within the ItemsRepeater.ItemTemplate to be the host control that will handle the selection.\*
SelectedItem|object|Two-ways bindable property for the current/first(in Multiple mode) selected item.\*
SelectedIndex|int|Two-ways bindable property for the current/first(in Multiple mode) selected index.\*
SelectedItems|IList\<object>|Two-ways bindable property for the current selected items.\*
SelectedIndexes|IList\<int>|Two-ways bindable property for the current selected indexes.\*
SelectionMode|ItemsSelectionMode|Gets or sets the selection behavior: `None`, `SingleOrNone`, `Single`, `Multiple` <br/> note: Changing this value will cause the `Selected-`properties to be re-coerced.
UseNestedSelectionHost|bool|Used to signal a selection-host should be found in the ItemTemplate, and it would replace the item template root.\*


### Remarks
- `Selected-`properties only takes effect when `SelectionMode` is set to a valid value that is not `None`.
Expand All @@ -21,6 +24,14 @@ SelectionMode|ItemsSelectionMode|Gets or sets the selection behavior: `None`, `S
- `SingleOrNone`: Up to one item can be selected at a time. The current item can be deselected.
- `Single`: One item is selected at any time. The current item cannot be deselected.
- `Multiple`: The current item cannot be deselected.
- Use `IsSelectionHost` and `UseNestedSelectionHost` when the target of selection cannot be the root element of the ItemTemplate:
```xml
<muxc:ItemsRepeater utu:ItemsRepeaterExtensions.UseNestedSelectionHost="True">
<muxc:ItemsRepeater.ItemTemplate>
<DataTemplate>
<Border>
<utu:Chip Content="{Binding}" utu:ItemsRepeaterExtensions.IsSelectionHost="True"/>
```

## Usage
```xml
Expand All @@ -45,4 +56,5 @@ xmlns:muxc="using:Microsoft.UI.Xaml.Controls"

### Remarks
- The selection feature from this extensions support ItemTemplate whose the root element is a `SelectorItem` or `ToggleButton`(which includes `Chip`).
- Use `IsSelectionHost` and `UseNestedSelectionHost` when the target of selection cannot be the root element of the ItemTemplate.
- `RadioButton`: Multiple mode is not supported due to control limitation.
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ internal static ItemsRepeater SetupItemsRepeater(object source, ItemsSelectionMo
ItemsSource = source,
ItemTemplate = XamlHelper.LoadXaml<DataTemplate>("""
<DataTemplate>
<utu:Chip />
<utu:Chip Content="{Binding}" />
</DataTemplate>
"""),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,17 @@
using System.Text;
using System.Threading.Tasks;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Windows.UI.Xaml;
using Uno.Toolkit.RuntimeTests.Helpers;
using Uno.Toolkit.UI;
using Uno.UI.RuntimeTests;

#if IS_WINUI
using Microsoft.UI.Xaml;
#else
using Windows.UI.Xaml;
#endif

using ChipControl = Uno.Toolkit.UI.Chip; // ios/macos: to avoid collision with `global::Chip` namespace...
using ItemsRepeater = Microsoft.UI.Xaml.Controls.ItemsRepeater;
using static Uno.Toolkit.RuntimeTests.Tests.ItemsRepeaterChipTests; // to borrow helper methods

Expand Down Expand Up @@ -40,4 +47,31 @@ public async Task When_Selection_Property_Changed(string property)
})();
Assert.AreEqual(true, IsChipSelectedAt(SUT, 1));
}

[TestMethod]
public async Task When_NestedSelectionHost()
{
var source = Enumerable.Range(0, 3).ToList();
var SUT = new ItemsRepeater
{
ItemsSource = source,
ItemTemplate = XamlHelper.LoadXaml<DataTemplate>("""
<DataTemplate>
<Border>
<utu:Chip Content="{Binding}" utu:ItemsRepeaterExtensions.IsSelectionHost="True" />
</Border>
</DataTemplate>
"""),
};
ItemsRepeaterExtensions.SetUseNestedSelectionHost(SUT, true);
ItemsRepeaterExtensions.SetSelectionMode(SUT, ItemsSelectionMode.Single);
ItemsRepeaterExtensions.SetSelectedIndex(SUT, 1);

await UnitTestUIContentHelperEx.SetContentAndWait(SUT);

var root = SUT.TryGetElement(1);
var chip = root?.FindChild<ChipControl>();

Assert.IsTrue(chip?.IsChecked == true);
}
}
149 changes: 135 additions & 14 deletions src/Uno.Toolkit.UI/Behaviors/ItemsRepeaterExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
using System;
#if __ANDROID__ || NETSTANDARD // (NETSTD contains both wasm+skia; only wasm is needed, and the check is done at runtime)
#define APPLY_UNO12632_WORKAROUND
#endif

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Runtime.InteropServices;
using System.Text;
using System.Windows.Input;
using Microsoft.Extensions.Logging;
Expand All @@ -28,6 +33,27 @@ public static partial class ItemsRepeaterExtensions
{
private static ILogger _logger { get; } = typeof(CommandExtensions).Log();

#region DependencyProperty: IsSelectionHost

/// <summary>
/// Property used to mark an element within the ItemsRepeater.ItemTemplate to be the host control that will handle the selection.
/// </summary>
/// <remarks>
/// This is used when the target of selection cannot be the root element of the ItemTemplate.
/// Note that <seealso cref="UseNestedSelectionHostProperty"/> should also be set on the ItemRepeater when using this property.
/// </remarks>
public static DependencyProperty IsSelectionHostProperty { [DynamicDependency(nameof(GetIsSelectionHost))] get; } = DependencyProperty.RegisterAttached(
"IsSelectionHost",
typeof(bool),
typeof(ItemsRepeaterExtensions),
new PropertyMetadata(default(bool)));

[DynamicDependency(nameof(SetIsSelectionHost))]
public static bool GetIsSelectionHost(DependencyObject obj) => (bool)obj.GetValue(IsSelectionHostProperty);
[DynamicDependency(nameof(GetIsSelectionHost))]
public static void SetIsSelectionHost(DependencyObject obj, bool value) => obj.SetValue(IsSelectionHostProperty, value);

#endregion
#region DependencyProperty: IsSynchronizingSelection

private static DependencyProperty IsSynchronizingSelectionProperty { [DynamicDependency(nameof(GetIsSynchronizingSelection))] get; } = DependencyProperty.RegisterAttached(
Expand Down Expand Up @@ -126,6 +152,24 @@ public static partial class ItemsRepeaterExtensions
private static void SetSelectionSubscription(ItemsRepeater obj, IDisposable value) => obj.SetValue(SelectionSubscriptionProperty, value);

#endregion
#region DependencyProperty: UseNestedSelectionHost

/// <summary>
/// Property used to signal a selection-host should be found in the ItemTemplate, and it would replace the item template root.
/// </summary>
public static DependencyProperty UseNestedSelectionHostProperty { [DynamicDependency(nameof(GetUseNestedSelectionHost))] get; } = DependencyProperty.RegisterAttached(
"UseNestedSelectionHost",
typeof(bool),
typeof(ItemsRepeaterExtensions),
new PropertyMetadata(default(bool)));

[DynamicDependency(nameof(SetUseNestedSelectionHost))]
public static bool GetUseNestedSelectionHost(DependencyObject obj) => (bool)obj.GetValue(UseNestedSelectionHostProperty);
[DynamicDependency(nameof(GetUseNestedSelectionHost))]
public static void SetUseNestedSelectionHost(DependencyObject obj, bool value) => obj.SetValue(UseNestedSelectionHostProperty, value);

#endregion


#region ItemCommand Impl
internal static void OnItemCommandChanged(ItemsRepeater sender, DependencyPropertyChangedEventArgs e)
Expand All @@ -150,14 +194,13 @@ internal static void OnItemCommandChanged(ItemsRepeater sender, DependencyProper

private static void OnItemsRepeaterCommandTapped(object sender, TappedRoutedEventArgs e)
{
// ItemsRepeater is more closely related to Panel than ItemsControl, and it cannot be templated.
// It is safe to assume all direct children of IR are materialized item template,
// and there can't be header/footer or wrapper (ItemContainer) among them.

if (sender is not ItemsRepeater ir) return;
if (e.OriginalSource is ItemsRepeater) return;
if (e.OriginalSource is DependencyObject source)
{
// Unlike for selection behaviors, we don't need to find the "selection host".
// The selection host is a unrelated concept in the command setup. Additionally,
// the template root would generally have the same context as the selection host.
Comment on lines +201 to +203
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Unlike for selection behaviors, we don't need to find the "selection host".
// The selection host is a unrelated concept in the command setup. Additionally,
// the template root would generally have the same context as the selection host.
// Unlike for selection behaviors, we don't need to find the "selection host".
// The selection host is an unrelated concept in the command setup. Additionally,
// the template root would generally have the same context as the "selection host".

if (ir.FindRootElementOf(source) is FrameworkElement root)
{
CommandExtensions.TryInvokeCommand(ir, CommandExtensions.GetCommandParameter(root) ?? root.DataContext);
Expand All @@ -175,7 +218,7 @@ private static void OnItemsRepeaterCommandTapped(object sender, TappedRoutedEven

// ItemsRepeater's children contains only materialized element; materialization and de-materialization can be track with
// ElementPrepared and ElementClearing events. Recycled elements are reused based on FIFO-rule, resulting in index desync.
// Selection state saved on the element (LVI.IsSelect, Chip.IsChecked) will also desync when it happens.
// Selection state is saved on the element (LVI.IsSelect, Chip.IsChecked) will also desync when it happens.
// !!! So it is important to save the selection state into a dp, and validate against that on element materialization and correct when necessary.

// Unlike ToggleButton (or Chip which derives from), SelectorItem is not normally selectable on click, unless nested under a Selector.
Expand All @@ -196,12 +239,18 @@ private static void OnSelectionModeChanged(DependencyObject sender, DependencyPr
{
ir.Tapped += OnItemsRepeaterTapped;
ir.ElementPrepared += OnItemsRepeaterElementPrepared;
#if APPLY_UNO12632_WORKAROUND
ir.ElementClearing += OnItemsRepeaterElementClearing;
#endif

SetSelectionSubscription(ir, new CompositeDisposable(
Disposable.Create(() =>
{
ir.Tapped -= OnItemsRepeaterTapped;
ir.ElementPrepared -= OnItemsRepeaterElementPrepared;
#if APPLY_UNO12632_WORKAROUND
ir.ElementClearing -= OnItemsRepeaterElementClearing;
#endif
}),
ir.RegisterDisposablePropertyChangedCallback(ItemsRepeater.ItemsSourceProperty, OnItemsRepeaterItemsSourceChanged)
));
Expand All @@ -212,7 +261,10 @@ private static void OnSelectionModeChanged(DependencyObject sender, DependencyPr
try
{
SetIsSynchronizingSelection(ir, true);


#if APPLY_UNO12632_WORKAROUND
ApplyNestedTappedEventBlocker(ir);
#endif
TrySynchronizeDefaultSelection(ir);
SynchronizeMaterializedElementsSelection(ir);
}
Expand Down Expand Up @@ -315,8 +367,17 @@ private static void OnItemsRepeaterElementPrepared(ItemsRepeater sender, Microso
// and we can rely on it to synchronize the selection on the view-level.
var selected = GetSelectedIndexes(sender)?.Contains(args.Index) ?? false;

SetItemSelection(args.Element, selected);
SetItemSelection(sender, args.Element, selected);
#if APPLY_UNO12632_WORKAROUND
ApplyNestedTappedEventBlocker(sender, args.Element);
#endif
}
#if APPLY_UNO12632_WORKAROUND
private static void OnItemsRepeaterElementClearing(ItemsRepeater sender, Microsoft.UI.Xaml.Controls.ItemsRepeaterElementClearingEventArgs args)
{
ClearNestedTappedEventBlocker(sender, args.Element);
}
#endif
private static void OnItemsRepeaterItemsSourceChanged(DependencyObject sender, DependencyProperty dp)
{
// When we reach here, ItemsSourceView is already updated.
Expand Down Expand Up @@ -345,7 +406,7 @@ private static void OnItemsRepeaterTapped(object sender, TappedRoutedEventArgs e
if (e.OriginalSource is ItemsRepeater) return;
if (e.OriginalSource is DependencyObject source)
{
if (ir.FindRootElementOf(source) is { } element)
if (ir.FindRootElementOf(source) is UIElement element)
{
ToggleItemSelectionAtCoerced(ir, ir.GetElementIndex(element));
}
Expand Down Expand Up @@ -495,7 +556,7 @@ private static void SynchronizeMaterializedElementsSelection(ItemsRepeater ir)
if (element is UIElement uie &&
ir.GetElementIndex(uie) is var index && index != -1)
{
SetItemSelection(uie, indexes.Contains(index));
SetItemSelection(ir, uie, indexes.Contains(index));
}
}
}
Expand Down Expand Up @@ -532,7 +593,7 @@ internal static void ToggleItemSelectionAtCoerced(ItemsRepeater ir, int index)
{
if (ir.TryGetElement(diffIndex) is { } materialized)
{
SetItemSelection(materialized, updated.Contains(diffIndex));
SetItemSelection(ir, materialized, updated.Contains(diffIndex));
}
else
{
Expand All @@ -546,13 +607,17 @@ internal static void ToggleItemSelectionAtCoerced(ItemsRepeater ir, int index)
SetIsSynchronizingSelection(ir, false);
}
}
internal static void SetItemSelection(DependencyObject x, bool value)
internal static void SetItemSelection(ItemsRepeater ir, DependencyObject itemRoot, bool value)
{
if (x is SelectorItem si)
var host = GetUseNestedSelectionHost(ir)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we maybe avoid needing to have both a UseNestedSelectionHost and a IsSelectionHost property? Could we get away with just IsSelectionHost?

Perhaps just search for a selection host if the itemRoot itself is anything other than a SelectorItem or ToggleButton, otherwise assume the root should be the selection host? Or are there common scenarios that this logic wouldn't support?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we absolutely need both properties, perhaps we should bubble up some sort of warning/error when one is used without the other?

? (itemRoot.GetFirstDescendant<DependencyObject>(GetIsSelectionHost) ?? itemRoot)
: itemRoot;

if (host is SelectorItem si)
{
si.IsSelected = value;
}
else if (x is ToggleButton toggle)
else if (host is ToggleButton toggle)
{
toggle.IsChecked = value;
}
Expand All @@ -561,4 +626,60 @@ internal static void SetItemSelection(DependencyObject x, bool value)
// todo: generic item is not supported
}
}

#if APPLY_UNO12632_WORKAROUND
// note: This issue only happens with ButtonBase on wasm and android where the Tapped event is registered on.

private static void ApplyNestedTappedEventBlocker(ItemsRepeater ir)
{
if (!IsWasm && !IsAndroid) return;

if (ir.ItemsSourceView is { Count: > 0 })
{
foreach (var element in ir.GetChildren())
{
ApplyNestedTappedEventBlocker(ir, element);
}
}
}
private static void ApplyNestedTappedEventBlocker(ItemsRepeater ir, DependencyObject itemRoot)
{
Console.WriteLine($"@xy droid:{IsAndroid}, wasm:{IsWasm}");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leftover debug message?

if (!IsWasm && !IsAndroid) return;

var host = GetUseNestedSelectionHost(ir)
? (itemRoot.GetFirstDescendant<DependencyObject>(GetIsSelectionHost) ?? itemRoot)
: itemRoot;

if (host is ButtonBase button)
{
button.Tapped -= BlockNestedTappedEvent;
button.Tapped += BlockNestedTappedEvent;
}
}
private static void ClearNestedTappedEventBlocker(ItemsRepeater ir, DependencyObject itemRoot)
{
if (!IsWasm && !IsAndroid) return;

var host = GetUseNestedSelectionHost(ir)
? (itemRoot.GetFirstDescendant<DependencyObject>(GetIsSelectionHost) ?? itemRoot)
: itemRoot;

if (host is ButtonBase button)
{
button.Tapped -= BlockNestedTappedEvent;
}
}
private static void BlockNestedTappedEvent(object sender, TappedRoutedEventArgs e)
{
// prevent the event to bubble up to the ItemsReapter.
e.Handled = true;
}

private static bool IsAndroid { get; }
#if __ANDROID__
= true;
#endif
private static bool IsWasm { get; } = RuntimeInformation.IsOSPlatform(OSPlatform.Create("BROWSER"));
#endif
}
14 changes: 11 additions & 3 deletions src/Uno.Toolkit.UI/Helpers/ItemsSelectionHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,22 @@ public static int IndexOf(this ItemsSourceView isv, object item)
}
#endif

/// <summary>
/// Update the selection indexes by toggling the provided index, and then coerced according to the selection mode.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL "indexes" is a thing and you're only supposed to use "indices" in the context of mathematical expressions

/// </summary>
/// <param name="mode">Selection mode</param>
/// <param name="length">Length of items</param>
/// <param name="selection">Current selection</param>
/// <param name="index">Index to toggle</param>
/// <returns>Updated selection indexes</returns>
public static int[] ToggleSelectionAtCoerced(ItemsSelectionMode mode, int length, IList<int> selection, int index)
{
if (length < 0) throw new ArgumentOutOfRangeException(nameof(length));
if (0 > index || index >= length) throw new ArgumentOutOfRangeException(nameof(index));

if (mode is ItemsSelectionMode.None)
{
return Array.Empty<int>();
return Array.Empty<int>();
}
else if (mode is ItemsSelectionMode.Single or ItemsSelectionMode.SingleOrNone)
{
Expand All @@ -67,14 +75,14 @@ public static int[] ToggleSelectionAtCoerced(ItemsSelectionMode mode, int length
}
}

public static UIElement? FindRootElementOf(this ItemsRepeater ir, DependencyObject node)
public static DependencyObject? FindRootElementOf(this ItemsRepeater ir, DependencyObject node)
{
// e.OriginalSource is the top-most element under the cursor.
// In order to find the materialized element, we have to walk up the visual-tree, to the first element right below IR:
// ItemsRepeater > (item template root) > (layer0...n) > (tapped element)
return node.GetAncestors(includeCurrent: true)
.ZipSkipOne()
.FirstOrDefault(x => x.Current is ItemsRepeater)
.Previous as UIElement;
.Previous;
}
}