[net7.0] Reconcile PointerOver, Pressed, and Focused states (#12431)

* Potential fix for #9753 on desktop * Fix test for release disabled button * Fix test for release disabled button * Add more tests; fix issues with "stuck" PointerOver states * Clean up extraneous debug stuff * D'oh Co-authored-by: E.Z. Hart <[email protected]>
dotnet · Jan 5, 2023 · cb7c44c · cb7c44c
1 parent e64dab3
commit cb7c44c
Show file tree

Hide file tree

Showing 10 changed files with 172 additions and 59 deletions.
diff --git a/src/Controls/src/Core/ButtonElement.cs b/src/Controls/src/Core/ButtonElement.cs
@@ -110,13 +110,11 @@ public static void ElementPressed(VisualElement visualElement, IButtonElement Bu
 		/// <param name="ButtonElementManager">The button element implementation to trigger the commands and events on.</param>
 		public static void ElementReleased(VisualElement visualElement, IButtonElement ButtonElementManager)
 		{
-			if (visualElement.IsEnabled == true)
-			{
-				IButtonController buttonController = ButtonElementManager as IButtonController;
-				ButtonElementManager.SetIsPressed(false);
-				visualElement.ChangeVisualStateInternal();
-				ButtonElementManager.PropagateUpReleased();
-			}
+			// Even if the button is disabled, we still want to remove the Pressed state;
+			// the button may have been disabled by the the pressing action
+			ButtonElementManager.SetIsPressed(false);
+			visualElement.ChangeVisualStateInternal();
+			ButtonElementManager.PropagateUpReleased();
 		}
 	}
 }
diff --git a/src/Controls/src/Core/VisualElement.cs b/src/Controls/src/Core/VisualElement.cs
@@ -1098,13 +1098,26 @@ private protected void SetPointerOver(bool value, bool callChangeVisualState = t
 		protected internal virtual void ChangeVisualState()
 		{
 			if (!IsEnabled)
+			{
 				VisualStateManager.GoToState(this, VisualStateManager.CommonStates.Disabled);
+			}
 			else if (IsPointerOver)
+			{
 				VisualStateManager.GoToState(this, VisualStateManager.CommonStates.PointerOver);
-			else if (IsFocused)
-				VisualStateManager.GoToState(this, VisualStateManager.CommonStates.Focused);
+			}
 			else
+			{
 				VisualStateManager.GoToState(this, VisualStateManager.CommonStates.Normal);
+			}
+
+			if (IsEnabled)
+			{
+				// Focus needs to be handled independently; otherwise, if no actual Focus state is supplied
+				// in the control's visual states, the state can end up stuck in PointerOver after the pointer
+				// exits and the control still has focus.
+				VisualStateManager.GoToState(this,
+					IsFocused ? VisualStateManager.CommonStates.Focused : VisualStateManager.CommonStates.Unfocused);
+			}
 		}
 
 		static void OnVisualChanged(BindableObject bindable, object oldValue, object newValue)

diff --git a/src/Controls/src/Core/VisualStateManager.cs b/src/Controls/src/Core/VisualStateManager.cs
@@ -18,6 +18,7 @@ public class CommonStates
 			public const string Focused = "Focused";
 			public const string Selected = "Selected";
 			public const string PointerOver = "PointerOver";
+			internal const string Unfocused = "Unfocused";
 		}
 
 		/// <include file="../../docs/Microsoft.Maui.Controls/VisualStateManager.xml" path="//Member[@MemberName='VisualStateGroupsProperty']/Docs/*" />

diff --git a/src/Controls/tests/Core.UnitTests/ButtonUnitTest.cs b/src/Controls/tests/Core.UnitTests/ButtonUnitTest.cs
@@ -1,5 +1,6 @@
 using System;
 using Xunit;
+using static Microsoft.Maui.Controls.Core.UnitTests.VisualStateTestHelpers;
 
 namespace Microsoft.Maui.Controls.Core.UnitTests
 {
@@ -70,7 +71,10 @@ public void TestReleasedEvent(bool isEnabled)
 
 			((IButtonController)view).SendReleased();
 
-			Assert.True(released == isEnabled ? true : false);
+			// Released should always fire, even if the button is disabled
+			// Otherwise, a press which disables a button will leave it in the
+			// Pressed state forever
+			Assert.True(released);
 		}
 
 		protected override Button CreateSource()
