fix(TabBar): dp exceptions when using TBI as ItemTemplate root

unoplatform · Jan 6, 2025 · 8f6073d · 8f6073d
1 parent 36b844a
commit 8f6073d
Show file tree

Hide file tree

Showing 4 changed files with 109 additions and 40 deletions.
diff --git 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<DataTemplate>("""
+					<DataTemplate>
+						<utu:TabBarItem Content="{Binding}" />
+					</DataTemplate>
+				"""),
+			};
+			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
@@ -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
@@ -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
@@ -195,6 +195,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