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(TabBar): dp exceptions when using TBI as ItemTemplate root #1299

Merged
merged 2 commits into from
Jan 7, 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
68 changes: 49 additions & 19 deletions src/Uno.Toolkit.RuntimeTests/Tests/TabBarTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,26 @@ namespace Uno.Toolkit.RuntimeTests.Tests
[RunsOnUIThread]
internal partial class TabBarTests // test cases
{
[TestMethod]
public async Task TabBar1285_ICS_With_TBI_ItemTemplate()
{
// note: this bug doesnt happen with ItemsSource = [TBI,...]
// because IsItemItsOwnContainerOverride=true. It only occurs
// with the ItemTemplate>DataTemplate>TBI setup (IsUsingOwnContainerAsTemplateRoot),
// which cause a ContentPresnter to be created as the item container.
var source = Enumerable.Range(0, 1).ToArray();
var SUT = new TabBar
{
ItemsSource = source,
ItemTemplate = XamlHelper.LoadXaml<DataTemplate>("""
<DataTemplate>
<utu:TabBarItem Content="{Binding}" />
</DataTemplate>
"""),
};
await UnitTestUIContentHelperEx.SetContentAndWait(SUT);
}

[TestMethod]
[DataRow(new int[0], null)]
[DataRow(new[] { 1 }, 1)]
Expand Down Expand Up @@ -137,15 +157,23 @@ public async Task SetSelectedIndex()
public async Task Verify_Indicator_Max_Size()
{
var source = Enumerable.Range(0, 3).Select(x => new TabBarItem { Content = x }).ToArray();
var indicator = new Border() { Height = 5, Background = new SolidColorBrush(Colors.Red) };
var SUT = new TabBar
{
ItemsSource = source,
SelectionIndicatorContent = indicator,
SelectionIndicatorContent = "asd",
SelectionIndicatorContentTemplate = XamlHelper.LoadXaml<DataTemplate>("""
<DataTemplate>
<Border x:Name="SutIndicator" Height="5" Background="Red" />
</DataTemplate>
"""),
};

await UnitTestUIContentHelperEx.SetContentAndWait(SUT);

var presenter = SUT.GetFirstDescendant<TabBarSelectionIndicatorPresenter>(x => x.Visibility == Visibility.Visible);
var indicator = presenter?.GetFirstDescendant<Border>("SutIndicator")!;
Assert.IsNotNull(indicator, "Failed to find Border#SutIndicator");

source[0].IsSelected = true;
await UnitTestsUIContentHelper.WaitForIdle();

Expand All @@ -163,37 +191,39 @@ public async Task Verify_Indicator_Max_Size()
}

[TestMethod]
[DataRow(Orientation.Horizontal, IndicatorTransitionMode.Snap, DisplayName = "Horizontal Snap")]
[DataRow(Orientation.Horizontal, IndicatorTransitionMode.Slide, DisplayName = "Horizontal Slide")]
[DataRow(Orientation.Vertical, IndicatorTransitionMode.Snap, DisplayName = "Vertical Snap")]
[DataRow(Orientation.Vertical, IndicatorTransitionMode.Slide, DisplayName = "Vertical Slide")]
[DataRow(Orientation.Horizontal, IndicatorTransitionMode.Snap)]
[DataRow(Orientation.Horizontal, IndicatorTransitionMode.Slide)]
[DataRow(Orientation.Vertical, IndicatorTransitionMode.Snap)]
[DataRow(Orientation.Vertical, IndicatorTransitionMode.Slide)]
public async Task Verify_Indicator_Transitions(Orientation orientation, IndicatorTransitionMode transitionMode)
{
const int NumItems = 3;
const double ItemSize = 100d;

var source = Enumerable.Range(0, NumItems).ToArray();
var indicator = new Border() { Background = new SolidColorBrush(Colors.Red) };
var SUT = new TabBar
{
Orientation = orientation,
ItemsSource = source,
SelectionIndicatorContent = indicator,
Width = orientation == Orientation.Horizontal ? ItemSize * NumItems : double.NaN,
Height = orientation == Orientation.Vertical ? ItemSize * NumItems : double.NaN,
SelectionIndicatorContent = "asd",
SelectionIndicatorContentTemplate = XamlHelper.LoadXaml<DataTemplate>($"""
<DataTemplate>
<Border x:Name="SutIndicator"
{(orientation == Orientation.Horizontal ? "Height" : "Width")}="5"
Background="Red" />
</DataTemplate>
"""),
SelectionIndicatorTransitionMode = transitionMode,
};

if (orientation == Orientation.Horizontal)
{
SUT.Width = ItemSize * NumItems;
indicator.Height = 5;
}
else
{
SUT.Height = ItemSize * NumItems;
indicator.Width = 5;
}

await UnitTestUIContentHelperEx.SetContentAndWait(SUT);

var presenter = SUT.GetFirstDescendant<TabBarSelectionIndicatorPresenter>(x => x.Visibility == Visibility.Visible);
var indicator = presenter?.GetFirstDescendant<Border>("SutIndicator")!;
Assert.IsNotNull(indicator, "Failed to find Border#SutIndicator");

for (int i = 0; i < NumItems; i++)
{
SUT.SelectedIndex = i;
Expand Down
120 changes: 80 additions & 40 deletions src/Uno.Toolkit.UI/Controls/TabBar/TabBar.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,32 +73,79 @@ protected override DependencyObject GetContainerForItemOverride()

protected override void PrepareContainerForItemOverride(DependencyObject element, object item)
{
base.PrepareContainerForItemOverride(element, item);
if (IsUsingOwnContainerAsTemplateRoot && element is ContentPresenter cp)
{
// ItemsControl::PrepareContainerForItemOverride will apply the ItemContainerStyle to the element which is not something we want here,
// since it can throw: The DP [WrongDP] is owned by [Control] and cannot be used on [ContentPresenter].
// While this doesnt break the control or the visual, it can cause a scaling performance degradation.

cp.ContentTemplate = ItemTemplate;
cp.ContentTemplateSelector = ItemTemplateSelector;

cp.DataContext = item;
SetContent(cp, item);

#if !HAS_UNO
// force template materialization
cp.Measure(Size.Empty);
#endif

void SetupTabBarItem(TabBarItem item)
if (cp.GetFirstChild() is TabBarItem tbi)
{
ApplyContainerStyle(tbi);
SetupTabBarItem(tbi);
}
}
else
{
item.IsSelected = IsSelected(IndexFromContainer(element));
item.Click += OnTabBarItemClick;
item.IsSelectedChanged += OnTabBarIsSelectedChanged;
base.PrepareContainerForItemOverride(element, item);
if (element is TabBarItem tbi)
{
SetupTabBarItem(tbi);
}
}

if (element is TabBarItem container)
void SetContent(ContentPresenter cp, object item)
{
SetupTabBarItem(container);
if (string.IsNullOrEmpty(DisplayMemberPath))
{
cp.Content = item;
}
else
{
cp.SetBinding(ContentPresenter.ContentProperty, new Binding
{
Source = item,
Path = new(DisplayMemberPath),
});
}
}
else if (IsUsingOwnContainerAsTemplateRoot &&
element is ContentPresenter outerContainer)
void ApplyContainerStyle(TabBarItem tbi)
{
var templateRoot = outerContainer.ContentTemplate.LoadContent();
if (templateRoot is TabBarItem tabBarItem)
var localStyleValue = tbi.ReadLocalValue(FrameworkElement.StyleProperty);
var isStyleSetFromTabBar = tbi.IsStyleSetFromTabBar;

if (localStyleValue == DependencyProperty.UnsetValue || isStyleSetFromTabBar)
{
outerContainer.ContentTemplate = null;
SetupTabBarItem(tabBarItem);
tabBarItem.DataContext = item;
tabBarItem.Style ??= ItemContainerStyle;
outerContainer.Content = tabBarItem;
var style = ItemContainerStyle ?? ItemContainerStyleSelector?.SelectStyle(item, tbi);
if (style is { })
{
tbi.Style = style;
tbi.IsStyleSetFromTabBar = true;
}
else
{
tbi.ClearValue(FrameworkElement.StyleProperty);
tbi.IsStyleSetFromTabBar = false;
}
}
}
void SetupTabBarItem(TabBarItem tbi)
{
tbi.IsSelected = IsSelected(IndexFromContainer(element));
tbi.Click += OnTabBarItemClick;
tbi.IsSelectedChanged += OnTabBarIsSelectedChanged;
}
}

internal virtual bool IsSelected(int index)
Expand All @@ -108,30 +155,26 @@ internal virtual bool IsSelected(int index)

protected override void ClearContainerForItemOverride(DependencyObject element, object item)
{
base.ClearContainerForItemOverride(element, item);

void TearDownTabBarItem(TabBarItem item)
if (IsUsingOwnContainerAsTemplateRoot && element is ContentPresenter cp)
{
item.Click -= OnTabBarItemClick;
item.IsSelectedChanged -= OnTabBarIsSelectedChanged;
if (!IsUsingOwnContainerAsTemplateRoot)
if (cp.GetFirstChild() is TabBarItem tbi)
{
item.Style = null;
TearDownTabBarItem(tbi);
}
}
if (element is TabBarItem container)
{
TearDownTabBarItem(container);
}
else if (IsUsingOwnContainerAsTemplateRoot &&
element is ContentPresenter outerContainer)
else
{
if (outerContainer.Content is TabBarItem innerContainer)
base.ClearContainerForItemOverride(element, item);
if (element is TabBarItem tbi)
{
TearDownTabBarItem(innerContainer);
innerContainer.DataContext = null;
TearDownTabBarItem(tbi);
}
outerContainer.Content = null;
}

void TearDownTabBarItem(TabBarItem item)
{
item.Click -= OnTabBarItemClick;
item.IsSelectedChanged -= OnTabBarIsSelectedChanged;
}
}

Expand Down Expand Up @@ -420,9 +463,9 @@ private void RaiseSelectionChangedEvent(object? prevItem, object? nextItem)
// Afterward, pass the resulting `ContentPresenter` as a parameter to this method.
internal TabBarItem? GetInnerContainer(DependencyObject? container)
{
if (IsUsingOwnContainerAsTemplateRoot && container is ContentPresenter cp)
if (IsUsingOwnContainerAsTemplateRoot)
{
return cp.Content as TabBarItem;
return (container as ContentPresenter)?.GetFirstChild() as TabBarItem;
}

return container as TabBarItem;
Expand All @@ -431,12 +474,9 @@ private void RaiseSelectionChangedEvent(object? prevItem, object? nextItem)
internal DependencyObject? InnerContainerFromIndex(int index)
{
var container = ContainerFromIndex(index);
if (IsUsingOwnContainerAsTemplateRoot && container is ContentPresenter cp)
{
container = cp.Content as DependencyObject;
}
var inner = GetInnerContainer(container);

return container;
return inner;
}

private TabBarItem? InnerContainerFromIndexSafe(int index)
Expand Down
2 changes: 2 additions & 0 deletions src/Uno.Toolkit.UI/Controls/TabBar/TabBarItem.Properties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ public object CommandParameter
DependencyProperty.Register(nameof(CommandParameter), typeof(object), typeof(TabBarItem), new PropertyMetadata(null, OnPropertyChanged));
#endregion

internal bool IsStyleSetFromTabBar { get; set; }

private static void OnPropertyChanged(DependencyObject sender, DependencyPropertyChangedEventArgs args)
{
var owner = (TabBarItem)sender;
Expand Down
16 changes: 13 additions & 3 deletions src/Uno.Toolkit.UI/Helpers/VisualTreeHelperEx.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@ void Print(object o, int depth)
.OfType<T>()
.FirstOrDefault();

public static T? GetFirstDescendant<T>(this DependencyObject reference, string name) where T : FrameworkElement => GetDescendants(reference)
.OfType<T>()
.FirstOrDefault(x => x.Name == name);

/// <summary>
/// Returns the first descendant of a specified type that satisfies the <paramref name="predicate"/>.
/// </summary>
Expand All @@ -146,9 +150,8 @@ void Print(object o, int depth)
.OfType<T>()
.FirstOrDefault(predicate);

public static T GetFirstDescendantOrThrow<T>(this DependencyObject reference, string name) where T : FrameworkElement => GetDescendants(reference)
.OfType<T>()
.FirstOrDefault(x => x.Name == name) ??
public static T GetFirstDescendantOrThrow<T>(this DependencyObject reference, string name) where T : FrameworkElement =>
GetFirstDescendant<T>(reference, name) ??
throw new Exception($"Unable to find element: {typeof(T).Name}#{name}");

/// <summary>
Expand Down Expand Up @@ -195,6 +198,13 @@ public static IEnumerable<DependencyObject> GetChildren(this DependencyObject re
.Select(x => VisualTreeHelper.GetChild(reference, x));
}

public static DependencyObject? GetFirstChild(this DependencyObject reference)
{
return VisualTreeHelper.GetChildrenCount(reference) > 0
? VisualTreeHelper.GetChild(reference, 0)
: null;
}

public static DependencyObject? GetTemplateRoot(this DependencyObject o) => o?.GetChildren().FirstOrDefault();
}
internal static partial class VisualTreeHelperEx // TreeGraph helper methods
Expand Down
Loading