Fixed TableView key handling

gui-cs · Oct 13, 2024 · 61be061 · 61be061
1 parent 8cb0c84
commit 61be061
Show file tree

Hide file tree

Showing 22 changed files with 159 additions and 137 deletions.
diff --git a/Terminal.Gui/View/View.Keyboard.cs b/Terminal.Gui/View/View.Keyboard.cs
@@ -268,63 +268,84 @@ public bool NewKeyDownEvent (Key key)
             return false;
         }
 
-        // By default the KeyBindingScope is View
-
+        // If there's a Focused subview, give it a chance (this recurses down the hierarchy)
         if (Focused?.NewKeyDownEvent (key) == true)
         {
             return true;
         }
 
+        // Before (fire the cancellable event)
         if (RaiseKeyDown (key) || key.Handled)
         {
             return true;
         }
 
         // During (this is what can be cancelled)
-        InvokingKeyBindings?.Invoke (this, key);
 
-        if (key.Handled)
+        if (RaiseInvokingKeyBindings (key) || key.Handled)
         {
             return true;
         }
 
-        // TODO: NewKeyDownEvent returns bool. It should be bool? so state of InvokeCommand can be reflected up stack
-
-        bool? handled = OnInvokingKeyBindings (key, KeyBindingScope.HotKey | KeyBindingScope.Focused);
-
-        if (handled is { } && (bool)handled)
+        if (RaiseProcessKeyDown(key) || key.Handled)
         {
             return true;
         }
 
-        // TODO: The below is not right. OnXXX handlers are supposed to fire the events.
-        // TODO: But I've moved it outside of the v-function to test something.
-        // After (fire the cancellable event)
-        // fire event
-        ProcessKeyDown?.Invoke (this, key);
+        return key.Handled;
 
-        if (!key.Handled && OnProcessKeyDown (key))
+        bool RaiseKeyDown (Key key)
         {
-            return true;
-        }
+            // Before (fire the cancellable event)
+            if (OnKeyDown (key) || key.Handled)
+            {
+                return true;
+            }
 
-        return key.Handled;
-    }
+            // fire event
+            KeyDown?.Invoke (this, key);
 
-    private bool RaiseKeyDown (Key key)
-    {
-        // Before (fire the cancellable event)
-        if (OnKeyDown (key) || key.Handled)
+            return key.Handled;
+        }
+
+        bool RaiseInvokingKeyBindings (Key key)
         {
-            return true;
+            // BUGBUG: The proper pattern is for the v-method (OnInvokingKeyBindings) to be called first, then the event
+            InvokingKeyBindings?.Invoke (this, key);
+
+            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 (key, KeyBindingScope.HotKey | KeyBindingScope.Focused);
+
+            if (handled is { } && (bool)handled)
+            {
+                return true;
+            }
+
+            return false;
         }
 
-        // fire event
-        KeyDown?.Invoke (this, key);
+        bool RaiseProcessKeyDown (Key key)
+        {
+            // BUGBUG: The proper pattern is for the v-method (OnProcessKeyDown) to be called first, then the event
+            ProcessKeyDown?.Invoke (this, key);
 
-        return key.Handled;
+            if (!key.Handled && OnProcessKeyDown (key))
+            {
+                return true;
+            }
+
+            return false;
+        }
     }
 
+
+
     /// <summary>
     ///     Low-level API called when the user presses a key, allowing a view to pre-process the key down event. This is
     ///     called from <see cref="NewKeyDownEvent"/> before <see cref="OnInvokingKeyBindings"/>.
@@ -341,7 +362,7 @@ private bool RaiseKeyDown (Key key)
     ///     </para>
     ///     <para>Fires the <see cref="KeyDown"/> event.</para>
     /// </remarks>
-    public virtual bool OnKeyDown (Key keyEvent)
+    protected virtual bool OnKeyDown (Key keyEvent)
     {
         return false;
     }
@@ -384,9 +405,8 @@ public virtual bool OnKeyDown (Key keyEvent)
     ///         KeyUp events.
     ///     </para>
     /// </remarks>
-    public virtual bool OnProcessKeyDown (Key keyEvent)
+    protected virtual bool OnProcessKeyDown (Key keyEvent)
     {
-        //ProcessKeyDown?.Invoke (this, keyEvent);
         return keyEvent.Handled;
     }
 
@@ -446,20 +466,20 @@ public bool NewKeyUpEvent (Key key)
         }
 
         return false;