@@ -230,5 +234,20 @@ private void AssertButtonContentLayoutsEqual(Button.ButtonContentLayout layout1,
 			Assert.Equal(layout1.Position, bcl.Position);
 			Assert.Equal(layout1.Spacing, bcl.Spacing);
 		}
+
+		[Fact]
+		public void PressedVisualState()
+		{
+			var vsgList = CreateTestStateGroups();
+			var stateGroup = vsgList[0];
+			var element = new Button();
+			VisualStateManager.SetVisualStateGroups(element, vsgList);
+
+			element.SendPressed();
+			Assert.Equal(stateGroup.CurrentState.Name, PressedStateName);
+
+			element.SendReleased();
+			Assert.NotEqual(stateGroup.CurrentState.Name, PressedStateName);
+		}
 	}
 }
diff --git a/src/Controls/tests/Core.UnitTests/CheckBoxUnitTests.cs b/src/Controls/tests/Core.UnitTests/CheckBoxUnitTests.cs
@@ -3,6 +3,7 @@
 using System.Linq;
 
 using Xunit;
+using static Microsoft.Maui.Controls.Core.UnitTests.VisualStateTestHelpers;
 
 namespace Microsoft.Maui.Controls.Core.UnitTests
 {
@@ -43,6 +44,24 @@ public void TestOnEventNotDoubleFired()
 
 			Assert.False(fired);
 		}
-	}
 
+		[Fact]
+		public void CheckedVisualStates()
+		{
+			var vsgList = CreateTestStateGroups();
+			string checkedStateName = CheckBox.IsCheckedVisualState;
+			var checkedState = new VisualState() { Name = checkedStateName };
+			var stateGroup = vsgList[0];
+			stateGroup.States.Add(checkedState);
+
+			var element = new CheckBox();
+			VisualStateManager.SetVisualStateGroups(element, vsgList);
+
+			element.IsChecked = true;
+			Assert.Equal(checkedStateName, stateGroup.CurrentState.Name);
+
+			element.IsChecked = false;
+			Assert.NotEqual(checkedStateName, stateGroup.CurrentState.Name);
+		}
+	}
 }
diff --git a/src/Controls/tests/Core.UnitTests/ImageButtonUnitTest.cs b/src/Controls/tests/Core.UnitTests/ImageButtonUnitTest.cs
@@ -3,10 +3,10 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Xunit;
+using static Microsoft.Maui.Controls.Core.UnitTests.VisualStateTestHelpers;
 
 namespace Microsoft.Maui.Controls.Core.UnitTests
 {
-
 	public class ImageButtonTests : CommandSourceTests<ImageButton>
 	{
 		[Fact]
@@ -42,7 +42,6 @@ public void TestAspectSizingWithConstrainedWidth()
 			Assert.Equal(5, result.Request.Height);
 		}
 
-
 		[Fact]
 		public void TestAspectFillSizingWithConstrainedHeight()
 		{
@@ -319,7 +318,10 @@ public void TestReleasedEvent(bool isEnabled)
 
 			((IButtonController)view).SendReleased();
 
-			Assert.True(released == isEnabled ? true : false);
+			// Released should always fire, even if the button is disabled
+			// Otherwise, a press which disables a button will leave it in the
+			// Pressed state forever
+			Assert.True(released);
 		}
 
 		protected override ImageButton CreateSource()
@@ -347,7 +349,6 @@ protected override BindableProperty CommandParameterProperty
 			get { return ImageButton.CommandParameterProperty; }
 		}
 
-
 		[Fact]
 		public void TestBindingContextPropagation()
 		{
@@ -418,5 +419,20 @@ public void ButtonClickWhenCommandCanExecuteFalse()
 
 			Assert.False(invoked);
 		}
+
+		[Fact]
+		public void PressedVisualState()
+		{
+			var vsgList = CreateTestStateGroups();
+			var stateGroup = vsgList[0];
+			var element = new ImageButton();
+			VisualStateManager.SetVisualStateGroups(element, vsgList);
+
+			element.SendPressed();
+			Assert.Equal(stateGroup.CurrentState.Name, PressedStateName);
+
+			element.SendReleased();
+			Assert.NotEqual(stateGroup.CurrentState.Name, PressedStateName);
+		}
 	}
 }
diff --git a/src/Controls/tests/Core.UnitTests/SwitchUnitTests.cs b/src/Controls/tests/Core.UnitTests/SwitchUnitTests.cs
@@ -1,8 +1,8 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-
 using Xunit;
