Merge pull request #3860 from unoplatform/dev/xygu/3358/combobox-plac…

…eholdertext fix(combobox): PlaceholderText not displaying
unoplatform · Sep 2, 2020 · c820ccb · c820ccb
2 parents 91df7e2 + bbe5e47
commit c820ccb
Show file tree

Hide file tree

Showing 5 changed files with 145 additions and 27 deletions.
diff --git a/...mplesApp.UITests/Windows_UI_Xaml_Controls/ComboBoxTests/UnoSamples_Tests.ComboBoxTests.cs b/...mplesApp.UITests/Windows_UI_Xaml_Controls/ComboBoxTests/UnoSamples_Tests.ComboBoxTests.cs
@@ -214,5 +214,67 @@ public void ComboBoxTests_ToggleDisabled()
 
 			TakeScreenshot("ComboBox Disabled");
 		}
+
+		[Test]
+		[AutoRetry]
+		[ActivePlatforms(Platform.Android)]
+		public void ComboBoxTests_PlaceholderText() => ComboBoxTests_PlaceholderText_Impl("TestBox", 4, combo =>
+		{
+			var implicitTextBlock = combo.Descendant().Marked("ContentPresenter").Child;
+			var text = implicitTextBlock.GetDependencyPropertyValue<string>("Text");
+
+			Assert.AreEqual("5", text, "item #4 (text: 5) should be now selected");
+		});
+
+		[Test]
+		[AutoRetry]
+		[ActivePlatforms(Platform.Android)]
+		public void ComboBoxTests_PlaceholderText_With_ItemTemplate() => ComboBoxTests_PlaceholderText_Impl("TestBox2", 4, combo =>
+		{
+			var descendants = combo.Descendant().Marked("ContentPresenter").Descendant();
+			//<StackPanel Orientation="Horizontal"><!-- templateRoot -->
+			//	<Rectangle ... />
+			//	<TextBlock Text="Item:" />
+			//	<TextBlock Text="{Binding}" />
+			//</StackPanel>
+			var offset = 2; // skipping the ContentPresenter#0 and the StackPanel#1
+			var text1 = descendants.AtIndex(offset + 1).GetDependencyPropertyValue<string>("Text");
+			var text2 = descendants.AtIndex(offset + 2).GetDependencyPropertyValue<string>("Text");
+
+			Assert.AreEqual("Item:", text1, "item #4 should be now selected with ItemTemplate");
+			Assert.AreEqual("5", text2, "item #4 (value: 5) should be now selected with ItemTemplate");
+		});
+
+		public void ComboBoxTests_PlaceholderText_Impl(string targetName, int selectionIndex, Action<QueryEx> selectionValidation)
+		{
+			Run("SamplesApp.Wasm.Windows_UI_Xaml_Controls.ComboBox.ComboBox_PlaceholderText");
+			_app.WaitForElement("sampleControl");
+
+			var testComboBox = _app.Marked(targetName);
+			var resetButton = _app.Marked("ResetButton");
+
+			// check initial value
+			var placeholderTextBlock = testComboBox.Descendant().Marked("PlaceholderTextBlock");
+			var expectedPlaceholderText = testComboBox.GetDependencyPropertyValue<string>("PlaceholderText");
+			var actualPlaceholderText = placeholderTextBlock.GetDependencyPropertyValue<string>("Text");
+			Assert.AreEqual(expectedPlaceholderText, actualPlaceholderText, "PlaceholderText should be shown prior to selection");
+
+			// open combobox flyout
+			testComboBox.FastTap();
+			var popupBorder = _app.Marked("PopupBorder");
+			_app.WaitForElement(popupBorder);
+
+			// select item at {selectionIndex}
+			var item5 = popupBorder.Descendant().WithClass("ComboBoxItem").AtIndex(selectionIndex);
+			item5.FastTap();
+			_app.WaitForDependencyPropertyValue<int>(testComboBox, "SelectedIndex", selectionIndex);
+			selectionValidation(testComboBox);
+
+			// clear selection
+			resetButton.FastTap();
+			_app.WaitForDependencyPropertyValue<int>(testComboBox, "SelectedIndex", -1);
+			actualPlaceholderText = placeholderTextBlock.GetDependencyPropertyValue<string>("Text");
+			Assert.AreEqual(expectedPlaceholderText, actualPlaceholderText, "PlaceholderText should be shown again when the selection is cleared");
+		}
 	}
 }