-    }
 
-    private bool RaiseKeyUp (Key key)
-    {
-        // Before (fire the cancellable event)
-        if (OnKeyUp (key) || key.Handled)
+        bool RaiseKeyUp (Key key)
         {
-            return true;
-        }
+            // Before (fire the cancellable event)
+            if (OnKeyUp (key) || key.Handled)
+            {
+                return true;
+            }
 
-        // fire event
-        KeyUp?.Invoke (this, key);
+            // fire event
+            KeyUp?.Invoke (this, key);
 
-        return key.Handled;
+            return key.Handled;
+        }
     }
 
 

diff --git a/Terminal.Gui/Views/DateField.cs b/Terminal.Gui/Views/DateField.cs
@@ -131,7 +131,7 @@ protected internal override bool OnMouseEvent  (MouseEvent ev)
     public virtual void OnDateChanged (DateTimeEventArgs<DateTime> args) { DateChanged?.Invoke (this, args); }
 
     /// <inheritdoc/>
-    public override bool OnProcessKeyDown (Key a)
+    protected override bool OnProcessKeyDown (Key a)
     {
         // Ignore non-numeric characters.
         if (a >= Key.D0 && a <= Key.D9)

diff --git a/Terminal.Gui/Views/FileDialog.cs b/Terminal.Gui/Views/FileDialog.cs
@@ -233,7 +233,7 @@ internal FileDialog (IFileSystem fileSystem)
         _tbPath.TextChanged += (s, e) => PathChanged ();
 
         _tableView.CellActivated += CellActivate;
-        _tableView.KeyUp += (s, k) => k.Handled = TableView_KeyUp (k);
+        _tableView.KeyDown += (s, k) => k.Handled = TableView_KeyUp (k);
         _tableView.SelectedCellChanged += TableView_SelectedCellChanged;
 
         _tableView.KeyBindings.ReplaceCommands (Key.Home, Command.Start);

diff --git a/Terminal.Gui/Views/HexView.cs b/Terminal.Gui/Views/HexView.cs
@@ -452,7 +452,7 @@ void SetAttribute (Attribute attribute)
     public virtual void OnPositionChanged () { PositionChanged?.Invoke (this, new HexViewEventArgs (Position, CursorPosition, BytesPerLine)); }
 
     /// <inheritdoc/>
-    public override bool OnProcessKeyDown (Key keyEvent)
+    protected override bool OnProcessKeyDown (Key keyEvent)
     {
         if (!AllowEdits)
         {

diff --git a/Terminal.Gui/Views/ListView.cs b/Terminal.Gui/Views/ListView.cs
@@ -803,7 +803,7 @@ public bool OnOpenSelectedItem ()
     }
 
     /// <inheritdoc/>
-    public override bool OnKeyDown (Key a)
+    protected override bool OnKeyDown (Key a)
     {
         // Enable user to find & select an item by typing text
         if (CollectionNavigatorBase.IsCompatibleKey (a) && (!AllowsMarking && a == Key.Space))

diff --git a/Terminal.Gui/Views/ScrollView.cs b/Terminal.Gui/Views/ScrollView.cs
@@ -388,7 +388,7 @@ public override void OnDrawContent (Rectangle viewport)
     }
 
     /// <inheritdoc/>
-    public override bool OnKeyDown (Key a)
+    protected override bool OnKeyDown (Key a)
     {
         if (base.OnKeyDown (a))
         {

diff --git a/Terminal.Gui/Views/TableView/TableView.cs b/Terminal.Gui/Views/TableView/TableView.cs
@@ -988,7 +988,7 @@ public override void OnDrawContent (Rectangle viewport)
     }
 
     /// <inheritdoc/>
-    public override bool OnKeyDown (Key key)
+    protected override bool OnKeyDown (Key key)
     {
         if (TableIsNullOrInvisible ())
         {

diff --git a/Terminal.Gui/Views/TextField.cs b/Terminal.Gui/Views/TextField.cs
@@ -1055,7 +1055,7 @@ protected override void OnHasFocusChanged (bool newHasFocus, View previousFocuse
     /// </summary>
     /// <param name="a"></param>
     /// <returns></returns>
-    public override bool OnProcessKeyDown (Key a)
+    protected override bool OnProcessKeyDown (Key a)
     {
         // Remember the cursor position because the new calculated cursor position is needed
         // to be set BEFORE the TextChanged event is triggered.

diff --git a/Terminal.Gui/Views/TextValidateField.cs b/Terminal.Gui/Views/TextValidateField.cs
@@ -597,7 +597,7 @@ public override void OnDrawContent (Rectangle viewport)
         }
 
         /// <inheritdoc/>
-        public override bool OnProcessKeyDown (Key a)
+        protected override bool OnProcessKeyDown (Key a)
         {
             if (_provider is null)
             {

diff --git a/Terminal.Gui/Views/TextView.cs b/Terminal.Gui/Views/TextView.cs
@@ -3703,7 +3703,7 @@ protected override void OnHasFocusChanged (bool newHasFocus, View? previousFocus
     }
 
     /// <inheritdoc/>
-    public override bool OnProcessKeyDown (Key a)
+    protected override bool OnProcessKeyDown (Key a)
     {
         if (!CanFocus)
         {

diff --git a/Terminal.Gui/Views/TileView.cs b/Terminal.Gui/Views/TileView.cs
@@ -286,7 +286,7 @@ public override void OnDrawContent (Rectangle viewport)
 
     //// BUGBUG: Why is this not handled by a key binding???
     /// <inheritdoc/>
-    public override bool OnProcessKeyDown (Key keyEvent)
+    protected override bool OnProcessKeyDown (Key keyEvent)
     {
         var focusMoved = false;
 

diff --git a/Terminal.Gui/Views/TimeField.cs b/Terminal.Gui/Views/TimeField.cs
@@ -177,7 +177,7 @@ protected internal override bool OnMouseEvent  (MouseEvent ev)
     }
 
     /// <inheritdoc/>
-    public override bool OnProcessKeyDown (Key a)
+    protected override bool OnProcessKeyDown (Key a)
     {
         // Ignore non-numeric characters.
         if (a.KeyCode is >= (KeyCode)(int)KeyCode.D0 and <= (KeyCode)(int)KeyCode.D9)

diff --git a/Terminal.Gui/Views/TreeView/TreeView.cs b/Terminal.Gui/Views/TreeView/TreeView.cs
@@ -1182,7 +1182,7 @@ protected override void OnHasFocusChanged (bool newHasFocus, [CanBeNull] View cu
     }
 
     /// <inheritdoc/>
-    public override bool OnKeyDown (Key keyEvent)
+    protected override bool OnKeyDown (Key keyEvent)
     {
         if (!Enabled)
         {

diff --git a/Terminal.Gui/Views/Wizard/Wizard.cs b/Terminal.Gui/Views/Wizard/Wizard.cs
@@ -386,7 +386,7 @@ public bool GoToStep (WizardStep newStep)
     /// </summary>
     /// <param name="key"></param>
     /// <returns></returns>
-    public override bool OnProcessKeyDown (Key key)
+    protected override bool OnProcessKeyDown (Key key)
     {
         //// BUGBUG: Why is this not handled by a key binding???
         if (!Modal)

diff --git a/UICatalog/Scenarios/LineDrawing.cs b/UICatalog/Scenarios/LineDrawing.cs
@@ -121,7 +121,7 @@ public override void Main ()
         tools.CurrentColor = canvas.GetNormalColor ();
         canvas.CurrentAttribute = tools.CurrentColor;
 
-        win.KeyDown += (s, e) => { e.Handled = canvas.OnKeyDown (e); };
+        win.KeyDown += (s, e) => { e.Handled = canvas.NewKeyDownEvent (e); };
 
         Application.Run (win);
         win.Dispose ();
@@ -290,7 +290,7 @@ public override void OnDrawContentComplete (Rectangle viewport)
     }
 
     //// BUGBUG: Why is this not handled by a key binding???
-    public override bool OnKeyDown (Key e)
+    protected override bool OnKeyDown (Key e)
     {
         // BUGBUG: These should be implemented with key bindings
         if (e.KeyCode == (KeyCode.Z | KeyCode.CtrlMask))

diff --git a/UICatalog/Scenarios/Snake.cs b/UICatalog/Scenarios/Snake.cs
@@ -357,7 +357,7 @@ public override void OnDrawContent (Rectangle viewport)
         }
 
         // BUGBUG: Should (can) this use key bindings instead.
-        public override bool OnKeyDown (Key keyEvent)
+        protected override bool OnKeyDown (Key keyEvent)
         {
             if (keyEvent.KeyCode == KeyCode.CursorUp)
             {

diff --git a/UICatalog/Scenarios/VkeyPacketSimulator.cs b/UICatalog/Scenarios/VkeyPacketSimulator.cs
@@ -112,7 +112,7 @@ public override void Main ()
 
                                     if (handled == null || handled == false)
                                     {
-                                        if (!tvOutput.OnProcessKeyDown (e))
+                                        if (!tvOutput.NewKeyDownEvent (e))
                                         {
                                             Application.Invoke (
                                                                 () => MessageBox.Query (

diff --git a/UnitTests/FileServices/FileDialogTests.cs b/UnitTests/FileServices/FileDialogTests.cs
@@ -140,7 +140,7 @@ public void DotDot_MovesToRoot_ThenPressBack ()
         AssertIsTheStartingDirectory (dlg.Path);
 
         Assert.IsType<TextField> (dlg.MostFocused);
-        Send ('v', ConsoleKey.DownArrow);
+        Application.OnKeyDown (Key.CursorDown);
 
         var tv = GetTableView(dlg);
         tv.SetFocus ();
@@ -152,15 +152,17 @@ public void DotDot_MovesToRoot_ThenPressBack ()
         AssertIsTheStartingDirectory (dlg.Path);
 
         // Accept navigation up a directory
-        Send ('\n', ConsoleKey.Enter);
+        Application.OnKeyDown (Key.Enter);
 
         AssertIsTheRootDirectory (dlg.Path);
 
         Assert.True (dlg.Canceled);
         Assert.False (selected);
 
-        // Now press the back button (in table view)
-        Send ('<', ConsoleKey.Backspace);
+        Assert.IsType<TableView> (dlg.MostFocused);
+
+        // Now press Backspace (in table view)
+        Application.OnKeyDown (Key.Backspace);
 
         // Should move us back to the root
         AssertIsTheStartingDirectory (dlg.Path);

diff --git a/UnitTests/Input/ResponderTests.cs b/UnitTests/Input/ResponderTests.cs
@@ -205,11 +205,11 @@ public void KeyPressed_Handled_True_Cancels_KeyPress ()
         var r = new View ();
         var args = new Key { KeyCode = KeyCode.Null };
 
-        Assert.False (r.OnKeyDown (args));
+        Assert.False (r.NewKeyDownEvent (args));
         Assert.False (args.Handled);
 
         r.KeyDown += (s, a) => a.Handled = true;
-        Assert.True (r.OnKeyDown (args));
+        Assert.True (r.NewKeyDownEvent (args));
         Assert.True (args.Handled);
 
         r.Dispose ();
@@ -232,8 +232,8 @@ public void New_Methods_Return_False ()
         var r = new View ();
 
         //Assert.False (r.OnKeyDown (new KeyEventArgs () { Key = Key.Unknown }));
-        Assert.False (r.OnKeyDown (new Key { KeyCode = KeyCode.Null }));
-        Assert.False (r.OnKeyUp (new Key { KeyCode = KeyCode.Null }));
+        Assert.False (r.NewKeyDownEvent (new Key { KeyCode = KeyCode.Null }));
+        Assert.False (r.NewKeyDownEvent (new Key { KeyCode = KeyCode.Null }));
         Assert.False (r.NewMouseEvent (new MouseEvent { Flags = MouseFlags.AllEvents }));
 
         var v = new View ();
@@ -293,6 +293,6 @@ public void Responder_Not_Notifying_Dispose ()
 
     public class DerivedView : View
     {
-        public override bool OnKeyDown (Key keyEvent) { return true; }
+        protected override bool OnKeyDown (Key keyEvent) { return true; }
     }
 }
diff --git a/UnitTests/View/KeyboardEventTests.cs b/UnitTests/View/KeyboardEventTests.cs
@@ -463,7 +463,7 @@ public class OnKeyTestView : View
             return CancelVirtualMethods;
         }
 
-        public override bool OnKeyDown (Key keyEvent)
+        protected override bool OnKeyDown (Key keyEvent)
         {
             OnKeyDownCalled = true;
 
@@ -477,7 +477,7 @@ public override bool OnKeyUp (Key keyEvent)
             return CancelVirtualMethods;
         }
 
-        public override bool OnProcessKeyDown (Key keyEvent)
+        protected override bool OnProcessKeyDown (Key keyEvent)
         {
             if (base.OnProcessKeyDown (keyEvent))
             {