FIX: UISelectable's OnSubmit and OnCancel are not triggered when using InputTestFixture with mock keyboard [ISXB-1561]

Assets/Tests/InputSystem/Plugins/MockKeyboardWithUITests.cs

@@ -0,0 +1,108 @@
+using System;
+using System.Collections;
+using NUnit.Framework;
+using UnityEngine;
+using UnityEngine.EventSystems;
+using UnityEngine.InputSystem;
+using UnityEngine.InputSystem.UI;
+using UnityEngine.TestTools;
+using UnityEngine.UI;
+
+[Description("Check https://jira.unity3d.com/browse/ISXB-1561 for more details.")]
+public class MockKeyboardWithUITests : InputTestFixture
+{
+    private Keyboard m_Keyboard;
+    private GameObject m_CanvasGo;
+    private GameObject m_EventSystemGo;
+    private GameObject m_ButtonGo;
+    private InputActionAsset m_InputActionAsset;
+
+    // We only use this simple class as an adapter for a lambda closure that we need for assertion to work below.
+    public class CancellationHandler : MonoBehaviour, ICancelHandler
+    {
+        public Action trigger;
+        public void OnCancel(BaseEventData eventData) => trigger();
+    }
+
+    public override void Setup()
+    {
+        base.Setup();
+
+        // Add a mock keyboard
+        m_Keyboard = InputSystem.AddDevice<Keyboard>();
+
+        // first creating the UI stuff
+        m_CanvasGo = new GameObject("canvas");
+        m_ButtonGo = new GameObject("button", typeof(Button), typeof(CancellationHandler));
+        m_ButtonGo.transform.SetParent(m_CanvasGo.transform, false);
+
+        // Now creating the Input layer
+        m_EventSystemGo = new GameObject("eventSystem", typeof(EventSystem));
+        EventSystem.current.SetSelectedGameObject(m_ButtonGo);
+
+        var inputModule = m_EventSystemGo.AddComponent<InputSystemUIInputModule>();
+
+        m_InputActionAsset = ScriptableObject.CreateInstance<InputActionAsset>();
+        var actionMap = new InputActionMap("UI");
+        var submitAction = actionMap.AddAction("Submit", binding: "Keyboard/enter", type: InputActionType.Button);
+        var cancelAction = actionMap.AddAction("Cancel", binding: "Keyboard/escape", type: InputActionType.Button);
+        m_InputActionAsset.AddActionMap(actionMap);
+        actionMap.Enable();
+        inputModule.actionsAsset = m_InputActionAsset;
+        inputModule.submit = InputActionReference.Create(submitAction);
+        inputModule.cancel = InputActionReference.Create(cancelAction);
+    }
+
+    public override void TearDown()
+    {
+        GameObject.Destroy(m_EventSystemGo);
+        GameObject.Destroy(m_CanvasGo);
+
+        GameObject.Destroy(m_InputActionAsset);
+
+        InputSystem.RemoveDevice(m_Keyboard);
+
+        base.TearDown();
+    }
+
+    private static bool[] FrameWaitingOptions = { true, false };
+
+    [UnityTest]
+    public IEnumerator MockKeyboard_PressEnter_TriggersButtonOnClick([ValueSource(nameof(FrameWaitingOptions))] bool waitFrame)
+    {
+        bool invokedListener = false;
+
+        m_ButtonGo.GetComponent<Button>().onClick.AddListener(() => invokedListener = true);
+
+        Press(m_Keyboard.enterKey);
+
+        // We'd like to test both options to be safe with rapid actions that happened within a single frame
+        if (waitFrame)
+            yield return null;
+
+        Release(m_Keyboard.enterKey);
+
+        yield return null;
+
+        Assert.IsTrue(invokedListener, "The button should have been clicked here.");
+    }
+
+    [UnityTest]
+    public IEnumerator MockKeyboard_PressEscape_TriggersCancelHandler([ValueSource(nameof(FrameWaitingOptions))] bool waitFrame)
+    {
+        bool invokedListener = false;
+        m_ButtonGo.GetComponent<CancellationHandler>().trigger = () => invokedListener = true;
+
+        Press(m_Keyboard.escapeKey);
+
+        // We'd like to test both options to be safe with rapid actions that happened within a single frame
+        if (waitFrame)
+            yield return null;
+
+        Release(m_Keyboard.escapeKey);
+
+        yield return null;
+
+        Assert.IsTrue(invokedListener, "The cancel event should have been raised here.");
+    }
+}

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -9,7 +9,6 @@ Due to package verification, the latest version below is the unpublished version
 however, it has to be formatted properly to pass verification tests.
 
 ## [Unreleased] - yyyy-mm-dd
