fix: Fix wrong MinuteIncrement coercion in TimePicker

jeromelaban · Jun 29, 2024 · 07aa8e3 · 07aa8e3
1 parent 7dca68c
commit 07aa8e3
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 28 deletions.
diff --git a/src/Uno.UI.FluentTheme.v2/themeresources_v2.xaml b/src/Uno.UI.FluentTheme.v2/themeresources_v2.xaml
@@ -10256,8 +10256,7 @@
         <ControlTemplate TargetType="Button">
           <!-- Uno specific (LinearGradientBrush borders): Additional Grid as template root needed #6457 -->
           <Grid>
-            <!-- Uno workaround: template-bind ContentTemplateSelector because it's not automatically propagated from the ContentControl #6452 -->
-            <ContentPresenter x:Name="ContentPresenter" Background="{TemplateBinding Background}" Foreground="{TemplateBinding Foreground}" BackgroundSizing="{TemplateBinding BackgroundSizing}" BorderBrush="{TemplateBinding BorderBrush}" BorderThickness="{TemplateBinding BorderThickness}" Content="{TemplateBinding Content}" ContentTemplate="{TemplateBinding ContentTemplate}" ContentTemplateSelector="{TemplateBinding ContentTemplateSelector}" ContentTransitions="{TemplateBinding ContentTransitions}" CornerRadius="{TemplateBinding CornerRadius}" Padding="{TemplateBinding Padding}" HorizontalContentAlignment="{TemplateBinding HorizontalContentAlignment}" VerticalContentAlignment="{TemplateBinding VerticalContentAlignment}" AutomationProperties.AccessibilityView="Raw" local:AnimatedIcon.State="Normal" xmlns:local="using:Microsoft.UI.Xaml.Controls">
+            <ContentPresenter x:Name="ContentPresenter" Background="{TemplateBinding Background}" Foreground="{TemplateBinding Foreground}" BackgroundSizing="{TemplateBinding BackgroundSizing}" BorderBrush="{TemplateBinding BorderBrush}" BorderThickness="{TemplateBinding BorderThickness}" Content="{TemplateBinding Content}" ContentTemplate="{TemplateBinding ContentTemplate}" ContentTransitions="{TemplateBinding ContentTransitions}" CornerRadius="{TemplateBinding CornerRadius}" Padding="{TemplateBinding Padding}" HorizontalContentAlignment="{TemplateBinding HorizontalContentAlignment}" VerticalContentAlignment="{TemplateBinding VerticalContentAlignment}" AutomationProperties.AccessibilityView="Raw" local:AnimatedIcon.State="Normal" xmlns:local="using:Microsoft.UI.Xaml.Controls">
               <ContentPresenter.BackgroundTransition>
                 <BrushTransition Duration="0:0:0.083" />
               </ContentPresenter.BackgroundTransition>

diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_TimePicker.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_TimePicker.cs
@@ -17,10 +17,32 @@
 namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls
 {
 	[TestClass]
+	[RunsOnUIThread]
 	public class Given_TimePicker
 	{
 		[TestMethod]
-		[RunsOnUIThread]
+		public async Task When_MinuteIncrement_In_Range_Should_Be_Set_Properly()
+		{
+			var timePicker = new TimePicker();
+			Assert.AreEqual(1, timePicker.MinuteIncrement);
+
+			timePicker.MinuteIncrement = 59;
+			Assert.AreEqual(59, timePicker.MinuteIncrement);
+		}
+
+		[TestMethod]
+		public async Task When_MinuteIncrement_Not_In_Range_Should_Throw_And_Keep_OldValue()
+		{
+			var timePicker = new TimePicker();
+			timePicker.MinuteIncrement = 17;
+			Assert.AreEqual(17, timePicker.MinuteIncrement);
+			Assert.ThrowsException<ArgumentOutOfRangeException>(() => timePicker.MinuteIncrement = 60);
+			Assert.AreEqual(17, timePicker.MinuteIncrement);
+			Assert.ThrowsException<ArgumentOutOfRangeException>(() => timePicker.MinuteIncrement = -1);
+			Assert.AreEqual(17, timePicker.MinuteIncrement);
+		}
+
+		[TestMethod]
 		public async Task When_SettingNullTime_ShouldNotCrash()
 		{
 			var timePicker = new TimePicker();
@@ -39,7 +61,6 @@ public async Task When_SettingNullTime_ShouldNotCrash()
 		}
 
 		[TestMethod]
-		[RunsOnUIThread]
 		public async Task When_PM_Opened_And_Closed()
 		{
 			// This tests whether the looping selector does not unexpectedly
@@ -65,7 +86,6 @@ public async Task When_PM_Opened_And_Closed()
 		}
 
 		[TestMethod]
-		[RunsOnUIThread]
 		[UnoWorkItem("https://github.com/unoplatform/uno/issues/15409")]
 		public async Task When_Opened_From_Button_Flyout()
 		{
@@ -104,12 +124,10 @@ public async Task When_Opened_From_Button_Flyout()
 		}
 
 		[TestMethod]
-		[RunsOnUIThread]
 		[UnoWorkItem("https://github.com/unoplatform/uno/issues/15256")]
 		public async Task When_Opened_And_Unloaded_Unloaded_Native() => await When_Opened_And_Unloaded(true);
 
 		[TestMethod]
-		[RunsOnUIThread]
 		[UnoWorkItem("https://github.com/unoplatform/uno/issues/15256")]
 		public async Task When_Opened_And_Unloaded_Managed() => await When_Opened_And_Unloaded(false);
 
@@ -159,12 +177,10 @@ private async Task When_Opened_And_Unloaded(bool useNative)
 
 #if HAS_UNO
 		[TestMethod]
-		[RunsOnUIThread]
 		[UnoWorkItem("https://github.com/unoplatform/uno/issues/15256")]
 		public async Task When_Flyout_Closed_FlyoutBase_Closed_Native() => await When_Flyout_Closed_FlyoutBase_Closed_Invoked(true);
 
 		[TestMethod]
-		[RunsOnUIThread]
 		[UnoWorkItem("https://github.com/unoplatform/uno/issues/15256")]
 		public async Task When_Flyout_Closed_FlyoutBase_Closed_Managed() => await When_Flyout_Closed_FlyoutBase_Closed_Invoked(false);
 
@@ -204,7 +220,6 @@ private async Task When_Flyout_Closed_FlyoutBase_Closed_Invoked(bool useNative)
 
 #if __IOS__
 		[TestMethod]
-		[RunsOnUIThread]
 		[UnoWorkItem("https://github.com/unoplatform/uno/issues/15263")]
 		public async Task When_App_Theme_Dark_Native_Flyout_Theme()
 		{
@@ -213,7 +228,6 @@ public async Task When_App_Theme_Dark_Native_Flyout_Theme()
 		}
 
 		[TestMethod]
-		[RunsOnUIThread]
 		[UnoWorkItem("https://github.com/unoplatform/uno/issues/15263")]
 		public async Task When_App_Theme_Light_Native_Flyout_Theme() => await When_Native_Flyout_Theme(UIKit.UIUserInterfaceStyle.Light);
 

diff --git a/src/Uno.UI/UI/Xaml/Controls/TimePicker/TimePicker.Properties.cs b/src/Uno.UI/UI/Xaml/Controls/TimePicker/TimePicker.Properties.cs
@@ -110,23 +110,8 @@ public int MinuteIncrement
 			new FrameworkPropertyMetadata(
 				defaultValue: 1,
 				options: FrameworkPropertyMetadataOptions.None,
-				propertyChangedCallback: (s, e) => ((TimePicker)s)?.OnMinuteIncrementChanged((int)e.OldValue, (int)e.NewValue),
-				coerceValueCallback: (s, e, _) =>
-				{
-					var value = (int)e;
-
-					if (value < 1)
-					{
-						return 1;
-					}
-
-					if (value > 30)
-					{
-						return 30;
-					}
-
-					return value;
-				}));
+				propertyChangedCallback: (s, e) => ((TimePicker)s)?.OnMinuteIncrementChanged((int)e.OldValue, (int)e.NewValue)
+			));
 
 	/// <summary>
 	/// Gets or sets the time currently selected in the time picker.