Improve VisualState order and prevent sticky Focused visual state

src/Controls/samples/Controls.Sample.Sandbox/MainPage.xaml.cs

@@ -6,4 +6,4 @@ public MainPage()
 	{
 		InitializeComponent();
 	}
-}
+}

src/Controls/src/Core/VisualElement/VisualElement.cs

@@ -20,7 +20,6 @@ namespace Microsoft.Maui.Controls
 	/// <remarks>
 	/// The base class for most .NET MAUI on-screen elements. Provides most properties, events, and methods for presenting an item on screen.
 	/// </remarks>
-
 	[DebuggerDisplay("{GetDebuggerDisplay(), nq}")]
 	public partial class VisualElement : NavigableElement, IAnimatable, IVisualElementController, IResourcesProvider, IStyleElement, IFlowDirectionController, IPropertyPropagationController, IVisualController, IWindowController, IView, IControlsVisualElement
 	{
@@ -1607,26 +1606,43 @@ private protected void SetPointerOver(bool value, bool callChangeVisualState = t
 		/// </summary>
 		protected internal virtual void ChangeVisualState()
 		{
+			// A disabled control should never be in a focused state as part of the feature
+			// of being disabled is that it cannot receive focus. If it was in focus, then
+			// it has to go out of focus.
+			var shouldFocus = IsFocused && IsEnabled;
+
+			// If the control cannot have focus, make sure it appears unfocused by moving to
+			// the Unfocused state.
+			if (!shouldFocus)
+			{
+				VisualStateManager.GoToState(this, VisualStateManager.FocusStates.Unfocused);
+			}
+
+			// Set the Disabled or Normal states depending on the value of IsEnabled and
+			// IsPointerOver. We set the PointerOver state later, after the Focused state.
 			if (!IsEnabled)
 			{
 				VisualStateManager.GoToState(this, VisualStateManager.CommonStates.Disabled);
 			}
-			else if (IsPointerOver)
+			else if (!IsPointerOver)
 			{
-				VisualStateManager.GoToState(this, VisualStateManager.CommonStates.PointerOver);
+				VisualStateManager.GoToState(this, VisualStateManager.CommonStates.Normal);
 			}
-			else
+
+			// Go to the Focus state after the Normal state, so that the Focus state can
+			// override the Normal state's properties if a control is both focused and
+			// hovered.
+			if (shouldFocus)
 			{
-				VisualStateManager.GoToState(this, VisualStateManager.CommonStates.Normal);
+				VisualStateManager.GoToState(this, VisualStateManager.FocusStates.Focused);
 			}
 
-			if (IsEnabled)
+			// The PointerOver state is applied last so that it can override all the states. Even
+			// though this state is separate here, it should still be part of the CommonStates
+			// visual state group.
+			if (IsPointerOver)
 			{
-				// 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);
+				VisualStateManager.GoToState(this, VisualStateManager.CommonStates.PointerOver);
 			}
 		}
 

src/Controls/src/Core/VisualStateManager.cs

@@ -15,10 +15,16 @@ public class CommonStates
 		{
 			public const string Normal = "Normal";
 			public const string Disabled = "Disabled";
-			public const string Focused = "Focused";
+			public const string Focused = FocusStates.Focused;
 			public const string Selected = "Selected";
 			public const string PointerOver = "PointerOver";
-			internal const string Unfocused = "Unfocused";
+		}
+
+		// TODO: .NET 10 - make public
+		internal class FocusStates
+		{
+			public const string Focused = "Focused";
+			public const string Unfocused = "Unfocused";
 		}
 
 		/// <summary>Bindable property for attached property <c>VisualStateGroups</c>.</summary>

src/Controls/tests/TestCases.HostApp/Issues/Issue19752.xaml

@@ -0,0 +1,67 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<ContentPage 
+  xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
+  xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+  x:Class="Maui.Controls.Sample.Issues.Issue19752"
+  xmlns:cv1="clr-namespace:Maui.Controls.Sample"
+  xmlns:local="clr-namespace:Maui.Controls.Sample.Issues"
+  x:Name="ThisMainPage"
+  Title="Main Page">
+
+  <VerticalStackLayout Padding="20,120" Spacing="20">
+    <VerticalStackLayout.Resources>
+      <Style TargetType="Button">
+        <Setter Property="VisualStateManager.VisualStateGroups">
+          <VisualStateGroupList>
+            <VisualStateGroup x:Name="CommonStates">
+              <VisualState x:Name="Normal">
+                <VisualState.Setters>
+                  <Setter Property="Text" Value="Normal"/>
+                </VisualState.Setters>
+              </VisualState>
+              <VisualState x:Name="PointerOver">
+                <VisualState.Setters>
+                  <Setter Property="Text" Value="PointerOver"/>
+                </VisualState.Setters>
+              </VisualState>
+              <VisualState x:Name="Pressed">
+                <VisualState.Setters>
+                  <Setter Property="Text" Value="Pressed"/>
+                </VisualState.Setters>
+              </VisualState>
+              <VisualState x:Name="Disabled">
+                <VisualState.Setters>
+                  <Setter Property="Text" Value="Disabled"/>
+                </VisualState.Setters>
+              </VisualState>
+            </VisualStateGroup>
+            <VisualStateGroup x:Name="FocusStates">
+              <VisualState x:Name="Focused">
+                <VisualState.Setters>
+                  <Setter Property="Text" Value="Focused"/>
+                  <Setter Property="BorderColor" Value="Red"/>
+                  <Setter Property="Margin" Value="20,0"/>
+                </VisualState.Setters>
+              </VisualState>
+              <VisualState x:Name="Unfocused">
+                <VisualState.Setters>
+                  <Setter Property="Text" Value="Unfocused"/>
+                  <Setter Property="BorderColor" Value="Lime"/>
+                  <Setter Property="Margin" Value="0"/>
+                </VisualState.Setters>
+              </VisualState>
+            </VisualStateGroup>
+          </VisualStateGroupList>
+        </Setter>
+      </Style>
+    </VerticalStackLayout.Resources>
+
+    <Button x:Name="button1" AutomationId="button1" Clicked="OnButtonClicked" />
+
+    <Button x:Name="button2" AutomationId="button2" Clicked="OnButtonClicked" IsEnabled="False" />
+
+    <Button x:Name="button3" AutomationId="button3" Clicked="OnButtonClicked" />
+
+  </VerticalStackLayout>
+
+</ContentPage>

src/Controls/tests/TestCases.HostApp/Issues/Issue19752.xaml.cs

@@ -0,0 +1,29 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.ComponentModel;
+using System.Runtime.CompilerServices;
+using System.Windows.Input;
+using System.Collections.Specialized;
+
+namespace Maui.Controls.Sample.Issues
+{
+	[XamlCompilation(XamlCompilationOptions.Compile)]
+	[Issue(IssueTracker.Github, 19752, "Button does not behave properly when pointer hovers over the button because it's in focused state.")]
+	public partial class Issue19752
+	{
+		public Issue19752()
+		{
+			InitializeComponent();
+		}
+
+		private void OnButtonClicked(object sender, EventArgs e)
+		{
+			// this code just enables all the buttons and disables the current one
+			// except for the first button which is always enabled
+
+			button2.IsEnabled = sender != button2;
+			button3.IsEnabled = sender != button3;
+		}
+    }
+}

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue19752.cs

@@ -0,0 +1,125 @@
+using NUnit.Framework;
+using UITest.Appium;
+using UITest.Core;
+
+namespace Microsoft.Maui.TestCases.Tests.Issues;
+
+[Category(UITestCategories.Focus)]
+public class Issue19752(TestDevice device) : _IssuesUITest(device)
+{
+	public override string Issue => "Button does not behave properly when pointer hovers over the button because it's in focused state.";
+
+	protected override bool ResetAfterEachTest => true;
+
+	[Test]
+	public void InitialStateAreAllCorrect()
+	{
+		Assert.That(App.FindElement("button1").GetText(), Is.EqualTo("Normal"));
+		Assert.That(App.FindElement("button2").GetText(), Is.EqualTo("Disabled"));
+		Assert.That(App.FindElement("button3").GetText(), Is.EqualTo("Normal"));
+	}
+
+	[Test]
+	public void HoveringOverButtonMovesToPointerOverState()
+	{
+		App.MoveCursor("button1");
+
+		// when the mouse moves over a button, it gets a state
+		Assert.That(App.FindElement("button1").GetText(), Is.EqualTo("PointerOver"));
+	}
+
+	// TODO: find a way to send actions to appium and then read values simultaneously
+	//	[Test]
+	//	public void PressingButtonMovesToPressedState()
+	//	{
+	//		Task.Run(() =>
+	//		{
+	//#if MACCATALYST
+	//			App.LongPress("button1");
+	//#else
+	//			App.TouchAndHold("button1");
+	//#endif
+	//		});
+
+	//		// pressing and holding the mouse is pressed
+	//		App.WaitForTextToBePresentInElement("button1", "Pressed");
+	//		Assert.That(App.FindElement("button1").GetText(), Is.EqualTo("Pressed"));
+	//	}
+
+	[Test]
+	public void PressingAndReleasingButtonMovesToPointerOverState()
+	{
+		var rectBefore = App.FindElement("button1").GetRect();
+
+		App.Tap("button1");
+
+		// pressing a button sets it to be focused, but the pointer over state is appplied after
+		Assert.That(App.FindElement("button1").GetText(), Is.EqualTo("PointerOver"));
+
+		// we are shrinking the focused button a bit
+		var rectAfter = App.FindElement("button1").GetRect();
+		Assert.That(rectBefore, Is.Not.EqualTo(rectAfter));
+	}
+
+	[Test]
+	public void HoveringOverButtonAndThenMovingOffMovesToNormalState()
+	{
+		var rectBefore = App.FindElement("button1").GetRect();
+
+		App.MoveCursor("button1");
+		App.MoveCursor("button2");
+
+		// hovering over a button and then moving off goes back to the normal state
+		// and does not affect focus
+		Assert.That(App.FindElement("button1").GetText(), Is.EqualTo("Normal"));
+
+		// we are shrinking the focused button a bit, but the button is still not focused
+		var rectAfter = App.FindElement("button1").GetRect();
+		Assert.That(rectBefore, Is.EqualTo(rectAfter));
+	}
+
+	[Test]
+	public void EnablingButtonMovesToNormalState()
+	{
+		App.Tap("button1");
+
+		// enabling a button just switches to the normal state
+		Assert.That(App.FindElement("button2").GetText(), Is.EqualTo("Normal"));
+	}
+
+	[Test]
+	public void DisablingUnfocusedButtonMovesToDisabledState()
+	{
+		var rectBefore = App.FindElement("button2").GetRect();
+
+		App.Tap("button1"); // focus button 1
+		App.Tap("button2"); // move the focus to button 2, but then disable it
+
+		// the button is disabled without a focus chnage as it never had focus
+		Assert.That(App.FindElement("button2").GetText(), Is.EqualTo("Disabled"));
+
+		// we are shrinking the focused button a bit, but the button never had focus
+		var rectAfter = App.FindElement("button2").GetRect();
+		Assert.That(rectBefore, Is.EqualTo(rectAfter));
+
+		// this forces focus to button 3 which is set on top of the normal state
+		Assert.That(App.FindElement("button3").GetText(), Is.EqualTo("Focused"));
+	}
+
+	[Test]
+	public void DisablingFocusedButtonMovesToDisabledState()
+	{
+		var rectBefore = App.FindElement("button3").GetRect();
+
+		App.Tap("button1"); // focus button 1
+		App.Tap("button2"); // move the focus to button 2, but then disable it forcing focus to button 3
+		App.Tap("button3"); // disable the focused button
+
+		// this disables the button, but the unfocus change is applied before all states
+		Assert.That(App.FindElement("button3").GetText(), Is.EqualTo("Disabled"));
+
+		// we are shrinking the focused button a bit, so it should have been unfocused after disabling
+		var rectAfter = App.FindElement("button3").GetRect();
+		Assert.That(rectBefore, Is.EqualTo(rectAfter));
+	}
+}