Fixed View.KeyDown/Up event handling

Terminal.Gui/View/View.Keyboard.cs

@@ -259,9 +259,9 @@ public partial class View // Keyboard APIs
     ///     </para>
     ///     <para>See <see href="../docs/keyboard.md">for an overview of Terminal.Gui keyboard APIs.</see></para>
     /// </remarks>
-    /// <param name="keyEvent"></param>
+    /// <param name="key"></param>
     /// <returns><see langword="true"/> if the event was handled.</returns>
-    public bool NewKeyDownEvent (Key keyEvent)
+    public bool NewKeyDownEvent (Key key)
     {
         if (!Enabled)
         {
@@ -270,28 +270,27 @@ public partial class View // Keyboard APIs
 
         // By default the KeyBindingScope is View
 
-        if (Focused?.NewKeyDownEvent (keyEvent) == true)
+        if (Focused?.NewKeyDownEvent (key) == true)
         {
             return true;
         }
 
-        // Before (fire the cancellable event)
-        if (OnKeyDown (keyEvent))
+        if (RaiseKeyDown (key) || key.Handled)
         {
             return true;
         }
 
         // During (this is what can be cancelled)
-        InvokingKeyBindings?.Invoke (this, keyEvent);
+        InvokingKeyBindings?.Invoke (this, key);
 
-        if (keyEvent.Handled)
+        if (key.Handled)
         {
             return true;
         }
 
         // TODO: NewKeyDownEvent returns bool. It should be bool? so state of InvokeCommand can be reflected up stack
 
-        bool? handled = OnInvokingKeyBindings (keyEvent, KeyBindingScope.HotKey | KeyBindingScope.Focused);
+        bool? handled = OnInvokingKeyBindings (key, KeyBindingScope.HotKey | KeyBindingScope.Focused);
 
         if (handled is { } && (bool)handled)
         {
@@ -302,14 +301,28 @@ public partial class View // Keyboard APIs
         // TODO: But I've moved it outside of the v-function to test something.
         // After (fire the cancellable event)
         // fire event
-        ProcessKeyDown?.Invoke (this, keyEvent);
+        ProcessKeyDown?.Invoke (this, key);
 
-        if (!keyEvent.Handled && OnProcessKeyDown (keyEvent))
+        if (!key.Handled && OnProcessKeyDown (key))
         {
             return true;
         }
 
-        return keyEvent.Handled;
+        return key.Handled;
+    }
+
+    private bool RaiseKeyDown (Key key)
+    {
+        // Before (fire the cancellable event)
+        if (OnKeyDown (key) || key.Handled)
+        {
+            return true;
+        }
+
+        // fire event
+        KeyDown?.Invoke (this, key);
+
+        return key.Handled;
     }
 
     /// <summary>
@@ -330,10 +343,7 @@ public partial class View // Keyboard APIs
     /// </remarks>
     public virtual bool OnKeyDown (Key keyEvent)
     {
-        // fire event
-        KeyDown?.Invoke (this, keyEvent);
-
-        return keyEvent.Handled;
+        return false;
     }
 
     /// <summary>
@@ -421,35 +431,38 @@ public partial class View // Keyboard APIs
     ///     </para>
     ///     <para>See <see href="../docs/keyboard.md">for an overview of Terminal.Gui keyboard APIs.</see></para>
     /// </remarks>
-    /// <param name="keyEvent"></param>
+    /// <param name="key"></param>
     /// <returns><see langword="true"/> if the event was handled.</returns>
-    public bool NewKeyUpEvent (Key keyEvent)
+    public bool NewKeyUpEvent (Key key)
     {
         if (!Enabled)
         {
             return false;
         }
 
-        if (Focused?.NewKeyUpEvent (keyEvent) == true)
+        if (RaiseKeyUp (key) || key.Handled)
         {
             return true;
         }
 
+        return false;
+    }
+
+    private bool RaiseKeyUp (Key key)
+    {
         // Before (fire the cancellable event)
-        if (OnKeyUp (keyEvent))
+        if (OnKeyUp (key) || key.Handled)
         {
             return true;
         }
 
-        // During (this is what can be cancelled)
-        // TODO: Until there's a clear use-case, we will not define 'during' event (e.g. OnDuringKeyUp). 
-
-        // After (fire the cancellable event InvokingKeyBindings)
-        // TODO: Until there's a clear use-case, we will not define an 'after' event (e.g. OnAfterKeyUp). 
+        // fire event
+        KeyUp?.Invoke (this, key);
 
-        return false;
+        return key.Handled;
     }
 
+
     /// <summary>Method invoked when a key is released. This method is called from <see cref="NewKeyUpEvent"/>.</summary>
     /// <param name="keyEvent">Contains the details about the key that produced the event.</param>
     /// <returns>
@@ -467,14 +480,6 @@ public partial class View // Keyboard APIs
     /// </remarks>
     public virtual bool OnKeyUp (Key keyEvent)
     {
-        // fire event
-        KeyUp?.Invoke (this, keyEvent);
-
-        if (keyEvent.Handled)
-        {
-            return true;
-        }
-
         return false;
     }
 

Terminal.Gui/Views/ListView.cs

@@ -806,7 +806,7 @@ public class ListView : View, IDesignable
     public override bool OnKeyDown (Key a)
     {
         // Enable user to find & select an item by typing text
-        if (CollectionNavigatorBase.IsCompatibleKey (a))
+        if (CollectionNavigatorBase.IsCompatibleKey (a) && (!AllowsMarking && a == Key.Space))
         {
             int? newItem = KeystrokeNavigator?.GetNextMatchingItem (SelectedItem, (char)a);
 

Terminal.Gui/Views/TableView/TableView.cs

@@ -988,7 +988,7 @@ public class TableView : View
     }
 
     /// <inheritdoc/>
-    public override bool OnKeyDown (Key keyEvent)
+    public override bool OnKeyDown (Key key)
     {
         if (TableIsNullOrInvisible ())
         {
@@ -998,12 +998,14 @@ public class TableView : View
         if (CollectionNavigator != null
             && HasFocus
             && Table.Rows != 0
-            && CollectionNavigatorBase.IsCompatibleKey (keyEvent)
-            && !keyEvent.KeyCode.HasFlag (KeyCode.CtrlMask)
-            && !keyEvent.KeyCode.HasFlag (KeyCode.AltMask)
-            && Rune.IsLetterOrDigit ((Rune)keyEvent))
-        {
-            return CycleToNextTableEntryBeginningWith (keyEvent);
+            && key != KeyBindings.GetKeyFromCommands (Command.Accept)
+            && key != CellActivationKey
+            && CollectionNavigatorBase.IsCompatibleKey (key)
+            && !key.KeyCode.HasFlag (KeyCode.CtrlMask)
+            && !key.KeyCode.HasFlag (KeyCode.AltMask)
+            && Rune.IsLetterOrDigit ((Rune)key))
+        {
+            return CycleToNextTableEntryBeginningWith (key);
         }
 
         return false;

Terminal.Gui/Views/TextView.cs

@@ -3688,7 +3688,7 @@ public class TextView : View
             return true;
         }
 
-        return base.OnKeyUp (key);
+        return false;
     }
 
     /// <inheritdoc/>

UnitTests/View/KeyboardEventTests.cs

@@ -108,7 +108,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
                             Assert.Equal (alt, e.IsAlt);
                             Assert.Equal (control, e.IsCtrl);
                             Assert.False (keyDown);
-                            Assert.False (view.OnKeyDownContinued);
+                            Assert.True (view.OnKeyDownCalled);
                             keyDown = true;
                         };
         view.ProcessKeyDown += (s, e) => { keyPressed = true; };
@@ -120,7 +120,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
                           Assert.Equal (alt, e.IsAlt);
                           Assert.Equal (control, e.IsCtrl);
                           Assert.False (keyUp);
-                          Assert.False (view.OnKeyUpContinued);
+                          Assert.True (view.OnKeyUpCalled);
                           keyUp = true;
                       };
 
@@ -138,7 +138,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
                                   )
                              );
         Assert.True (keyPressed);
