diff --git a/Terminal.Gui/Application/Application.Mouse.cs b/Terminal.Gui/Application/Application.Mouse.cs index 3af3808b2..b884eaefb 100644 --- a/Terminal.Gui/Application/Application.Mouse.cs +++ b/Terminal.Gui/Application/Application.Mouse.cs @@ -1,5 +1,6 @@ -#nullable enable +#nullable enable namespace Terminal.Gui; + public static partial class Application // Mouse handling { #region Mouse handling @@ -36,16 +37,13 @@ public static partial class Application // Mouse handling /// View that will receive all mouse events until is invoked. public static void GrabMouse (View? view) { - if (view is null) + if (view is null || OnGrabbingMouse (view)) { return; } - if (!OnGrabbingMouse (view)) - { - OnGrabbedMouse (view); - MouseGrabView = view; - } + OnGrabbedMouse (view); + MouseGrabView = view; } /// Releases the mouse grab, so mouse events will be routed to the view on which the mouse is. @@ -56,6 +54,10 @@ public static partial class Application // Mouse handling return; } +#if DEBUG_IDISPOSABLE + ObjectDisposedException.ThrowIf (MouseGrabView.WasDisposed, MouseGrabView); +#endif + if (!OnUnGrabbingMouse (MouseGrabView)) { View view = MouseGrabView; @@ -64,6 +66,7 @@ public static partial class Application // Mouse handling } } + /// A delegate callback throws an exception. private static bool OnGrabbingMouse (View? view) { if (view is null) @@ -77,6 +80,7 @@ public static partial class Application // Mouse handling return evArgs.Cancel; } + /// A delegate callback throws an exception. private static bool OnUnGrabbingMouse (View? view) { if (view is null) @@ -90,6 +94,7 @@ public static partial class Application // Mouse handling return evArgs.Cancel; } + /// A delegate callback throws an exception. private static void OnGrabbedMouse (View? view) { if (view is null) @@ -100,6 +105,7 @@ public static partial class Application // Mouse handling GrabbedMouse?.Invoke (view, new (view)); } + /// A delegate callback throws an exception. private static void OnUnGrabbedMouse (View? view) { if (view is null) @@ -110,8 +116,6 @@ public static partial class Application // Mouse handling UnGrabbedMouse?.Invoke (view, new (view)); } -#nullable enable - // Used by OnMouseEvent to track the last view that was clicked on. internal static View? MouseEnteredView { get; set; } @@ -166,51 +170,49 @@ public static partial class Application // Mouse handling if ((MouseGrabView.Viewport with { Location = Point.Empty }).Contains (viewRelativeMouseEvent.Position) is false) { // The mouse has moved outside the bounds of the view that grabbed the mouse - MouseGrabView?.NewMouseLeaveEvent (mouseEvent); + MouseGrabView.NewMouseLeaveEvent (mouseEvent); } //System.Diagnostics.Debug.WriteLine ($"{nme.Flags};{nme.X};{nme.Y};{mouseGrabView}"); - if (MouseGrabView?.NewMouseEvent (viewRelativeMouseEvent) == true) + if (MouseGrabView.NewMouseEvent (viewRelativeMouseEvent) is true) { return; } } - if (view is { WantContinuousButtonPressed: true }) - { - WantContinuousButtonPressedView = view; - } - else - { - WantContinuousButtonPressedView = null; - } + // We can combine this into the switch expression to reduce cognitive complexity even more and likely + // avoid one or two of these checks in the process, as well. + WantContinuousButtonPressedView = view switch + { + { WantContinuousButtonPressed: true } => view, + _ => null + }; - if (view is not Adornment) + if (view is not Adornment + && (view is null || view == ApplicationOverlapped.OverlappedTop) + && Current is { Modal: false } + && ApplicationOverlapped.OverlappedTop != null + && mouseEvent.Flags is not MouseFlags.ReportMousePosition and not 0) { - if ((view is null || view == ApplicationOverlapped.OverlappedTop) - && Current is { Modal: false } - && ApplicationOverlapped.OverlappedTop != null - && mouseEvent.Flags != MouseFlags.ReportMousePosition - && mouseEvent.Flags != 0) + // This occurs when there are multiple overlapped "tops" + // E.g. "Mdi" - in the Background Worker Scenario + View? top = ApplicationOverlapped.FindDeepestTop (Top!, mouseEvent.Position); + view = View.FindDeepestView (top, mouseEvent.Position); + + if (view is { } && view != ApplicationOverlapped.OverlappedTop && top != Current && top is { }) { - // This occurs when there are multiple overlapped "tops" - // E.g. "Mdi" - in the Background Worker Scenario - View? top = ApplicationOverlapped.FindDeepestTop (Top!, mouseEvent.Position); - view = View.FindDeepestView (top, mouseEvent.Position); - - if (view is { } && view != ApplicationOverlapped.OverlappedTop && top != Current && top is { }) - { - ApplicationOverlapped.MoveCurrent ((Toplevel)top); - } + ApplicationOverlapped.MoveCurrent ((Toplevel)top); } } + // May be null before the prior condition or the condition may set it as null. + // So, the checking must be outside the prior condition. if (view is null) { return; } - MouseEvent? me = null; + MouseEvent? me; if (view is Adornment adornment) { @@ -236,8 +238,7 @@ public static partial class Application // Mouse handling View = view }; } - - if (me is null) + else { return; } @@ -263,13 +264,8 @@ public static partial class Application // Mouse handling //Debug.WriteLine ($"OnMouseEvent: ({a.MouseEvent.X},{a.MouseEvent.Y}) - {a.MouseEvent.Flags}"); - while (view.NewMouseEvent (me) != true) + while (view.NewMouseEvent (me) is not true && MouseGrabView is not { }) { - if (MouseGrabView is { }) - { - break; - } - if (view is Adornment adornmentView) { view = adornmentView.Parent!.SuperView; diff --git a/Terminal.Gui/Views/Menu/ContextMenu.cs b/Terminal.Gui/Views/Menu/ContextMenu.cs index e2b2127ac..1158eec3a 100644 --- a/Terminal.Gui/Views/Menu/ContextMenu.cs +++ b/Terminal.Gui/Views/Menu/ContextMenu.cs @@ -105,18 +105,17 @@ public sealed class ContextMenu : IDisposable /// Disposes the context menu object. public void Dispose () { - if (IsShow) - { - _menuBar.MenuAllClosed -= MenuBar_MenuAllClosed; - _menuBar.Dispose (); - _menuBar = null; - IsShow = false; - } + _menuBar.MenuAllClosed -= MenuBar_MenuAllClosed; + Application.UngrabMouse (); + _menuBar?.Dispose (); + _menuBar = null; + IsShow = false; if (_container is { }) { _container.Closing -= Container_Closing; _container.Deactivate -= Container_Deactivate; + _container.Disposing -= Container_Disposing; } } @@ -124,7 +123,7 @@ public sealed class ContextMenu : IDisposable public void Hide () { _menuBar?.CleanUp (); - Dispose (); + IsShow = false; } /// Event invoked when the is changed. @@ -139,11 +138,13 @@ public sealed class ContextMenu : IDisposable if (_menuBar is { }) { Hide (); + Dispose (); } _container = Application.Current; - _container.Closing += Container_Closing; + _container!.Closing += Container_Closing; _container.Deactivate += Container_Deactivate; + _container.Disposing += Container_Disposing; Rectangle frame = Application.Screen; Point position = Position; @@ -219,7 +220,9 @@ public sealed class ContextMenu : IDisposable _menuBar.OpenMenu (); } - private void Container_Deactivate (object sender, ToplevelEventArgs e) { Hide (); } private void Container_Closing (object sender, ToplevelClosingEventArgs obj) { Hide (); } - private void MenuBar_MenuAllClosed (object sender, EventArgs e) { Dispose (); } + private void Container_Deactivate (object sender, ToplevelEventArgs e) { Hide (); } + private void Container_Disposing (object sender, EventArgs e) { Dispose (); } + + private void MenuBar_MenuAllClosed (object sender, EventArgs e) { Hide (); } } diff --git a/Terminal.Gui/Views/Menu/MenuBar.cs b/Terminal.Gui/Views/Menu/MenuBar.cs index 22b30ea86..65ce66ecd 100644 --- a/Terminal.Gui/Views/Menu/MenuBar.cs +++ b/Terminal.Gui/Views/Menu/MenuBar.cs @@ -479,6 +479,16 @@ public class MenuBar : View, IDesignable } SetNeedsDisplay (); + + if (Application.MouseGrabView is { } && Application.MouseGrabView is MenuBar && Application.MouseGrabView != this) + { + var menuBar = Application.MouseGrabView as MenuBar; + + if (menuBar!.IsMenuOpen) + { + menuBar.CleanUp (); + } + } Application.UngrabMouse (); _isCleaning = false; } diff --git a/Terminal.sln.DotSettings b/Terminal.sln.DotSettings index 0142f001b..5c8380e3d 100644 --- a/Terminal.sln.DotSettings +++ b/Terminal.sln.DotSettings @@ -406,6 +406,7 @@ True True True + True True True diff --git a/UICatalog/Scenarios/ContextMenus.cs b/UICatalog/Scenarios/ContextMenus.cs index c0ae3cb80..e271ccd69 100644 --- a/UICatalog/Scenarios/ContextMenus.cs +++ b/UICatalog/Scenarios/ContextMenus.cs @@ -93,9 +93,22 @@ public class ContextMenus : Scenario Application.MouseEvent -= ApplicationMouseEvent; }; + var menu = new MenuBar + { + Menus = + [ + new ( + "File", + new MenuItem [] { new ("Quit", "", () => Application.RequestStop (), null, null, Application.QuitKey) }) + ] + }; + + var top = new Toplevel (); + top.Add (appWindow, menu); + // Run - Start the application. - Application.Run (appWindow); - appWindow.Dispose (); + Application.Run (top); + top.Dispose (); // Shutdown - Calling Application.Shutdown is required. Application.Shutdown (); diff --git a/UnitTests/Views/ContextMenuTests.cs b/UnitTests/Views/ContextMenuTests.cs index 99f9e3dfe..57dcfebe3 100644 --- a/UnitTests/Views/ContextMenuTests.cs +++ b/UnitTests/Views/ContextMenuTests.cs @@ -390,7 +390,8 @@ public class ContextMenuTests (ITestOutputHelper output) Assert.True (Application.OnKeyDown (ContextMenu.DefaultKey)); Assert.True (tf.ContextMenu.MenuBar.IsMenuOpen); Assert.True (Application.OnKeyDown (ContextMenu.DefaultKey)); - Assert.Null (tf.ContextMenu.MenuBar); + // The last context menu bar opened is always preserved + Assert.NotNull (tf.ContextMenu.MenuBar); top.Dispose (); } @@ -1390,7 +1391,8 @@ public class ContextMenuTests (ITestOutputHelper output) Assert.True (tf1.HasFocus); Assert.False (tf2.HasFocus); Assert.Equal (2, win.Subviews.Count); - Assert.Null (tf2.ContextMenu.MenuBar); + // The last context menu bar opened is always preserved + Assert.NotNull (tf2.ContextMenu.MenuBar); Assert.Equal (win.Focused, tf1); Assert.Null (Application.MouseGrabView); Assert.Equal (tf1, Application.MouseEnteredView); @@ -1400,7 +1402,8 @@ public class ContextMenuTests (ITestOutputHelper output) Assert.False (tf1.HasFocus); Assert.True (tf2.HasFocus); Assert.Equal (2, win.Subviews.Count); - Assert.Null (tf2.ContextMenu.MenuBar); + // The last context menu bar opened is always preserved + Assert.NotNull (tf2.ContextMenu.MenuBar); Assert.Equal (win.Focused, tf2); Assert.Null (Application.MouseGrabView); Assert.Equal (tf2, Application.MouseEnteredView);