diff --git a/...SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/ComboBox/ComboBox_PlaceholderText.xaml b/...SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/ComboBox/ComboBox_PlaceholderText.xaml
@@ -10,14 +10,29 @@
 			 d:DesignHeight="300"
 			 d:DesignWidth="400">
 
-	<controls:SampleControl SampleDescription="ComboBox with PlaceholderText">
-		<controls:SampleControl.SampleContent>
-			<DataTemplate>
+	<StackPanel>
+		<ComboBox x:Name="TestBox"
+				  PlaceholderText="PlaceholderText"
+				  ItemsSource="{Binding [SampleItems]}" />
+		<ComboBox x:Name="TestBox2"
+				  PlaceholderText="PlaceholderText + ItemTemplate"
+				  ItemsSource="{Binding [SampleItems]}">
+			<ComboBox.ItemTemplate>
+				<DataTemplate>
+					<StackPanel Orientation="Horizontal">
+						<Rectangle Height="12"
+								   Width="12"
+								   Fill="Pink" />
+						<TextBlock Text="Item:" />
+						<TextBlock Text="{Binding}" />
+					</StackPanel>
+				</DataTemplate>
+			</ComboBox.ItemTemplate>
+		</ComboBox>
 
-				<ComboBox PlaceholderText="PlaceholderText"
-						  ItemsSource="{Binding [SampleItems]}" />
+		<Button x:Name="ResetButton"
+				Content="Reset Selection"
+				Click="ResetSelection" />
+	</StackPanel>
 
-			</DataTemplate>
-		</controls:SampleControl.SampleContent>
-	</controls:SampleControl>
 </UserControl>
diff --git a/...plesApp/UITests.Shared/Windows_UI_Xaml_Controls/ComboBox/ComboBox_PlaceholderText.xaml.cs b/...plesApp/UITests.Shared/Windows_UI_Xaml_Controls/ComboBox/ComboBox_PlaceholderText.xaml.cs
@@ -1,6 +1,7 @@
 using Windows.UI.Xaml.Controls;
 using SamplesApp.Windows_UI_Xaml_Controls.Models;
 using Uno.UI.Samples.Controls;
+using Windows.UI.Xaml;
 
 namespace SamplesApp.Wasm.Windows_UI_Xaml_Controls.ComboBox
 {
@@ -11,5 +12,11 @@ public ComboBox_PlaceholderText()
 		{
 			this.InitializeComponent();
 		}
+
+		void ResetSelection(object sender, RoutedEventArgs e)
+		{
+			TestBox.SelectedItem = null;
+			TestBox2.SelectedItem = null;
+		}
 	}
 }
diff --git a/src/Uno.UI/UI/Xaml/Controls/ComboBox/ComboBox.cs b/src/Uno.UI/UI/Xaml/Controls/ComboBox/ComboBox.cs
@@ -43,9 +43,12 @@ public partial class ComboBox : ListViewBase // TODO: Selector
 		public event EventHandler<object> DropDownClosed;
 		public event EventHandler<object> DropDownOpened;
 
+		private bool _areItemTemplatesForwarded = false;
+
 		private IPopup _popup;
 		private Border _popupBorder;
 		private ContentPresenter _contentPresenter;
+		private TextBlock _placeholderTextBlock;
 		private ContentPresenter _headerContentPresenter;
 
 		/// <summary>
@@ -79,6 +82,7 @@ protected override void OnApplyTemplate()
 			_popup = this.GetTemplateChild("Popup") as IPopup;
 			_popupBorder = this.GetTemplateChild("PopupBorder") as Border;
 			_contentPresenter = this.GetTemplateChild("ContentPresenter") as ContentPresenter;
