From ee196fec48327b2fc0651d168ea79e8b505dc347 Mon Sep 17 00:00:00 2001 From: Tig Date: Mon, 14 Oct 2024 08:41:22 -0600 Subject: [PATCH] WIP: Keyboard event improvements --- Terminal.Gui/View/View.Keyboard.cs | 147 +++++++++------------ Terminal.Gui/Views/Menu/Menu.cs | 4 +- Terminal.Gui/Views/ScrollView.cs | 2 +- Terminal.Gui/Views/TextField.cs | 2 +- Terminal.Gui/Views/TextView.cs | 2 +- UICatalog/Scenarios/VkeyPacketSimulator.cs | 2 +- UnitTests/View/KeyboardEventTests.cs | 28 ++-- 7 files changed, 83 insertions(+), 104 deletions(-) diff --git a/Terminal.Gui/View/View.Keyboard.cs b/Terminal.Gui/View/View.Keyboard.cs index c3ccd8ca9..c1a28a5b9 100644 --- a/Terminal.Gui/View/View.Keyboard.cs +++ b/Terminal.Gui/View/View.Keyboard.cs @@ -292,16 +292,17 @@ public partial class View // Keyboard APIs // During (this is what can be cancelled) - if (RaiseInvokingKeyBindings (key) || key.Handled) + if (RaiseInvokingKeyBindingsAndInvokeCommands (key) is not false || key.Handled) + { + return true; + } + + if (RaiseProcessKeyDown (key) || key.Handled) { return true; } // After - if (RaiseProcessKeyDown (key) || key.Handled) - { - return true; - } return key.Handled; @@ -319,28 +320,6 @@ public partial class View // Keyboard APIs return key.Handled; } - bool RaiseInvokingKeyBindings (Key key) - { - // 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; - } - bool RaiseProcessKeyDown (Key key) { // BUGBUG: The proper pattern is for the v-method (OnProcessKeyDown) to be called first, then the event @@ -529,70 +508,78 @@ public partial class View // Keyboard APIs private Dictionary CommandImplementations { get; } = new (); /// - /// Low-level API called when a user presses a key; invokes any key bindings set on the view. This is called - /// during after has returned. + /// /// - /// - /// Fires the event. - /// See for an overview of Terminal.Gui keyboard APIs. - /// - /// Contains the details about the key that produced the event. - /// The scope. - /// - /// if no event was raised; input proessing should continue. - /// if the event was raised and was not handled (or cancelled); input proessing should - /// continue. - /// if the event was raised and handled (or cancelled); input proessing should stop. - /// - public virtual bool? OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope) + /// + /// + /// + internal bool? RaiseInvokingKeyBindingsAndInvokeCommands (Key key) { + // Before // fire event only if there's a hotkey binding for the key - if (KeyBindings.TryGet (keyEvent, scope, out KeyBinding kb)) + if (!KeyBindings.TryGet (key, KeyBindingScope.Focused | KeyBindingScope.HotKey, out KeyBinding kb)) { - InvokingKeyBindings?.Invoke (this, keyEvent); - - if (keyEvent.Handled) - { - return true; - } + return null; } + KeyBindingScope scope = kb.Scope; + + // During + // 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 stac + bool? handled = OnInvokingKeyBindings (key, scope); + + if (handled is { } && (bool)handled) + { + return true; + } + + // After + // * If no key binding was found, `InvokeKeyBindings` returns `null`. // Continue passing the event (return `false` from `OnInvokeKeyBindings`). // * If key bindings were found, but none handled the key (all `Command`s returned `false`), // `InvokeKeyBindings` returns `false`. Continue passing the event (return `false` from `OnInvokeKeyBindings`).. // * If key bindings were found, and any handled the key (at least one `Command` returned `true`), // `InvokeKeyBindings` returns `true`. Continue passing the event (return `false` from `OnInvokeKeyBindings`). - bool? handled = InvokeKeyBindings (keyEvent, scope); + handled = InvokeCommands (key, scope); if (handled is { } && (bool)handled) { // Stop processing if any key binding handled the key. // DO NOT stop processing if there are no matching key bindings or none of the key bindings handled the key - return true; + return handled; } - if (Margin is { } && ProcessAdornmentKeyBindings (Margin, keyEvent, scope, ref handled)) + if (Margin is { } && ProcessAdornmentKeyBindings (Margin, key, scope, ref handled)) { return true; } - if (Padding is { } && ProcessAdornmentKeyBindings (Padding, keyEvent, scope, ref handled)) + if (Padding is { } && ProcessAdornmentKeyBindings (Padding, key, scope, ref handled)) { return true; } - if (Border is { } && ProcessAdornmentKeyBindings (Border, keyEvent, scope, ref handled)) + if (Border is { } && ProcessAdornmentKeyBindings (Border, key, scope, ref handled)) { return true; } - if (ProcessSubViewKeyBindings (keyEvent, scope, ref handled)) + if (ProcessSubViewKeyBindings (key, scope, ref handled)) { return true; } return handled; + } private bool ProcessAdornmentKeyBindings (Adornment adornment, Key keyEvent, KeyBindingScope scope, ref bool? handled) @@ -705,6 +692,28 @@ public partial class View // Keyboard APIs return false; } + + /// + /// Low-level API called when a user presses a key; invokes any key bindings set on the view. This is called + /// during after has returned. + /// + /// + /// Fires the event. + /// See for an overview of Terminal.Gui keyboard APIs. + /// + /// Contains the details about the key that produced the event. + /// The scope. + /// + /// if no event was raised; input proessing should continue. + /// if the event was raised and was not handled (or cancelled); input proessing should + /// continue. + /// if the event was raised and handled (or cancelled); input proessing should stop. + /// + protected virtual bool? OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope) + { + return false; + } + /// /// Raised when a key is pressed that may be mapped to a key binding. Set to true to /// stop the key from being processed by other views. @@ -712,7 +721,7 @@ public partial class View // Keyboard APIs public event EventHandler? InvokingKeyBindings; /// - /// Invokes any binding that is registered on this and matches the + /// Invokes the Commands bound to . /// See for an overview of Terminal.Gui keyboard APIs. /// /// The key event passed. @@ -723,7 +732,7 @@ public partial class View // Keyboard APIs /// should continue. /// if at least one command was invoked and handled (or cancelled); input proessing should stop. /// - protected bool? InvokeKeyBindings (Key key, KeyBindingScope scope) + protected bool? InvokeCommands (Key key, KeyBindingScope scope) { bool? toReturn = null; @@ -753,30 +762,6 @@ public partial class View // Keyboard APIs #endif return InvokeCommands (binding.Commands, key, binding); - - foreach (Command command in binding.Commands) - { - if (!CommandImplementations.ContainsKey (command)) - { - throw new NotSupportedException ( - @$"A KeyBinding was set up for the command {command} ({key}) but that command is not supported by this View ({GetType ().Name})" - ); - } - - // each command has its own return value - bool? thisReturn = InvokeCommand (command, key, binding); - - // if we haven't got anything yet, the current command result should be used - toReturn ??= thisReturn; - - // if ever see a true then that's what we will return - if (thisReturn ?? false) - { - toReturn = true; - } - } - - return toReturn; } #endregion Key Bindings diff --git a/Terminal.Gui/Views/Menu/Menu.cs b/Terminal.Gui/Views/Menu/Menu.cs index 1744251f3..7d7715a53 100644 --- a/Terminal.Gui/Views/Menu/Menu.cs +++ b/Terminal.Gui/Views/Menu/Menu.cs @@ -306,7 +306,7 @@ internal sealed class Menu : View } /// - public override bool? OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope) + protected override bool? OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope) { bool? handled = base.OnInvokingKeyBindings (keyEvent, scope); @@ -317,7 +317,7 @@ internal sealed class Menu : View // TODO: Determine if there's a cleaner way to handle this. // This supports the case where the menu bar is a context menu - return _host.OnInvokingKeyBindings (keyEvent, scope); + return _host.RaiseInvokingKeyBindingsAndInvokeCommands (keyEvent); } private void Current_TerminalResized (object? sender, SizeChangedEventArgs e) diff --git a/Terminal.Gui/Views/ScrollView.cs b/Terminal.Gui/Views/ScrollView.cs index 6a2c6ffbe..9ad1a75f1 100644 --- a/Terminal.Gui/Views/ScrollView.cs +++ b/Terminal.Gui/Views/ScrollView.cs @@ -395,7 +395,7 @@ public class ScrollView : View return true; } - bool? result = InvokeKeyBindings (a, KeyBindingScope.HotKey | KeyBindingScope.Focused); + bool? result = InvokeCommands (a, KeyBindingScope.HotKey | KeyBindingScope.Focused); if (result is { }) { diff --git a/Terminal.Gui/Views/TextField.cs b/Terminal.Gui/Views/TextField.cs index 984f151f4..b27cd82e0 100644 --- a/Terminal.Gui/Views/TextField.cs +++ b/Terminal.Gui/Views/TextField.cs @@ -1014,7 +1014,7 @@ public class TextField : View } /// - public override bool? OnInvokingKeyBindings (Key a, KeyBindingScope scope) + protected override bool? OnInvokingKeyBindings (Key a, KeyBindingScope scope) { // Give autocomplete first opportunity to respond to key presses if (SelectedLength == 0 && Autocomplete.Suggestions.Count > 0 && Autocomplete.ProcessKey (a)) diff --git a/Terminal.Gui/Views/TextView.cs b/Terminal.Gui/Views/TextView.cs index f5436ea96..770e4293b 100644 --- a/Terminal.Gui/Views/TextView.cs +++ b/Terminal.Gui/Views/TextView.cs @@ -3664,7 +3664,7 @@ public class TextView : View } /// - public override bool? OnInvokingKeyBindings (Key a, KeyBindingScope scope) + protected override bool? OnInvokingKeyBindings (Key a, KeyBindingScope scope) { if (!a.IsValid) { diff --git a/UICatalog/Scenarios/VkeyPacketSimulator.cs b/UICatalog/Scenarios/VkeyPacketSimulator.cs index 6bea34c91..059203ea7 100644 --- a/UICatalog/Scenarios/VkeyPacketSimulator.cs +++ b/UICatalog/Scenarios/VkeyPacketSimulator.cs @@ -108,7 +108,7 @@ public class VkeyPacketSimulator : Scenario if (_outputStarted) { // If the key wasn't handled by the TextView will popup a Dialog with the keys pressed. - bool? handled = tvOutput.OnInvokingKeyBindings (e, KeyBindingScope.HotKey | KeyBindingScope.Focused); + bool? handled = tvOutput.NewKeyDownEvent (e); if (handled == null || handled == false) { diff --git a/UnitTests/View/KeyboardEventTests.cs b/UnitTests/View/KeyboardEventTests.cs index d60e4ec35..fc601590c 100644 --- a/UnitTests/View/KeyboardEventTests.cs +++ b/UnitTests/View/KeyboardEventTests.cs @@ -224,7 +224,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews { Assert.Equal (KeyCode.A, e.KeyCode); Assert.False (keyPressed); - Assert.False (view.OnInvokingKeyBindingsContinued); + Assert.False (view.OnInvokingKeyBindingsCalled); e.Handled = true; invokingKeyBindings = true; }; @@ -244,7 +244,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews Assert.False (keyPressed); Assert.True (view.OnKeyDownCalled); - Assert.False (view.OnInvokingKeyBindingsContinued); + Assert.False (view.OnInvokingKeyBindingsCalled); Assert.False (view.OnKeyPressedContinued); } @@ -272,7 +272,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews { Assert.Equal (KeyCode.A, e.KeyCode); Assert.False (keyPressed); - Assert.False (view.OnInvokingKeyBindingsContinued); + Assert.False (view.OnInvokingKeyBindingsCalled); e.Handled = true; invokingKeyBindings = true; }; @@ -292,7 +292,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews Assert.False (keyPressed); Assert.True (view.OnKeyDownCalled); - Assert.False (view.OnInvokingKeyBindingsContinued); + Assert.False (view.OnInvokingKeyBindingsCalled); Assert.False (view.OnKeyPressedContinued); } @@ -361,7 +361,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews { Assert.Equal (KeyCode.A, e.KeyCode); Assert.False (keyPressed); - Assert.False (view.OnInvokingKeyBindingsContinued); + Assert.False (view.OnInvokingKeyBindingsCalled); e.Handled = false; invokingKeyBindings = true; }; @@ -381,7 +381,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews Assert.True (keyPressed); Assert.True (view.OnKeyDownCalled); - Assert.True (view.OnInvokingKeyBindingsContinued); + Assert.True (view.OnInvokingKeyBindingsCalled); Assert.False (view.OnKeyPressedContinued); } @@ -408,7 +408,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews Assert.True (view.OnKeyUpCalled); Assert.False (view.OnKeyDownCalled); - Assert.False (view.OnInvokingKeyBindingsContinued); + Assert.False (view.OnInvokingKeyBindingsCalled); Assert.False (view.OnKeyPressedContinued); } @@ -421,7 +421,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews var view = new KeyBindingsTestView (); view.CommandReturns = toReturn; - bool? result = view.OnInvokingKeyBindings (Key.A, KeyBindingScope.HotKey | KeyBindingScope.Focused); + bool? result = view.RaiseInvokingKeyBindingsAndInvokeCommands (Key.A); Assert.Equal (expected, result); } @@ -443,22 +443,16 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews { public OnKeyTestView () { CanFocus = true; } public bool CancelVirtualMethods { set; private get; } - public bool OnInvokingKeyBindingsContinued { get; set; } + public bool OnInvokingKeyBindingsCalled { get; set; } public bool OnKeyDownCalled { get; set; } public bool OnKeyPressedContinued { get; set; } public bool OnKeyUpCalled { get; set; } public override string Text { get; set; } - public override bool? OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope) + protected override bool? OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope) { - bool? handled = base.OnInvokingKeyBindings (keyEvent, scope); - if (handled != null && (bool)handled) - { - return true; - } - - OnInvokingKeyBindingsContinued = true; + OnInvokingKeyBindingsCalled = true; return CancelVirtualMethods; }