-- Fixed InputControl picker not updating correctly when the Input Actions Window was dirty. [ISXB-1221](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1221)
 
 ### Changed
 - Expanded `RebindingUISample` to include a "game mode" state and a "menu state" to be more similar to a real game. Also added action-performed indicators (`InputActionIndicator`) illustrating when actions get triggered.
@@ -20,6 +19,8 @@ however, it has to be formatted properly to pass verification tests.
 - Added a new Monobehavior `InputActionLabel` to rebinding sample to allow dynamic text showing relevant binding for an `InputAction`.
 
 ### Fixed
+- Fixed InputControl picker not updating correctly when the Input Actions Window was dirty. [ISXB-1221](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1221)
+- Fixed UISelectable's OnSubmit and OnCancel not being triggered when using InputTestFixture with mock keyboard [ISXB-1561](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1561)
 - Fixed an issue in `RebindingUISample` that fired actions bound to the same control as the target control in a rebinding process. ISXB-1524.
 - Fixed an issue in `RebindingUISample` preventing UI navigation without Keyboard and Mouse present.
 - Fixed an issue in `RebindActionUI` which resulted in active binding not being shown after a scene reload. ISXB-1588.

Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs

@@ -914,11 +914,15 @@ internal void ProcessNavigation(ref NavigationModel navigationState)
 
                 data.device = m_SubmitCancelState.device;
 
-                if (cancelAction != null && cancelAction.WasPerformedThisDynamicUpdate())
+                if (cancelAction != null && m_NavigationState.wasCancelButtonReleased)
                     ExecuteEvents.Execute(eventSystem.currentSelectedGameObject, data, ExecuteEvents.cancelHandler);
-                if (!data.used && submitAction != null && submitAction.WasPerformedThisDynamicUpdate())
+                if (!data.used && submitAction != null && m_NavigationState.wasSubmitButtonReleased)
                     ExecuteEvents.Execute(eventSystem.currentSelectedGameObject, data, ExecuteEvents.submitHandler);
             }
+
+            // We clear the following flags here to mark that we've processed them already
+            m_NavigationState.wasSubmitButtonReleased = false;
+            m_NavigationState.wasCancelButtonReleased = false;
         }
 
         private bool IsMoveAllowed(AxisEventData eventData)
@@ -1403,7 +1407,7 @@ public InputActionReference move
         public InputActionReference submit
         {
             get => m_SubmitAction;
-            set => SwapAction(ref m_SubmitAction, value, m_ActionsHooked, m_OnSubmitCancelDelegate);
+            set => SwapAction(ref m_SubmitAction, value, m_ActionsHooked, m_OnSubmitDelegate);
         }
 
         /// <summary>
@@ -1443,7 +1447,7 @@ public InputActionReference submit
         public InputActionReference cancel
         {
             get => m_CancelAction;
-            set => SwapAction(ref m_CancelAction, value, m_ActionsHooked, m_OnSubmitCancelDelegate);
+            set => SwapAction(ref m_CancelAction, value, m_ActionsHooked, m_OnCancelDelegate);
         }
 
         /// <summary>
@@ -2287,11 +2291,6 @@ private void OnMoveCallback(InputAction.CallbackContext context)
             m_NavigationState.device = context.control.device;
         }
 