+			_placeholderTextBlock = this.GetTemplateChild("PlaceholderTextBlock") as TextBlock;
 
 			if (_popup is PopupBase popup)
 			{
@@ -93,18 +97,7 @@ protected override void OnApplyTemplate()
 
 			if (_contentPresenter != null)
 			{
-				_contentPresenter.SetBinding(
-					ContentPresenter.ContentTemplateProperty,
-					new Binding(new PropertyPath("ItemTemplate"), null)
-					{
-						RelativeSource = RelativeSource.TemplatedParent
-					});
-				_contentPresenter.SetBinding(
-					ContentPresenter.ContentTemplateSelectorProperty,
-					new Binding(new PropertyPath("ItemTemplateSelector"), null)
-					{
-						RelativeSource = RelativeSource.TemplatedParent
-					});
+				_contentPresenter.SynchronizeContentWithOuterTemplatedParent = false;
 
 				_contentPresenter.DataContextChanged += (snd, evt) =>
 				{
@@ -248,10 +241,11 @@ internal override void OnItemClicked(int clickedIndex)
 
 		private void UpdateContentPresenter()
 		{
-			if (_contentPresenter != null)
+			if (_contentPresenter == null) return;
+
+			if (SelectedItem != null)
 			{
 				var item = GetSelectionContent();
-
 				var itemView = item as _View;
 
 				if (itemView != null)
@@ -273,12 +267,34 @@ private void UpdateContentPresenter()
 				}
 
 				_contentPresenter.Content = item;
-
 				if (itemView != null && itemView.GetVisualTreeParent() != _contentPresenter)
 				{
 					// Item may have been put back in list, reattach it to _contentPresenter
 					_contentPresenter.AddChild(itemView);
 				}
+				if (!_areItemTemplatesForwarded)
+				{
+					SetContentPresenterBinding(ContentPresenter.ContentTemplateProperty, nameof(ItemTemplate));
+					SetContentPresenterBinding(ContentPresenter.ContentTemplateSelectorProperty, nameof(ItemTemplateSelector));
+
+					_areItemTemplatesForwarded = true;
+				}
+			}
+			else
+			{
+				_contentPresenter.Content = _placeholderTextBlock;
+				if (_areItemTemplatesForwarded)
+				{
+					_contentPresenter.ClearValue(ContentPresenter.ContentTemplateProperty);
+					_contentPresenter.ClearValue(ContentPresenter.ContentTemplateSelectorProperty);
+
+					_areItemTemplatesForwarded = false;
+				}
+			}
+
+			void SetContentPresenterBinding(DependencyProperty targetProperty, string sourcePropertyPath)
+			{
+				_contentPresenter.SetBinding(targetProperty, new Binding(sourcePropertyPath) { RelativeSource = RelativeSource.TemplatedParent });
 			}
 		}
 

diff --git a/src/Uno.UI/UI/Xaml/Controls/ContentPresenter/ContentPresenter.cs b/src/Uno.UI/UI/Xaml/Controls/ContentPresenter/ContentPresenter.cs
@@ -53,6 +53,12 @@ private void InitializeContentPresenter()
 		{
 		}
 
+		/// <summary>
+		/// Indicates if the content should inherit templated parent from the presenter, or its templated parent.
+		/// </summary>
+		/// <remarks>Clear this flag to let the control nested directly under this ContentPresenter to inherit the correct templated parent</remarks>
+		internal bool SynchronizeContentWithOuterTemplatedParent { get; set; } = true;
+
 		/// <summary>
 		/// Determines if the current ContentPresenter is hosting a native control.
 		/// </summary>
@@ -695,11 +701,23 @@ private void SynchronizeContentTemplatedParent()
 			{
 				if (_contentTemplateRoot is IFrameworkElement binder)
 				{
-					var templatedParent = _contentTemplateRoot is ImplicitTextBlock
-						? this // ImplicitTextBlock is a special case that requires its TemplatedParent to be the ContentPresenter
-						: (this.TemplatedParent as IFrameworkElement)?.TemplatedParent;
+					binder.TemplatedParent = FindTemplatedParent();
 
-					binder.TemplatedParent = templatedParent;
+					DependencyObject FindTemplatedParent()
+					{
+						// ImplicitTextBlock is a special case that requires its TemplatedParent to be the ContentPresenter
+						if (_contentTemplateRoot is ImplicitTextBlock) return this;
+
+						// Sometimes when content is a child view defined in the xaml, the direct TemplatedParent should be used,
+						// but only if the content hasnt been overwritten yet. If the content has been overwritten,
+						// either ImplicitTextBlock or the DataTemplate (requiring the outter TemplatedParent) would has been used.
+						if (!SynchronizeContentWithOuterTemplatedParent && _dataTemplateUsedLastUpdate == null)
+						{
+							return this.TemplatedParent;
+						}
+
+						return (this.TemplatedParent as IFrameworkElement)?.TemplatedParent;
+					}
 				}
 				else if (_contentTemplateRoot is DependencyObject dependencyObject)
 				{