From 8f6073d389200d319ba7f59f48f30e5c17ef3d26 Mon Sep 17 00:00:00 2001 From: xiaoy312 Date: Wed, 4 Dec 2024 12:30:11 -0500 Subject: [PATCH 1/2] fix(TabBar): dp exceptions when using TBI as ItemTemplate root --- .../Tests/TabBarTests.cs | 20 +++ src/Uno.Toolkit.UI/Controls/TabBar/TabBar.cs | 120 ++++++++++++------ .../Controls/TabBar/TabBarItem.Properties.cs | 2 + .../Helpers/VisualTreeHelperEx.cs | 7 + 4 files changed, 109 insertions(+), 40 deletions(-) diff --git a/src/Uno.Toolkit.RuntimeTests/Tests/TabBarTests.cs b/src/Uno.Toolkit.RuntimeTests/Tests/TabBarTests.cs index 49556f0a9..6f58fbcc3 100644 --- a/src/Uno.Toolkit.RuntimeTests/Tests/TabBarTests.cs +++ b/src/Uno.Toolkit.RuntimeTests/Tests/TabBarTests.cs @@ -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(""" + + + + """), + }; + await UnitTestUIContentHelperEx.SetContentAndWait(SUT); + } + [TestMethod] [DataRow(new int[0], null)] [DataRow(new[] { 1 }, 1)] diff --git a/src/Uno.Toolkit.UI/Controls/TabBar/TabBar.cs b/src/Uno.Toolkit.UI/Controls/TabBar/TabBar.cs index 5193cf862..9ad5a2bfa 100644 --- a/src/Uno.Toolkit.UI/Controls/TabBar/TabBar.cs +++ b/src/Uno.Toolkit.UI/Controls/TabBar/TabBar.cs @@ -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) @@ -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; } } @@ -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; @@ -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) diff --git a/src/Uno.Toolkit.UI/Controls/TabBar/TabBarItem.Properties.cs b/src/Uno.Toolkit.UI/Controls/TabBar/TabBarItem.Properties.cs index 293ceeeb6..2f62e4e33 100644 --- a/src/Uno.Toolkit.UI/Controls/TabBar/TabBarItem.Properties.cs +++ b/src/Uno.Toolkit.UI/Controls/TabBar/TabBarItem.Properties.cs @@ -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; diff --git a/src/Uno.Toolkit.UI/Helpers/VisualTreeHelperEx.cs b/src/Uno.Toolkit.UI/Helpers/VisualTreeHelperEx.cs index 030df4e71..cf9a2a7b6 100644 --- a/src/Uno.Toolkit.UI/Helpers/VisualTreeHelperEx.cs +++ b/src/Uno.Toolkit.UI/Helpers/VisualTreeHelperEx.cs @@ -195,6 +195,13 @@ public static IEnumerable 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 From 1815d9b129ebbe978baeeab9bd1ee3cbeb02e694 Mon Sep 17 00:00:00 2001 From: xiaoy312 Date: Mon, 6 Jan 2025 13:43:12 -0500 Subject: [PATCH 2/2] chore: fix tabbar indicator tests --- .../Tests/TabBarTests.cs | 48 +++++++++++-------- .../Helpers/VisualTreeHelperEx.cs | 9 ++-- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/Uno.Toolkit.RuntimeTests/Tests/TabBarTests.cs b/src/Uno.Toolkit.RuntimeTests/Tests/TabBarTests.cs index 6f58fbcc3..0a2ef84be 100644 --- a/src/Uno.Toolkit.RuntimeTests/Tests/TabBarTests.cs +++ b/src/Uno.Toolkit.RuntimeTests/Tests/TabBarTests.cs @@ -157,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(""" + + + + """), }; await UnitTestUIContentHelperEx.SetContentAndWait(SUT); + var presenter = SUT.GetFirstDescendant(x => x.Visibility == Visibility.Visible); + var indicator = presenter?.GetFirstDescendant("SutIndicator")!; + Assert.IsNotNull(indicator, "Failed to find Border#SutIndicator"); + source[0].IsSelected = true; await UnitTestsUIContentHelper.WaitForIdle(); @@ -183,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($""" + + + + """), 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(x => x.Visibility == Visibility.Visible); + var indicator = presenter?.GetFirstDescendant("SutIndicator")!; + Assert.IsNotNull(indicator, "Failed to find Border#SutIndicator"); + for (int i = 0; i < NumItems; i++) { SUT.SelectedIndex = i; diff --git a/src/Uno.Toolkit.UI/Helpers/VisualTreeHelperEx.cs b/src/Uno.Toolkit.UI/Helpers/VisualTreeHelperEx.cs index cf9a2a7b6..d8c0b8d70 100644 --- a/src/Uno.Toolkit.UI/Helpers/VisualTreeHelperEx.cs +++ b/src/Uno.Toolkit.UI/Helpers/VisualTreeHelperEx.cs @@ -135,6 +135,10 @@ void Print(object o, int depth) .OfType() .FirstOrDefault(); + public static T? GetFirstDescendant(this DependencyObject reference, string name) where T : FrameworkElement => GetDescendants(reference) + .OfType() + .FirstOrDefault(x => x.Name == name); + /// /// Returns the first descendant of a specified type that satisfies the . /// @@ -146,9 +150,8 @@ void Print(object o, int depth) .OfType() .FirstOrDefault(predicate); - public static T GetFirstDescendantOrThrow(this DependencyObject reference, string name) where T : FrameworkElement => GetDescendants(reference) - .OfType() - .FirstOrDefault(x => x.Name == name) ?? + public static T GetFirstDescendantOrThrow(this DependencyObject reference, string name) where T : FrameworkElement => + GetFirstDescendant(reference, name) ?? throw new Exception($"Unable to find element: {typeof(T).Name}#{name}"); ///