-        Assert.True (view.OnKeyDownContinued);
+        Assert.True (view.OnKeyDownCalled);
         Assert.True (view.OnKeyPressedContinued);
 
         view.NewKeyUpEvent (
@@ -150,7 +150,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
                                 )
                            );
         Assert.True (keyUp);
-        Assert.True (view.OnKeyUpContinued);
+        Assert.True (view.OnKeyUpCalled);
     }
 
     [Fact]
@@ -215,7 +215,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
                         {
                             Assert.Equal (KeyCode.A, e.KeyCode);
                             Assert.False (keyDown);
-                            Assert.False (view.OnKeyDownContinued);
+                            Assert.True (view.OnKeyDownCalled);
                             e.Handled = false;
                             keyDown = true;
                         };
@@ -243,7 +243,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
         Assert.True (invokingKeyBindings);
         Assert.False (keyPressed);
 
-        Assert.True (view.OnKeyDownContinued);
+        Assert.True (view.OnKeyDownCalled);
         Assert.False (view.OnInvokingKeyBindingsContinued);
         Assert.False (view.OnKeyPressedContinued);
     }
@@ -263,7 +263,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
                         {
                             Assert.Equal (KeyCode.A, e.KeyCode);
                             Assert.False (keyDown);
-                            Assert.False (view.OnKeyDownContinued);
+                            Assert.True (view.OnKeyDownCalled);
                             e.Handled = true;
                             keyDown = true;
                         };
