From b2cf674e0baa006f3ef9f3195c2aef2d2ba715dc Mon Sep 17 00:00:00 2001 From: Tig Date: Tue, 9 Dec 2025 08:32:04 -0700 Subject: [PATCH] Fixes #4468 - `MouseGrab` regressions (#4469) * Fixed mouse grab issue * Fixed mouse grab regrssions. * Update Terminal.Gui/ViewBase/View.Mouse.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Terminal.Gui/ViewBase/View.Mouse.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Terminal.Gui/ViewBase/View.Mouse.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Terminal.Gui/ViewBase/View.Mouse.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Terminal.Gui/ViewBase/View.Mouse.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update Terminal.Gui/ViewBase/View.Mouse.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * code cleanup * Update Terminal.Gui/ViewBase/View.Mouse.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Addressing pr feedback * updated mouse.md --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Terminal.Gui/App/Mouse/MouseImpl.cs | 7 - Terminal.Gui/ViewBase/View.Hierarchy.cs | 5 + Terminal.Gui/ViewBase/View.Mouse.cs | 359 +++++++++++------- .../ViewBase/Mouse/MouseTests.cs | 13 +- docfx/docs/mouse.md | 99 ++++- 5 files changed, 330 insertions(+), 153 deletions(-) diff --git a/Terminal.Gui/App/Mouse/MouseImpl.cs b/Terminal.Gui/App/Mouse/MouseImpl.cs index ce9e0a9b6..59b5d16f9 100644 --- a/Terminal.Gui/App/Mouse/MouseImpl.cs +++ b/Terminal.Gui/App/Mouse/MouseImpl.cs @@ -291,13 +291,6 @@ internal class MouseImpl : IMouse, IDisposable return; } -#if DEBUG_IDISPOSABLE - if (View.EnableDebugIDisposableAsserts) - { - ObjectDisposedException.ThrowIf (MouseGrabView.WasDisposed, MouseGrabView); - } -#endif - if (!RaiseUnGrabbingMouseEvent (MouseGrabView)) { View view = MouseGrabView; diff --git a/Terminal.Gui/ViewBase/View.Hierarchy.cs b/Terminal.Gui/ViewBase/View.Hierarchy.cs index 729212d1a..53848fbfd 100644 --- a/Terminal.Gui/ViewBase/View.Hierarchy.cs +++ b/Terminal.Gui/ViewBase/View.Hierarchy.cs @@ -239,6 +239,11 @@ public partial class View // SuperView/SubView hierarchy management (SuperView, Logging.Warning ($"{view} cannot be Removed. It has not been added to {this}."); } + if (App?.Mouse.MouseGrabView == view) + { + App.Mouse.UngrabMouse (); + } + Rectangle touched = view.Frame; bool hadFocus = view.HasFocus; diff --git a/Terminal.Gui/ViewBase/View.Mouse.cs b/Terminal.Gui/ViewBase/View.Mouse.cs index 146057f63..60a41d1f3 100644 --- a/Terminal.Gui/ViewBase/View.Mouse.cs +++ b/Terminal.Gui/ViewBase/View.Mouse.cs @@ -1,12 +1,13 @@ using System.ComponentModel; +using System.Diagnostics; namespace Terminal.Gui.ViewBase; public partial class View // Mouse APIs { /// - /// Handles , we have detected a button - /// down in the view and have grabbed the mouse. + /// Handles , we have detected a button + /// down in the view and have grabbed the mouse. /// public IMouseHeldDown? MouseHeldDown { get; set; } @@ -227,22 +228,76 @@ public partial class View // Mouse APIs public bool WantMousePositionReports { get; set; } /// - /// Processes a new . This method is called by when a - /// mouse - /// event occurs. + /// Processes a mouse event for this view. This is the main entry point for mouse input handling, + /// called by when the mouse interacts with this view. /// /// /// - /// A view must be both enabled and visible to receive mouse events. + /// This method orchestrates the complete mouse event handling pipeline: + /// + /// + /// + /// + /// Validates pre-conditions (view must be enabled and visible) + /// + /// + /// + /// + /// Raises for low-level handling via + /// and event subscribers + /// + /// + /// + /// + /// Handles mouse grab scenarios when or + /// are set (press/release/click) + /// + /// + /// + /// + /// Invokes commands bound to mouse clicks via + /// (default: → event) + /// + /// + /// + /// + /// Handles mouse wheel events via and + /// + /// + /// + /// + /// Continuous Button Press: When is + /// and the user holds a mouse button down, this method is repeatedly called + /// with (or Button2-4) events, enabling repeating button + /// behavior (e.g., scroll buttons). /// /// - /// If is , and the user presses and holds the - /// mouse button, will be repeatedly called with the same for - /// as long as the mouse button remains pressed. + /// Mouse Grab: Views with or + /// enabled automatically grab the mouse on button press, + /// receiving all subsequent mouse events until the button is released, even if the mouse moves + /// outside the view's . + /// + /// + /// Most views should handle mouse clicks by subscribing to the event or + /// overriding rather than overriding this method. Override this method + /// only for custom low-level mouse handling (e.g., drag-and-drop). /// /// - /// - /// if the event was handled, otherwise. + /// + /// The mouse event to process. Coordinates in are relative + /// to the view's . + /// + /// + /// if the event was handled and should not be propagated; + /// if the event was not handled and should continue propagating; + /// if the view declined to handle the event (e.g., disabled or not visible). + /// + /// + /// + /// + /// + /// + /// public bool? NewMouseEvent (MouseEventArgs mouseEvent) { // Pre-conditions @@ -269,17 +324,17 @@ public partial class View // Mouse APIs } // Post-Conditions + if (HighlightStates != MouseState.None || WantContinuousButtonPressed) { if (WhenGrabbedHandlePressed (mouseEvent)) { - return mouseEvent.Handled; + // If we raised Clicked/Activated on the grabbed view, we are done + // regardless of whether the event was handled. + return true; } - if (WhenGrabbedHandleReleased (mouseEvent)) - { - return mouseEvent.Handled; - } + WhenGrabbedHandleReleased (mouseEvent); if (WhenGrabbedHandleClicked (mouseEvent)) { @@ -287,6 +342,15 @@ public partial class View // Mouse APIs } } + // We get here if the view did not handle the mouse event via OnMouseEvent/MouseEvent, and + // it did not handle the press/release/clicked events via HandlePress/HandleRelease/HandleClicked + if (mouseEvent.IsSingleDoubleOrTripleClicked) + { + // Logging.Debug ($"{mouseEvent.Flags};{mouseEvent.Position}"); + + return RaiseCommandsBoundToMouse (mouseEvent); + } + if (mouseEvent.IsWheel) { return RaiseMouseWheelEvent (mouseEvent); @@ -322,11 +386,6 @@ public partial class View // Mouse APIs MouseEvent?.Invoke (this, mouseEvent); - if (!mouseEvent.Handled) - { - mouseEvent.Handled = InvokeCommandsBoundToMouse (mouseEvent) == true; - } - return mouseEvent.Handled; } @@ -353,138 +412,166 @@ public partial class View // Mouse APIs #region WhenGrabbed Handlers /// - /// INTERNAL For cases where the view is grabbed and the mouse is clicked, this method handles the released event - /// (typically - /// when or are set). + /// INTERNAL: For cases where the view is grabbed and the mouse is pressed, this method handles the pressed events from + /// the driver. + /// When is set, this method will raise the Clicked/Selecting event + /// via each time it is called (after the first time the mouse is pressed). /// - /// - /// Marked internal just to support unit tests - /// /// - /// , if the event was handled, otherwise. - internal bool WhenGrabbedHandleReleased (MouseEventArgs mouseEvent) - { - mouseEvent.Handled = false; - - if (mouseEvent.IsReleased) - { - if (App?.Mouse.MouseGrabView == this) - { - //Logging.Debug ($"{Id} - {MouseState}"); - MouseState &= ~MouseState.Pressed; - MouseState &= ~MouseState.PressedOutside; - } - - return mouseEvent.Handled = true; - } - - return false; - } - - /// - /// INTERNAL For cases where the view is grabbed and the mouse is clicked, this method handles the released event - /// (typically - /// when or are set). - /// - /// - /// - /// Marked internal just to support unit tests - /// - /// - /// - /// , if the event was handled, otherwise. + /// , if processing should stop, otherwise. private bool WhenGrabbedHandlePressed (MouseEventArgs mouseEvent) { - mouseEvent.Handled = false; - - if (mouseEvent.IsPressed) + if (!mouseEvent.IsPressed) { - // The first time we get pressed event, grab the mouse and set focus - if (App?.Mouse.MouseGrabView != this) - { - App?.Mouse.GrabMouse (this); - - if (!HasFocus && CanFocus) - { - // Set the focus, but don't invoke Accept - SetFocus (); - } - - mouseEvent.Handled = true; - } - - if (Viewport.Contains (mouseEvent.Position)) - { - //Logging.Debug ($"{Id} - Inside Viewport: {MouseState}"); - // The mouse is inside. - if (HighlightStates.HasFlag (MouseState.Pressed)) - { - MouseState |= MouseState.Pressed; - } - - // Always clear PressedOutside when the mouse is pressed inside the Viewport - MouseState &= ~MouseState.PressedOutside; - } - - if (!Viewport.Contains (mouseEvent.Position)) - { - // Logging.Debug ($"{Id} - Outside Viewport: {MouseState}"); - // The mouse is outside. - // When WantContinuousButtonPressed is set we want to keep the mouse state as pressed (e.g. a repeating button). - // This shows the user that the button is doing something, even if the mouse is outside the Viewport. - if (HighlightStates.HasFlag (MouseState.PressedOutside) && !WantContinuousButtonPressed) - { - MouseState |= MouseState.PressedOutside; - } - } - - return mouseEvent.Handled = true; + return false; } - return false; + Debug.Assert (!mouseEvent.Handled); + mouseEvent.Handled = false; + + // If the user has just pressed the mouse, grab the mouse and set focus + if (App is null || App.Mouse.MouseGrabView != this) + { + App?.Mouse.GrabMouse (this); + + if (!HasFocus && CanFocus) + { + // Set the focus, but don't invoke Accept + SetFocus (); + } + + // This prevents raising Clicked/Selecting the first time the mouse is pressed. + mouseEvent.Handled = true; + } + + if (Viewport.Contains (mouseEvent.Position)) + { + //Logging.Debug ($"{Id} - Inside Viewport: {MouseState}"); + // The mouse is inside. + if (HighlightStates.HasFlag (MouseState.Pressed)) + { + MouseState |= MouseState.Pressed; + } + + // Always clear PressedOutside when the mouse is pressed inside the Viewport + MouseState &= ~MouseState.PressedOutside; + } + else + { + // Logging.Debug ($"{Id} - Outside Viewport: {MouseState}"); + // The mouse is outside. + // When WantContinuousButtonPressed is set we want to keep the mouse state as pressed (e.g. a repeating button). + // This shows the user that the button is doing something, even if the mouse is outside the Viewport. + if (HighlightStates.HasFlag (MouseState.PressedOutside) && !WantContinuousButtonPressed) + { + MouseState |= MouseState.PressedOutside; + } + } + + if (!mouseEvent.Handled && WantContinuousButtonPressed && App?.Mouse.MouseGrabView == this) + { + // Ignore the return value here, because the semantics of WhenGrabbedHandlePressed is the return + // value indicates whether processing should stop or not. + RaiseCommandsBoundToMouse (mouseEvent); + + return true; + } + + return mouseEvent.Handled = true; } - /// - /// INTERNAL For cases where the view is grabbed and the mouse is clicked, this method handles the click event + /// INTERNAL: For cases where the view is grabbed, this method handles the released events from the driver /// (typically /// when or are set). /// - /// - /// Marked internal just to support unit tests - /// /// - /// , if the event was handled, otherwise. - internal bool WhenGrabbedHandleClicked (MouseEventArgs mouseEvent) + internal void WhenGrabbedHandleReleased (MouseEventArgs mouseEvent) { - mouseEvent.Handled = false; - - if (App?.Mouse.MouseGrabView == this && mouseEvent.IsSingleClicked) + if (App is { } && App.Mouse.MouseGrabView == this) { - // We're grabbed. Clicked event comes after the last Release. This is our signal to ungrab - App?.Mouse.UngrabMouse (); - - // TODO: Prove we need to unset MouseState.Pressed and MouseState.PressedOutside here - // TODO: There may be perf gains if we don't unset these flags here + //Logging.Debug ($"{Id} - {MouseState}"); MouseState &= ~MouseState.Pressed; MouseState &= ~MouseState.PressedOutside; - - // If mouse is still in bounds, generate a click - if (!WantMousePositionReports && Viewport.Contains (mouseEvent.Position)) - { - // By default, this will raise Selecting/OnSelecting - Subclasses can override this via AddCommand (Command.Select ...). - mouseEvent.Handled = InvokeCommandsBoundToMouse (mouseEvent) == true; - } - - return mouseEvent.Handled = true; } - - return false; } + /// + /// INTERNAL: For cases where the view is grabbed, this method handles the click events from the driver + /// (typically + /// when or are set). + /// + /// + /// , if processing should stop; otherwise. + internal bool WhenGrabbedHandleClicked (MouseEventArgs mouseEvent) + { + if (App is null || App.Mouse.MouseGrabView != this || !mouseEvent.IsSingleClicked) + { + return false; + } + + // Logging.Debug ($"{mouseEvent.Flags};{mouseEvent.Position}"); + + // We're grabbed. Clicked event comes after the last Release. This is our signal to ungrab + App?.Mouse.UngrabMouse (); + + // TODO: Prove we need to unset MouseState.Pressed and MouseState.PressedOutside here + // TODO: There may be perf gains if we don't unset these flags here + MouseState &= ~MouseState.Pressed; + MouseState &= ~MouseState.PressedOutside; + + // If mouse is still in bounds, return false to indicate a click should be raised. + return WantMousePositionReports || !Viewport.Contains (mouseEvent.Position); + } #endregion WhenGrabbed Handlers + #region Mouse Click Events + + /// + /// INTERNAL API: Converts mouse click events into s by invoking the commands bound + /// to the mouse button via . By default, all mouse clicks are bound to + /// which raises the event. + /// + protected bool RaiseCommandsBoundToMouse (MouseEventArgs args) + { + // Pre-conditions + if (!Enabled) + { + // QUESTION: Is this right? Should a disabled view eat mouse clicks? + return args.Handled = false; + } + + Debug.Assert (!args.Handled); + + // Logging.Debug ($"{args.Flags};{args.Position}"); + + MouseEventArgs clickedArgs = new (); + + clickedArgs.Flags = args.IsPressed + ? args.Flags switch + { + MouseFlags.Button1Pressed => MouseFlags.Button1Clicked, + MouseFlags.Button2Pressed => MouseFlags.Button2Clicked, + MouseFlags.Button3Pressed => MouseFlags.Button3Clicked, + MouseFlags.Button4Pressed => MouseFlags.Button4Clicked, + _ => clickedArgs.Flags + } + : args.Flags; + + clickedArgs.Position = args.Position; + clickedArgs.ScreenPosition = args.ScreenPosition; + clickedArgs.View = args.View; + + // By default, this will raise Activating/OnActivating - Subclasses can override this via + // ReplaceCommand (Command.Activate ...). + args.Handled = InvokeCommandsBoundToMouse (clickedArgs) == true; + + return args.Handled; + } + + #endregion Mouse Click Events + #region Mouse Wheel Events /// Raises the / event. @@ -601,18 +688,26 @@ public partial class View // Mouse APIs } /// - /// Called when has changed, indicating the View should be highlighted or not. The passed in the event + /// Called when has changed, indicating the View should be highlighted or not. The + /// passed in the event /// indicates the highlight style that will be applied. /// protected virtual void OnMouseStateChanged (EventArgs args) { } /// - /// RaisedCalled when has changed, indicating the View should be highlighted or not. The passed in the event + /// Raised when has changed, indicating the View should be highlighted or not. The + /// passed in the event /// indicates the highlight style that will be applied. /// public event EventHandler>? MouseStateChanged; #endregion MouseState Handling - private void DisposeMouse () { } + private void DisposeMouse () + { + if (App?.Mouse.MouseGrabView == this) + { + App.Mouse.UngrabMouse (); + } + } } diff --git a/Tests/UnitTestsParallelizable/ViewBase/Mouse/MouseTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Mouse/MouseTests.cs index 9fa3d824f..1659b40fb 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Mouse/MouseTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/Mouse/MouseTests.cs @@ -3,11 +3,20 @@ using Xunit.Abstractions; namespace ViewBaseTests.Mouse; - -[Collection ("Global Test Setup")] [Trait ("Category", "Input")] public class MouseTests (ITestOutputHelper output) : TestsAllViews { + [Fact] + public void Default_MouseBindings () + { + var testView = new View (); + + Assert.Contains (MouseFlags.Button1Clicked, testView.MouseBindings.GetAllFromCommands (Command.Select)); +// Assert.Contains (MouseFlags.Button1DoubleClicked, testView.MouseBindings.GetAllFromCommands (Command.Accept)); + + Assert.Equal (5, testView.MouseBindings.GetBindings ().Count ()); + } + [Theory] [InlineData (false, false, false)] [InlineData (true, false, true)] diff --git a/docfx/docs/mouse.md b/docfx/docs/mouse.md index 6c5aaed88..494f2a13b 100644 --- a/docfx/docs/mouse.md +++ b/docfx/docs/mouse.md @@ -126,12 +126,16 @@ Mouse events are processed through the following workflow using the [Cancellable 1. **Driver Level**: The driver captures platform-specific mouse events and converts them to `MouseEventArgs` 2. **Application Level**: `IApplication.Mouse.RaiseMouseEvent` determines the target view and routes the event -3. **View Level**: The target view processes the event through: - - `OnMouseEvent` (virtual method that can be overridden) - - `MouseEvent` event (for event subscribers) - - Mouse bindings (if the event wasn't handled) which invoke commands - - Command handlers (e.g., `OnSelecting` for `Command.Select`) - - High-level events like `MouseEnter`, `MouseLeave` +3. **View Level**: The target view processes the event through `View.NewMouseEvent()`: + 1. **Pre-condition validation** - Checks if view is enabled, visible, and wants the event type + 2. **Low-level MouseEvent** - Raises `OnMouseEvent()` and `MouseEvent` event + 3. **Mouse grab handling** - If `HighlightStates` or `WantContinuousButtonPressed` are set: + - Automatically grabs mouse on button press + - Handles press/release/click lifecycle + - Sets focus if view is focusable + - Updates `MouseState` (Pressed, PressedOutside) + 4. **Command invocation** - For click events, invokes commands via `MouseBindings` (default: `Command.Select` ? `Selecting` event) + 5. **Mouse wheel handling** - Raises `OnMouseWheel()` and `MouseWheel` event ### Handling Mouse Events Directly @@ -228,15 +232,17 @@ public class MultiButtonView : View } ``` -## Mouse State +## Mouse State and Mouse Grab + +### Mouse State The @Terminal.Gui.ViewBase.View.MouseState property provides an abstraction for the current state of the mouse, enabling views to do interesting things like change their appearance based on the mouse state. Mouse states include: -* **Normal** - Default state when mouse is not interacting with the view +* **None** - No mouse interaction with the view * **In** - Mouse is positioned over the view (inside the viewport) * **Pressed** - Mouse button is pressed down while over the view -* **PressedOutside** - Mouse was pressed inside but moved outside the view +* **PressedOutside** - Mouse was pressed inside but moved outside the view (when not using `WantContinuousButtonPressed`) It works in conjunction with the @Terminal.Gui.ViewBase.View.HighlightStates which is a list of mouse states that will cause a view to become highlighted. @@ -253,6 +259,9 @@ view.MouseStateChanged += (sender, e) => case MouseState.Pressed: // Change appearance when pressed break; + case MouseState.PressedOutside: + // Mouse was pressed inside but moved outside + break; } }; ``` @@ -264,6 +273,59 @@ Configure which states should cause highlighting: view.HighlightStates = MouseState.In | MouseState.Pressed; ``` +### Mouse Grab + +Views with `HighlightStates` or `WantContinuousButtonPressed` enabled automatically **grab the mouse** when a button is pressed. This means: + +1. **Automatic Grab**: The view receives all mouse events until the button is released, even if the mouse moves outside the view's `Viewport` +2. **Focus Management**: If the view is focusable (`CanFocus = true`), it automatically receives focus on the first button press +3. **State Tracking**: The view's `MouseState` is updated to reflect press/release/outside states +4. **Automatic Ungrab**: The mouse is released when: + - The button is released (via `WhenGrabbedHandleClicked()`) + - The view is removed from its parent hierarchy (via `View.OnRemoved()`) + - The application ends (via `App.End()`) + +#### Continuous Button Press + +When `WantContinuousButtonPressed` is set to `true`, the view receives repeated click events while the button is held down: + +```cs +view.WantContinuousButtonPressed = true; + +view.Selecting += (s, e) => +{ + // This will be called repeatedly while the button is held down + // Useful for scroll buttons, increment/decrement buttons, etc. + DoRepeatAction(); + e.Handled = true; +}; +``` + +**Note**: With `WantContinuousButtonPressed`, the `MouseState.PressedOutside` flag has no effect - the view continues to receive events and maintains the pressed state even when the mouse moves outside. + +#### Mouse Grab Lifecycle + +``` +Button Press (inside view) + ? +Mouse Grabbed Automatically + ?? View receives focus (if CanFocus) + ?? MouseState |= MouseState.Pressed + ?? All mouse events route to this view + +Mouse Move (while grabbed) + ?? Inside Viewport: MouseState remains Pressed + ?? Outside Viewport: MouseState |= MouseState.PressedOutside + (unless WantContinuousButtonPressed is true) + +Button Release + ? +Mouse Ungrabbed Automatically + ?? MouseState &= ~MouseState.Pressed + ?? MouseState &= ~MouseState.PressedOutside + ?? Click event raised (if still in bounds) +``` + ## Mouse Button and Movement Concepts * **Down** - Indicates the user pushed a mouse button down. @@ -355,12 +417,25 @@ view.MouseEvent += (s, e) => * **Use Mouse Bindings and Commands** for simple mouse interactions - they integrate well with the Command system and work alongside keyboard bindings * **Use the `Selecting` event** to handle mouse clicks - it's raised by the default `Command.Select` binding for all mouse buttons -* **Access mouse details via CommandContext** when you need position or flags in `Selecting` handlers -* **Handle Mouse Events directly** for complex interactions like drag-and-drop or custom gestures +* **Access mouse details via CommandContext** when you need position or flags in `Selecting` handlers: + ```cs + view.Selecting += (s, e) => + { + if (e.Context is CommandContext { Binding.MouseEventArgs: { } mouseArgs }) + { + Point position = mouseArgs.Position; + MouseFlags flags = mouseArgs.Flags; + // Handle with position and flags + } + }; + ``` +* **Handle Mouse Events directly** only for complex interactions like drag-and-drop or custom gestures (override `OnMouseEvent` or subscribe to `MouseEvent`) +* **Use `HighlightStates`** to enable automatic mouse grab and visual feedback - views will automatically grab the mouse and update their appearance +* **Use `WantContinuousButtonPressed`** for repeating actions (scroll buttons, increment/decrement) - the view will receive repeated events while the button is held * **Respect platform conventions** - use right-click for context menus, double-click for default actions * **Provide keyboard alternatives** - ensure all mouse functionality has keyboard equivalents * **Test with different terminals** - mouse support varies between terminal applications -* **Use Mouse State** to provide visual feedback when users hover or interact with views +* **Mouse grab is automatic** - you don't need to manually call `GrabMouse()`/`UngrabMouse()` when using `HighlightStates` or `WantContinuousButtonPressed` ## Limitations and Considerations