-        private void OnSubmitCancelCallback(InputAction.CallbackContext context)
-        {
-            m_SubmitCancelState.device = context.control.device;
-        }
-
         private void OnTrackedDeviceOrientationCallback(InputAction.CallbackContext context)
         {
             var index = GetPointerStateIndexFor(ref context);
@@ -2318,6 +2317,26 @@ private void OnTrackedDevicePositionCallback(InputAction.CallbackContext context
 #endif
         }
 
+        private void OnSubmit(InputAction.CallbackContext context)
+        {
+            m_SubmitCancelState.device = context.control.device;
+
+            if (!context.ReadValueAsButton())
+            {
+                m_NavigationState.wasSubmitButtonReleased = true;
+            }
+        }
+
+        private void OnCancel(InputAction.CallbackContext context)
+        {
+            m_SubmitCancelState.device = context.control.device;
+
+            if (!context.ReadValueAsButton())
+            {
+                m_NavigationState.wasCancelButtonReleased = true;
+            }
+        }
+
         private void OnControlsChanged(object obj)
         {
             m_NeedToPurgeStalePointers = true;
@@ -2509,12 +2528,14 @@ private void HookActions()
                 m_OnScrollWheelDelegate = OnScrollCallback;
             if (m_OnMoveDelegate == null)
                 m_OnMoveDelegate = OnMoveCallback;
-            if (m_OnSubmitCancelDelegate == null)
-                m_OnSubmitCancelDelegate = OnSubmitCancelCallback;
             if (m_OnTrackedDeviceOrientationDelegate == null)
                 m_OnTrackedDeviceOrientationDelegate = OnTrackedDeviceOrientationCallback;
             if (m_OnTrackedDevicePositionDelegate == null)
                 m_OnTrackedDevicePositionDelegate = OnTrackedDevicePositionCallback;
+            if (m_OnSubmitDelegate == null)
+                m_OnSubmitDelegate = OnSubmit;
+            if (m_OnCancelDelegate == null)
+                m_OnCancelDelegate = OnCancel;
 
             SetActionCallbacks(true);
         }
@@ -2532,14 +2553,14 @@ private void SetActionCallbacks(bool install)
             m_ActionsHooked = install;
             SetActionCallback(m_PointAction, m_OnPointDelegate, install);
             SetActionCallback(m_MoveAction, m_OnMoveDelegate, install);
-            SetActionCallback(m_SubmitAction, m_OnSubmitCancelDelegate, install);
-            SetActionCallback(m_CancelAction, m_OnSubmitCancelDelegate, install);
             SetActionCallback(m_LeftClickAction, m_OnLeftClickDelegate, install);
             SetActionCallback(m_RightClickAction, m_OnRightClickDelegate, install);
             SetActionCallback(m_MiddleClickAction, m_OnMiddleClickDelegate, install);
             SetActionCallback(m_ScrollWheelAction, m_OnScrollWheelDelegate, install);
             SetActionCallback(m_TrackedDeviceOrientationAction, m_OnTrackedDeviceOrientationDelegate, install);
             SetActionCallback(m_TrackedDevicePositionAction, m_OnTrackedDevicePositionDelegate, install);
+            SetActionCallback(m_SubmitAction, m_OnSubmitDelegate, install);
+            SetActionCallback(m_CancelAction, m_OnCancelDelegate, install);
         }
 
         private static void SetActionCallback(InputActionReference actionReference, Action<InputAction.CallbackContext> callback, bool install)
@@ -2649,13 +2670,14 @@ private struct InputActionReferenceState
 
         private Action<InputAction.CallbackContext> m_OnPointDelegate;
         private Action<InputAction.CallbackContext> m_OnMoveDelegate;
-        private Action<InputAction.CallbackContext> m_OnSubmitCancelDelegate;
         private Action<InputAction.CallbackContext> m_OnLeftClickDelegate;
         private Action<InputAction.CallbackContext> m_OnRightClickDelegate;
         private Action<InputAction.CallbackContext> m_OnMiddleClickDelegate;
         private Action<InputAction.CallbackContext> m_OnScrollWheelDelegate;
         private Action<InputAction.CallbackContext> m_OnTrackedDevicePositionDelegate;
         private Action<InputAction.CallbackContext> m_OnTrackedDeviceOrientationDelegate;
+        private Action<InputAction.CallbackContext> m_OnSubmitDelegate;
+        private Action<InputAction.CallbackContext> m_OnCancelDelegate;
         private Action<object> m_OnControlsChangedDelegate;
 
         // Pointer-type input (also tracking-type).

Packages/com.unity.inputsystem/InputSystem/Plugins/UI/NavigationModel.cs

@@ -11,10 +11,14 @@ internal struct NavigationModel
         public float lastMoveTime;
         public AxisEventData eventData;
         public InputDevice device;
+        public bool wasSubmitButtonReleased;
+        public bool wasCancelButtonReleased;
 
         public void Reset()
         {
             move = Vector2.zero;
+            wasCancelButtonReleased = false;
+            wasSubmitButtonReleased = false;
         }
     }