+using static Microsoft.Maui.Controls.Core.UnitTests.VisualStateTestHelpers;
 
 namespace Microsoft.Maui.Controls.Core.UnitTests
 {
@@ -151,6 +151,29 @@ public void InitialStateIsNullIfNormalOnOffNotAvailable()
 			var groups1 = VisualStateManager.GetVisualStateGroups(switch1);
 			Assert.Null(groups1[0].CurrentState);
 		}
+
+		[Fact]
+		public void OnOffVisualStates()
+		{
+			var vsgList = VisualStateTestHelpers.CreateTestStateGroups();
+			var stateGroup = vsgList[0];
+			var element = new Switch();
+			VisualStateManager.SetVisualStateGroups(element, vsgList);
+
+			string onStateName = Switch.SwitchOnVisualState;
+			string offStateName = Switch.SwitchOffVisualState;
+			var onState = new VisualState() { Name = onStateName };
+			var offState = new VisualState() { Name = offStateName };
+
+			stateGroup.States.Add(onState);
+			stateGroup.States.Add(offState);
+
+			element.IsToggled = true;
+			Assert.Equal(stateGroup.CurrentState.Name, onStateName);
+
+			element.IsToggled = false;
+			Assert.Equal(stateGroup.CurrentState.Name, offStateName);
+		}
 	}
 
 }
diff --git a/src/Controls/tests/Core.UnitTests/VisualElementTests.cs b/src/Controls/tests/Core.UnitTests/VisualElementTests.cs
@@ -1,5 +1,7 @@
 using Microsoft.Maui.Primitives;
 using Xunit;
+using static Microsoft.Maui.Controls.Core.UnitTests.VisualStateTestHelpers;
+
 namespace Microsoft.Maui.Controls.Core.UnitTests
 {
 	public class VisualElementTests
@@ -68,5 +70,17 @@ public void BindingContextPropagatesToBackground()
 			Assert.Equal(bc1, brush2.BindingContext);
 
 		}
+
+		[Fact]
+		public void FocusedElementGetsFocusedVisualState()
+		{
+			var vsgList = CreateTestStateGroups();
+			var stateGroup = vsgList[0];
+			var element = new Button();
+			VisualStateManager.SetVisualStateGroups(element, vsgList);
+
+			element.SetValue(VisualElement.IsFocusedPropertyKey, true);
+			Assert.Equal(stateGroup.CurrentState.Name, FocusedStateName);
+		}
 	}
 }
diff --git a/src/Controls/tests/Core.UnitTests/VisualStateManagerTests.cs b/src/Controls/tests/Core.UnitTests/VisualStateManagerTests.cs
@@ -6,50 +6,12 @@
 using Microsoft.Maui.Controls.Internals;
 using Microsoft.Maui.Graphics;
 using Xunit;