@@ -291,7 +291,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
         Assert.False (invokingKeyBindings);
         Assert.False (keyPressed);
 
-        Assert.False (view.OnKeyDownContinued);
+        Assert.True (view.OnKeyDownCalled);
         Assert.False (view.OnInvokingKeyBindingsContinued);
         Assert.False (view.OnKeyPressedContinued);
     }
@@ -352,7 +352,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
                         {
                             Assert.Equal (KeyCode.A, e.KeyCode);
                             Assert.False (keyDown);
-                            Assert.False (view.OnKeyDownContinued);
+                            Assert.True (view.OnKeyDownCalled);
                             e.Handled = false;
                             keyDown = true;
                         };
@@ -380,7 +380,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
         Assert.True (invokingKeyBindings);
         Assert.True (keyPressed);
 
-        Assert.True (view.OnKeyDownContinued);
+        Assert.True (view.OnKeyDownCalled);
         Assert.True (view.OnInvokingKeyBindingsContinued);
         Assert.False (view.OnKeyPressedContinued);
     }
@@ -406,8 +406,8 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
         view.NewKeyUpEvent (Key.A);
         Assert.True (keyUp);
 
-        Assert.False (view.OnKeyUpContinued);
-        Assert.False (view.OnKeyDownContinued);
+        Assert.True (view.OnKeyUpCalled);
+        Assert.False (view.OnKeyDownCalled);
         Assert.False (view.OnInvokingKeyBindingsContinued);
         Assert.False (view.OnKeyPressedContinued);
     }
@@ -444,9 +444,9 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
         public OnKeyTestView () { CanFocus = true; }
         public bool CancelVirtualMethods { set; private get; }
         public bool OnInvokingKeyBindingsContinued { get; set; }
-        public bool OnKeyDownContinued { get; set; }
+        public bool OnKeyDownCalled { get; set; }
         public bool OnKeyPressedContinued { get; set; }
-        public bool OnKeyUpContinued { get; set; }
+        public bool OnKeyUpCalled { get; set; }
         public override string Text { get; set; }
 
         public override bool? OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope)
@@ -465,24 +465,14 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
 
         public override bool OnKeyDown (Key keyEvent)
         {
-            if (base.OnKeyDown (keyEvent))
-            {
-                return true;
-            }
-
-            OnKeyDownContinued = true;
+            OnKeyDownCalled = true;
 
             return CancelVirtualMethods;
         }
 
         public override bool OnKeyUp (Key keyEvent)
         {
-            if (base.OnKeyUp (keyEvent))
-            {
-                return true;
-            }
-
-            OnKeyUpContinued = true;
+            OnKeyUpCalled = true;
 
             return CancelVirtualMethods;
         }

UnitTests/Views/ListViewTests.cs

@@ -539,10 +539,10 @@ Item 6",
 
         lv.KeyBindings.Add (Key.Space.WithShift, Command.Select, Command.Down);
 
-        Key ev = Key.Space.WithShift;
+        Key key = Key.Space.WithShift;
 
         // view should indicate that it has accepted and consumed the event
-        Assert.True (lv.NewKeyDownEvent (ev));
+        Assert.True (lv.NewKeyDownEvent (key));
 
         // first item should now be selected
         Assert.Equal (0, lv.SelectedItem);
@@ -553,7 +553,7 @@ Item 6",
         Assert.False (lv.Source.IsMarked (2));
 
         // Press key combo again
-        Assert.True (lv.NewKeyDownEvent (ev));
+        Assert.True (lv.NewKeyDownEvent (key));
 
         // second item should now be selected
         Assert.Equal (1, lv.SelectedItem);
@@ -564,21 +564,21 @@ Item 6",
         Assert.False (lv.Source.IsMarked (2));
 
         // Press key combo again
-        Assert.True (lv.NewKeyDownEvent (ev));
+        Assert.True (lv.NewKeyDownEvent (key));
         Assert.Equal (2, lv.SelectedItem);
         Assert.True (lv.Source.IsMarked (0));
         Assert.True (lv.Source.IsMarked (1));
         Assert.False (lv.Source.IsMarked (2));
 
         // Press key combo again
-        Assert.True (lv.NewKeyDownEvent (ev));
+        Assert.True (lv.NewKeyDownEvent (key));
         Assert.Equal (2, lv.SelectedItem); // cannot move down any further
         Assert.True (lv.Source.IsMarked (0));
         Assert.True (lv.Source.IsMarked (1));
         Assert.True (lv.Source.IsMarked (2)); // but can toggle marked
 
         // Press key combo again 
-        Assert.True (lv.NewKeyDownEvent (ev));
+        Assert.True (lv.NewKeyDownEvent (key));
         Assert.Equal (2, lv.SelectedItem); // cannot move down any further
         Assert.True (lv.Source.IsMarked (0));
         Assert.True (lv.Source.IsMarked (1));