diff --git a/Terminal.Gui/View/View.Keyboard.cs b/Terminal.Gui/View/View.Keyboard.cs
index c1a28a5b9..cf7f9cdf1 100644
--- a/Terminal.Gui/View/View.Keyboard.cs
+++ b/Terminal.Gui/View/View.Keyboard.cs
@@ -1,5 +1,6 @@
#nullable enable
using System.Diagnostics;
+using Microsoft.CodeAnalysis.FlowAnalysis;
namespace Terminal.Gui;
@@ -292,17 +293,20 @@ public partial class View // Keyboard APIs
// During (this is what can be cancelled)
- if (RaiseInvokingKeyBindingsAndInvokeCommands (key) is not false || key.Handled)
- {
- return true;
- }
+ // TODO: NewKeyDownEvent returns bool. It should be bool? so state of RaiseInvokingKeyBindingsAndInvokeCommands can be reflected up stack
- if (RaiseProcessKeyDown (key) || key.Handled)
+
+ if (RaiseInvokingKeyBindingsAndInvokeCommands (key) is true || key.Handled)
{
return true;
}
// After
+ // TODO: Is ProcessKeyDown really the right name?
+ if (RaiseProcessKeyDown (key) || key.Handled)
+ {
+ return true;
+ }
return key.Handled;
@@ -322,14 +326,13 @@ public partial class View // Keyboard APIs
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);
-
- if (!key.Handled && OnProcessKeyDown (key))
+ if (OnProcessKeyDown (key) || key.Handled)
{
return true;
}
+ ProcessKeyDown?.Invoke (this, key);
+
return false;
}
}
@@ -512,17 +515,22 @@ public partial class View // Keyboard APIs
///
///
///
- ///
+ ///
+ /// if no command was invoked or there was no matching key binding; input processing should continue.
+ /// if a command was invoked and was not handled (or cancelled); input processing should
+ /// continue.
+ /// if was handled or a command was invoked and handled (or cancelled); input processing should stop.
+ ///
internal bool? RaiseInvokingKeyBindingsAndInvokeCommands (Key key)
{
- // Before
- // fire event only if there's a hotkey binding for the key
- if (!KeyBindings.TryGet (key, KeyBindingScope.Focused | KeyBindingScope.HotKey, out KeyBinding kb))
- {
- return null;
- }
+ //// Before
+ //// fire event only if there's a hotkey binding for the key
+ //if (!KeyBindings.TryGet (key, KeyBindingScope.Focused | KeyBindingScope.HotKey, out KeyBinding kb))
+ //{
+ // return null;
+ //}
- KeyBindingScope scope = kb.Scope;
+ KeyBindingScope scope = KeyBindingScope.Focused | KeyBindingScope.HotKey;
// During
// BUGBUG: The proper pattern is for the v-method (OnInvokingKeyBindings) to be called first, then the event
@@ -533,14 +541,13 @@ public partial class View // Keyboard APIs
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)
+ if (OnInvokingKeyBindings (key, scope))
{
return true;
}
+ bool? handled;
+
// After
// * If no key binding was found, `InvokeKeyBindings` returns `null`.
@@ -636,7 +643,7 @@ public partial class View // Keyboard APIs
return true;
}
- bool? subViewHandled = subview.OnInvokingKeyBindings (keyEvent, scope);
+ bool? subViewHandled = subview.RaiseInvokingKeyBindingsAndInvokeCommands (keyEvent);
if (subViewHandled is { })
{
@@ -694,26 +701,25 @@ public partial class View // Keyboard APIs
///
- /// 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.
+ /// Called 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.
///
///
- /// 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
+ /// if the event was raised and was not handled (or cancelled); input processing should
/// continue.
- /// if the event was raised and handled (or cancelled); input proessing should stop.
+ /// if the event was raised and handled (or cancelled); input processing should stop.
///
- protected virtual bool? OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope)
+ protected virtual bool OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope)
{
return false;
}
+ // TODO: This does not carry KeyBindingScope, but OnInvokingKeyBindings does
///
/// 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.
@@ -727,10 +733,10 @@ public partial class View // Keyboard APIs
/// The key event passed.
/// The scope.
///
- /// if no command was invoked; input proessing should continue.
- /// if at least one command was invoked and was not handled (or cancelled); input proessing
+ /// if no command was invoked; input processing should continue.
+ /// if at least one command was invoked and was not handled (or cancelled); input processing
/// should continue.
- /// if at least one command was invoked and handled (or cancelled); input proessing should stop.
+ /// if at least one command was invoked and handled (or cancelled); input processing should stop.
///
protected bool? InvokeCommands (Key key, KeyBindingScope scope)
{
diff --git a/Terminal.Gui/Views/Menu/Menu.cs b/Terminal.Gui/Views/Menu/Menu.cs
index 7d7715a53..9ee580d49 100644
--- a/Terminal.Gui/Views/Menu/Menu.cs
+++ b/Terminal.Gui/Views/Menu/Menu.cs
@@ -305,19 +305,11 @@ internal sealed class Menu : View
return true;
}
- ///
- protected override bool? OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope)
+ ///
+ protected override bool OnProcessKeyDown (Key keyEvent)
{
- bool? handled = base.OnInvokingKeyBindings (keyEvent, scope);
-
- if (handled is { } && (bool)handled)
- {
- return true;
- }
-
- // 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.RaiseInvokingKeyBindingsAndInvokeCommands (keyEvent);
+ // We didn't handle the key, pass it on to host
+ return _host.RaiseInvokingKeyBindingsAndInvokeCommands (keyEvent) == true;
}
private void Current_TerminalResized (object? sender, SizeChangedEventArgs e)
diff --git a/Terminal.Gui/Views/TextField.cs b/Terminal.Gui/Views/TextField.cs
index b27cd82e0..d1cfed3b1 100644
--- a/Terminal.Gui/Views/TextField.cs
+++ b/Terminal.Gui/Views/TextField.cs
@@ -1014,7 +1014,7 @@ public class TextField : View
}
///
- protected 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 770e4293b..4cc86bf2f 100644
--- a/Terminal.Gui/Views/TextView.cs
+++ b/Terminal.Gui/Views/TextView.cs
@@ -3664,7 +3664,7 @@ public class TextView : View
}
///
- protected override bool? OnInvokingKeyBindings (Key a, KeyBindingScope scope)
+ protected override bool OnInvokingKeyBindings (Key a, KeyBindingScope scope)
{
if (!a.IsValid)
{
diff --git a/UnitTests/View/KeyboardEventTests.cs b/UnitTests/View/Keyboard/KeyboardEventTests.cs
similarity index 86%
rename from UnitTests/View/KeyboardEventTests.cs
rename to UnitTests/View/Keyboard/KeyboardEventTests.cs
index fc601590c..ae597a596 100644
--- a/UnitTests/View/KeyboardEventTests.cs
+++ b/UnitTests/View/Keyboard/KeyboardEventTests.cs
@@ -12,7 +12,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
///
[Theory]
[MemberData (nameof (AllViewTypes))]
- public void AllViews_KeyDown_All_EventsFire (Type viewType)
+ public void AllViews_NewKeyDownEvent_All_EventsFire (Type viewType)
{
var view = CreateInstanceIfNotGeneric (viewType);
@@ -49,7 +49,8 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
keyDownProcessed = true;
};
- Assert.True (view.NewKeyDownEvent (Key.A)); // this will be true because the ProcessKeyDown event handled it
+ // Key.Empty is invalid, but it's used here to test that the event is fired
+ Assert.True (view.NewKeyDownEvent (Key.Empty)); // this will be true because the ProcessKeyDown event handled it
Assert.True (keyDown);
Assert.True (invokingKeyBindings);
Assert.True (keyDownProcessed);
@@ -62,7 +63,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
///
[Theory]
[MemberData (nameof (AllViewTypes))]
- public void AllViews_KeyUp_All_EventsFire (Type viewType)
+ public void AllViews_NewKeyUpEvent_All_EventsFire (Type viewType)
{
var view = CreateInstanceIfNotGeneric (viewType);
@@ -92,13 +93,13 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
[InlineData (true, false, false)]
[InlineData (true, true, false)]
[InlineData (true, true, true)]
- public void Events_Are_Called_With_Only_Key_Modifiers (bool shift, bool alt, bool control)
+ public void NewKeyDownUpEvents_Events_Are_Raised_With_Only_Key_Modifiers (bool shift, bool alt, bool control)
{
var keyDown = false;
var keyPressed = false;
var keyUp = false;
- var view = new OnKeyTestView ();
+ var view = new OnNewKeyTestView ();
view.CancelVirtualMethods = false;
view.KeyDown += (s, e) =>
@@ -139,7 +140,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
);
Assert.True (keyPressed);
Assert.True (view.OnKeyDownCalled);
- Assert.True (view.OnKeyPressedContinued);
+ Assert.True (view.OnProcessKeyDownCalled);
view.NewKeyUpEvent (
new (
@@ -154,7 +155,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
}
[Fact]
- public void InvokingKeyBindings_Handled_Cancels ()
+ public void NewKeyDownEvent_InvokingKeyBindings_Handled_Cancels ()
{
var view = new View ();
var keyPressInvoked = false;
@@ -201,13 +202,13 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
}
[Fact]
- public void InvokingKeyBindings_Handled_True_Stops_Processing ()
+ public void NewKeyDownEvent_InvokingKeyBindings_Handled_True_Stops_Processing ()
{
var keyDown = false;
var invokingKeyBindings = false;
var keyPressed = false;
- var view = new OnKeyTestView ();
+ var view = new OnNewKeyTestView ();
Assert.True (view.CanFocus);
view.CancelVirtualMethods = false;
@@ -233,7 +234,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
{
Assert.Equal (KeyCode.A, e.KeyCode);
Assert.False (keyPressed);
- Assert.False (view.OnKeyPressedContinued);
+ Assert.False (view.OnProcessKeyDownCalled);
e.Handled = true;
keyPressed = true;
};
@@ -245,17 +246,17 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
Assert.True (view.OnKeyDownCalled);
Assert.False (view.OnInvokingKeyBindingsCalled);
- Assert.False (view.OnKeyPressedContinued);
+ Assert.False (view.OnProcessKeyDownCalled);
}
[Fact]
- public void KeyDown_Handled_True_Stops_Processing ()
+ public void NewKeyDownEvent_Handled_True_Stops_Processing ()
{
var keyDown = false;
var invokingKeyBindings = false;
var keyPressed = false;
- var view = new OnKeyTestView ();
+ var view = new OnNewKeyTestView ();
Assert.True (view.CanFocus);
view.CancelVirtualMethods = false;
@@ -281,7 +282,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
{
Assert.Equal (KeyCode.A, e.KeyCode);
Assert.False (keyPressed);
- Assert.False (view.OnKeyPressedContinued);
+ Assert.False (view.OnProcessKeyDownCalled);
e.Handled = true;
keyPressed = true;
};
@@ -293,11 +294,11 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
Assert.True (view.OnKeyDownCalled);
Assert.False (view.OnInvokingKeyBindingsCalled);
- Assert.False (view.OnKeyPressedContinued);
+ Assert.False (view.OnProcessKeyDownCalled);
}
[Fact]
- public void KeyPress_Handled_Cancels ()
+ public void NewKeyDownEvent_KeyDown_Handled_Stops_Processing ()
{
var view = new View ();
var invokingKeyBindingsInvoked = false;
@@ -338,13 +339,13 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
}
[Fact]
- public void KeyPressed_Handled_True_Stops_Processing ()
+ public void NewKeyDownEvent_ProcessKeyDown_Handled_Stops_Processing ()
{
var keyDown = false;
var invokingKeyBindings = false;
- var keyPressed = false;
+ var processKeyDown = false;
- var view = new OnKeyTestView ();
+ var view = new OnNewKeyTestView ();
Assert.True (view.CanFocus);
view.CancelVirtualMethods = false;
@@ -360,7 +361,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
view.InvokingKeyBindings += (s, e) =>
{
Assert.Equal (KeyCode.A, e.KeyCode);
- Assert.False (keyPressed);
+ Assert.False (processKeyDown);
Assert.False (view.OnInvokingKeyBindingsCalled);
e.Handled = false;
invokingKeyBindings = true;
@@ -369,28 +370,28 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
view.ProcessKeyDown += (s, e) =>
{
Assert.Equal (KeyCode.A, e.KeyCode);
- Assert.False (keyPressed);
- Assert.False (view.OnKeyPressedContinued);
+ Assert.False (processKeyDown);
+ Assert.True (view.OnProcessKeyDownCalled);
e.Handled = true;
- keyPressed = true;
+ processKeyDown = true;
};
view.NewKeyDownEvent (Key.A);
Assert.True (keyDown);
Assert.True (invokingKeyBindings);
- Assert.True (keyPressed);
+ Assert.True (processKeyDown);
Assert.True (view.OnKeyDownCalled);
Assert.True (view.OnInvokingKeyBindingsCalled);
- Assert.False (view.OnKeyPressedContinued);
+ Assert.True (view.OnProcessKeyDownCalled);
}
[Fact]
- public void KeyUp_Handled_True_Stops_Processing ()
+ public void NewKeyUpEvent_KeyUp_Handled_True_Stops_Processing ()
{
var keyUp = false;
- var view = new OnKeyTestView ();
+ var view = new OnNewKeyTestView ();
Assert.True (view.CanFocus);
view.CancelVirtualMethods = false;
@@ -398,7 +399,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
{
Assert.Equal (KeyCode.A, e.KeyCode);
Assert.False (keyUp);
- Assert.False (view.OnKeyPressedContinued);
+ Assert.False (view.OnProcessKeyDownCalled);
e.Handled = true;
keyUp = true;
};
@@ -409,14 +410,14 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
Assert.True (view.OnKeyUpCalled);
Assert.False (view.OnKeyDownCalled);
Assert.False (view.OnInvokingKeyBindingsCalled);
- Assert.False (view.OnKeyPressedContinued);
+ Assert.False (view.OnProcessKeyDownCalled);
}
[Theory]
[InlineData (null, null)]
[InlineData (true, true)]
[InlineData (false, false)]
- public void OnInvokingKeyBindings_Returns_Nullable_Properly (bool? toReturn, bool? expected)
+ public void RaiseInvokingKeyBindingsAndInvokeCommands_Returns_Nullable_Properly (bool? toReturn, bool? expected)
{
var view = new KeyBindingsTestView ();
view.CommandReturns = toReturn;
@@ -439,17 +440,17 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
}
/// A view that overrides the OnKey* methods so we can test that they are called.
- public class OnKeyTestView : View
+ public class OnNewKeyTestView : View
{
- public OnKeyTestView () { CanFocus = true; }
+ public OnNewKeyTestView () { CanFocus = true; }
public bool CancelVirtualMethods { set; private get; }
public bool OnInvokingKeyBindingsCalled { get; set; }
public bool OnKeyDownCalled { get; set; }
- public bool OnKeyPressedContinued { get; set; }
+ public bool OnProcessKeyDownCalled { get; set; }
public bool OnKeyUpCalled { get; set; }
public override string Text { get; set; }
- protected override bool? OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope)
+ protected override bool OnInvokingKeyBindings (Key keyEvent, KeyBindingScope scope)
{
OnInvokingKeyBindingsCalled = true;
@@ -473,12 +474,7 @@ public class KeyboardEventTests (ITestOutputHelper output) : TestsAllViews
protected override bool OnProcessKeyDown (Key keyEvent)
{
- if (base.OnProcessKeyDown (keyEvent))
- {
- return true;
- }
-
- OnKeyPressedContinued = true;
+ OnProcessKeyDownCalled = true;
return CancelVirtualMethods;
}
diff --git a/UnitTests/Views/MenuBarTests.cs b/UnitTests/Views/MenuBarTests.cs
index 736e79890..849d9b8f4 100644
--- a/UnitTests/Views/MenuBarTests.cs
+++ b/UnitTests/Views/MenuBarTests.cs
@@ -1384,7 +1384,7 @@ wo
foreach (Key key in keys)
{
- top.NewKeyDownEvent (new (key));
+ top.NewKeyDownEvent (key);
Application.MainLoop.RunIteration ();
}
diff --git a/docfx/docs/events.md b/docfx/docs/events.md
new file mode 100644
index 000000000..19e853f6a
--- /dev/null
+++ b/docfx/docs/events.md
@@ -0,0 +1,126 @@
+# Terminal.Gui Event Deep Dive
+
+Terminal.Gui exposes and uses events in many places. This deep dive covers the patterns used, where they are used, and notes any exceptions.
+
+## Tenets for Terminal.Gui Events (Unless you know better ones...)
+
+Tenets higher in the list have precedence over tenets lower in the list.
+
+* **UI Interaction and Live Data Are Different Beasts** - TG distinguishes between events used for human interaction and events for live data. We don't believe in a one-size-fits-all eventing model. For UI interactions we use `EventHandler`. For data binding we think `INotifyPropertyChanged` is groovy. For some callbacks we use `Action`.
+
+## Lexicon and Taxonomy
+
+* *Action*
+* *Event*
+* *Command*
+* *Invoke*
+* *Raise*
+* *Listen*
+* *Handle/Handling/Handled* - Applies to scenarios where an event can either be handled by an event listener (or override) vs not handled. Events that originate from a user action like mouse moves and key presses are examples.
+* *Cancel/Cancelling/Cancelled* - Applies to scenarios where something can be cancelled. Changing the `Orientation` of a `Slider` is cancelable.
+
+## Useful External Documentation
+
+* [.NET Naming Guidelines - Names of Events](https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/names-of-type-members?redirectedfrom=MSDN#names-of-events)
+* [.NET Design for Extensibility - Events and Callbacks](https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/events-and-callbacks)
+* [C# Event Implementation Fundamentals, Best Practices and Conventions](https://www.codeproject.com/Articles/20550/C-Event-Implementation-Fundamentals-Best-Practices)
+
+## Naming
+
+TG follows the *naming* advice provided in [.NET Naming Guidelines - Names of Events](https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/names-of-type-members?redirectedfrom=MSDN#names-of-events).
+
+## `EventHandler` style event best-practices
+
+* Implement a helper method for raising the event: `RaisexxxEvent`.
+ * If the event is cancelable, the return type should be either `bool` or `bool?`.
+ * Can be `private`, `internal`, or `public` depending on the situation. `internal` should only be used to enable unit tests.
+* Raising an event involves FIRST calling the `protected virtual` method, THEN invoking the `EventHandler.
+
+## `Action` style callback best-practices
+
+- tbd
+
+## `INotifyPropertyChanged` style notification best practices
+
+- tbd
+
+## Common Patterns
+
+The primary pattern for events is the `event/EventHandler` idiom. We use the `Action` idiom sparingly. We support `INotifyPropertyChanged` in cases where data binding is relevant.
+
+
+
+## Cancellable Event Pattern
+
+A cancellable event is really two events and some activity that takes place between those events. The "pre-event" happens before the activity. The activity then takes place (or not). If the activity takes place, then the "post-event" is typically raised. So, to be precise, no event is being cancelled even though we say we have a cancellable event. Rather, the activity that takes place between the two events is what is cancelled — and likely prevented from starting at all.
+
+### **Before** - If any pre-conditions are met raise the "pre-event", typically named in the form of "xxxChanging". e.g.
+
+ - A `protected virtual` method is called. This method is named `OnxxxChanging` and the base implementation simply does `return false`.
+ - If the `OnxxxChanging` method returns `true` it means a derived class canceled the event. Processing should stop.
+ - Otherwise, the `xxxChanging` event is invoked via `xxxChanging?.Invoke(args)`. If `args.Cancel/Handled == true` it means a subscriber has cancelled the event. Processing should stop.
+
+
+### **During** - Do work.
+
+### **After** - Raise the "post-event", typically named in the form of "xxxChanged"
+
+ - A `protected virtual` method is called. This method is named `OnxxxChanged` has a return type of `void`.
+ - The `xxxChanged` event is invoked via `xxxChanging?.Invoke(args)`.
+
+The `OrientationHelper` class supporting `IOrientation` and a `View` having an `Orientation` property illustrates the preferred TG pattern for cancelable events.
+
+```cs
+ ///
+ /// Gets or sets the orientation of the View.
+ ///
+ public Orientation Orientation
+ {
+ get => _orientation;
+ set
+ {
+ if (_orientation == value)
+ {
+ return;
+ }
+
+ // Best practice is to invoke the virtual method first.
+ // This allows derived classes to handle the event and potentially cancel it.
+ if (_owner?.OnOrientationChanging (value, _orientation) ?? false)
+ {
+ return;
+ }
+
+ // If the event is not canceled by the virtual method, raise the event to notify any external subscribers.
+ CancelEventArgs args = new (in _orientation, ref value);
+ OrientationChanging?.Invoke (_owner, args);
+
+ if (args.Cancel)
+ {
+ return;
+ }
+
+ // If the event is not canceled, update the value.
+ Orientation old = _orientation;
+
+ if (_orientation != value)
+ {
+ _orientation = value;
+
+ if (_owner is { })
+ {
+ _owner.Orientation = value;
+ }
+ }
+
+ // Best practice is to invoke the virtual method first.
+ _owner?.OnOrientationChanged (_orientation);
+
+ // Even though Changed is not cancelable, it is still a good practice to raise the event after.
+ OrientationChanged?.Invoke (_owner, new (in _orientation));
+ }
+ }
+```
+
+
+