+using static Microsoft.Maui.Controls.Core.UnitTests.VisualStateTestHelpers;
 
 namespace Microsoft.Maui.Controls.Core.UnitTests
 {
-
 	public class VisualStateManagerTests : IDisposable
 	{
-		const string NormalStateName = "Normal";
-		const string InvalidStateName = "Invalid";
-		const string FocusedStateName = "Focused";
-		const string DisabledStateName = "Disabled";
-		const string CommonStatesName = "CommonStates";
-
-		static VisualStateGroupList CreateTestStateGroups()
-		{
-			var stateGroups = new VisualStateGroupList();
-			var visualStateGroup = new VisualStateGroup { Name = CommonStatesName };
-			var normalState = new VisualState { Name = NormalStateName };
-			var invalidState = new VisualState { Name = InvalidStateName };
-			var focusedState = new VisualState { Name = FocusedStateName };
-			var disabledState = new VisualState { Name = DisabledStateName };
-
-			visualStateGroup.States.Add(normalState);
-			visualStateGroup.States.Add(invalidState);
-			visualStateGroup.States.Add(focusedState);
-			visualStateGroup.States.Add(disabledState);
-
-			stateGroups.Add(visualStateGroup);
-
-			return stateGroups;
-		}
-
-		static VisualStateGroupList CreateStateGroupsWithoutNormalState()
-		{
-			var stateGroups = new VisualStateGroupList();
-			var visualStateGroup = new VisualStateGroup { Name = CommonStatesName };
-			var invalidState = new VisualState { Name = InvalidStateName };
-
-			visualStateGroup.States.Add(invalidState);
-
-			stateGroups.Add(visualStateGroup);
-
-			return stateGroups;
-		}
-
 		[Fact]
 		public void InitialStateIsNormalIfAvailable()
 		{
@@ -182,7 +144,7 @@ public void StateNamesMustBeUniqueWithinGroupListWhenAddingGroup()
 		public void GroupNamesMustBeUniqueWithinGroupList()
 		{
 			IList<VisualStateGroup> vsgs = CreateTestStateGroups();
-			var secondGroup = new VisualStateGroup { Name = CommonStatesName };
+			var secondGroup = new VisualStateGroup { Name = CommonStatesGroupName };
 
 			Assert.Throws<InvalidOperationException>(() => vsgs.Add(secondGroup));
 		}
@@ -234,7 +196,6 @@ public void VerifyVisualStateChanges()
 			label1.SetValue(VisualElement.IsFocusedPropertyKey, false);
 			groups1 = VisualStateManager.GetVisualStateGroups(label1);
 			Assert.Equal(groups1[0].CurrentState.Name, NormalStateName);
-
 		}
 
 		[Fact]
@@ -334,7 +295,7 @@ public void VisualElementGoesToCorrectStateWhenSetterHasTarget()
 		public void CanRemoveAStateAndAddANewStateWithTheSameName()
 		{
 			var stateGroups = new VisualStateGroupList();
-			var visualStateGroup = new VisualStateGroup { Name = CommonStatesName };
+			var visualStateGroup = new VisualStateGroup { Name = CommonStatesGroupName };
 			var normalState = new VisualState { Name = NormalStateName };
 			var invalidState = new VisualState { Name = InvalidStateName };
 
@@ -353,7 +314,7 @@ public void CanRemoveAStateAndAddANewStateWithTheSameName()
 		public void CanRemoveAGroupAndAddANewGroupWithTheSameName()
 		{
 			var stateGroups = new VisualStateGroupList();
-			var visualStateGroup = new VisualStateGroup { Name = CommonStatesName };
+			var visualStateGroup = new VisualStateGroup { Name = CommonStatesGroupName };
 			var secondVisualStateGroup = new VisualStateGroup { Name = "Whatevs" };
 			var normalState = new VisualState { Name = NormalStateName };
 			var invalidState = new VisualState { Name = InvalidStateName };

diff --git a/src/Controls/tests/Core.UnitTests/VisualStateTestHelpers.cs b/src/Controls/tests/Core.UnitTests/VisualStateTestHelpers.cs
@@ -0,0 +1,49 @@
+namespace Microsoft.Maui.Controls.Core.UnitTests
+{
+	public class VisualStateTestHelpers 
+	{
+		public const string NormalStateName = "Normal";
+		public const string PressedStateName = "Pressed";
+		public const string InvalidStateName = "Invalid";
+		public const string UnfocusedStateName = "Unfocused";
+		public const string FocusedStateName = "Focused";
+		public const string DisabledStateName = "Disabled";
+		public const string CommonStatesGroupName = "CommonStates";
+		public const string FocusStatesGroupName = "FocusStates";
+
+		public static VisualStateGroupList CreateTestStateGroups()
+		{
+			var stateGroups = new VisualStateGroupList();
+			var commonStatesGroup = new VisualStateGroup { Name = CommonStatesGroupName };
+			var normalState = new VisualState { Name = NormalStateName };
+			var focusState = new VisualState { Name = FocusedStateName };
+			var pressedState = new VisualState { Name = PressedStateName };
+			var invalidState = new VisualState { Name = InvalidStateName };
+
+			var disabledState = new VisualState { Name = DisabledStateName };
+
+			commonStatesGroup.States.Add(normalState);
+			commonStatesGroup.States.Add(pressedState);
+			commonStatesGroup.States.Add(invalidState);
+			commonStatesGroup.States.Add(focusState);
+			commonStatesGroup.States.Add(disabledState);
+
+			stateGroups.Add(commonStatesGroup);
+
+			return stateGroups;
+		}
+
+		public static VisualStateGroupList CreateStateGroupsWithoutNormalState()
+		{
+			var stateGroups = new VisualStateGroupList();
+			var visualStateGroup = new VisualStateGroup { Name = CommonStatesGroupName };
+			var invalidState = new VisualState { Name = InvalidStateName };
+
+			visualStateGroup.States.Add(invalidState);
+
+			stateGroups.Add(visualStateGroup);
+
+			return stateGroups;
+		}
+	}
+}