Skip to content

Commit

Permalink
Merge pull request #1299 from unoplatform/dev/xygu/20241204/tabbar-ne…
Browse files Browse the repository at this point in the history
…sted-tbi-containerstyle

fix(TabBar): dp exceptions when using TBI as ItemTemplate root
  • Loading branch information
Xiaoy312 authored Jan 7, 2025
2 parents c84aa79 + 1815d9b commit f20a9d4
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 62 deletions.
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

0 comments on commit f20a9d4

Please sign in to comment.