Tig 11 months ago
parent
commit
c39318c521

+ 39 - 50
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
     /// <param name="view">View that will receive all mouse events until <see cref="UngrabMouse"/> is invoked.</param>
     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;
     }
 
     /// <summary>Releases the mouse grab, so mouse events will be routed to the view on which the mouse is.</summary>
@@ -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
         }
     }
 
+    /// <exception cref="Exception">A delegate callback throws an exception.</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;
     }
 
+    /// <exception cref="Exception">A delegate callback throws an exception.</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;
     }
 
+    /// <exception cref="Exception">A delegate callback throws an exception.</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));
     }
 
+    /// <exception cref="Exception">A delegate callback throws an exception.</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; }
 
@@ -179,58 +183,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 DEBUG_IDISPOSABLE
-            if (MouseGrabView.WasDisposed)
-            {
-                throw new ObjectDisposedException (MouseGrabView.GetType ().FullName);
-            }
-#endif
-            if (MouseGrabView?.NewMouseEvent (viewRelativeMouseEvent) == true)
+            if (MouseGrabView.NewMouseEvent (viewRelativeMouseEvent) is true)
             {
                 return;
             }
         }
 
-        if (view is { WantContinuousButtonPressed: true })
+        // 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
+         && (view is null || view == ApplicationOverlapped.OverlappedTop)
+         && Current is { Modal: false }
+         && ApplicationOverlapped.OverlappedTop != null
+         && mouseEvent.Flags is not MouseFlags.ReportMousePosition and not 0)
         {
-            WantContinuousButtonPressedView = view;
-        }
-        else
-        {
-            WantContinuousButtonPressedView = null;
-        }
+            // 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 not Adornment)
-        {
-            if ((view is null || view == ApplicationOverlapped.OverlappedTop)
-                && Current is { Modal: false }
-                && ApplicationOverlapped.OverlappedTop != null
-                && mouseEvent.Flags != MouseFlags.ReportMousePosition
-                && mouseEvent.Flags != 0)
+            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)
         {
@@ -256,8 +251,7 @@ public static partial class Application // Mouse handling
                 View = view
             };
         }
-
-        if (me is null)
+        else
         {
             return;
         }
@@ -283,13 +277,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;

+ 17 - 18
Terminal.Gui/View/View.Navigation.cs

@@ -41,12 +41,6 @@ public partial class View // Focus and cross-view navigation management (TabStop
     {
         set
         {
-#if DEBUG_IDISPOSABLE
-            if (WasDisposed)
-            {
-                throw new ObjectDisposedException (GetType ().FullName);
-            }
-#endif
             if (HasFocus != value)
             {
                 if (value)
@@ -67,12 +61,6 @@ public partial class View // Focus and cross-view navigation management (TabStop
         }
         get
         {
-#if DEBUG_IDISPOSABLE
-            if (WasDisposed)
-            {
-                throw new ObjectDisposedException (GetType ().FullName);
-            }
-#endif
             return _hasFocus;
         }
     }
@@ -427,19 +415,30 @@ public partial class View // Focus and cross-view navigation management (TabStop
 
         if (focusedIndex < index.Length - 1)
         {
+            // We're moving w/in the subviews
             next = focusedIndex + 1;
         }
         else
         {
-            if (behavior == TabBehavior.TabGroup && behavior == TabStop && SuperView?.TabStop == TabBehavior.TabGroup)
+            if (SuperView is { })
             {
-                // Go down the subview-hierarchy and leave
-                // BUGBUG: This doesn't seem right
-                Focused.HasFocus = false;
+                return false;
+            }
+
+            // We're moving beyond the last subview
+            // If our superview 
+            //if (behavior == TabBehavior.TabGroup && behavior == TabStop && SuperView?.TabStop == TabBehavior.TabGroup)
+            {
+                next = 0;
+                //return 
 
-                // TODO: Should we check the return value of SetHasFocus?
+                //// Go down the subview-hierarchy and leave
+                //// BUGBUG: This doesn't seem right
+                //Focused.HasFocus = false;
 
-                return false;
+                //// TODO: Should we check the return value of SetHasFocus?
+
+                //return false;
             }
         }
 

+ 1 - 1
Terminal.Gui/Views/ColorBar.cs

@@ -121,7 +121,7 @@ internal abstract class ColorBar : View, IColorBar
             }
 
             mouseEvent.Handled = true;
-            FocusFirst (null);
+            RestoreFocus (null);
 
             return true;
         }

+ 4 - 0
Terminal.Gui/Views/ColorModelStrategy.cs

@@ -43,6 +43,10 @@ internal class ColorModelStrategy
 
     public void SetBarsToColor (IList<IColorBar> bars, Color newValue, ColorModel model)
     {
+        if (bars.Count == 0)
+        {
+            return;
+        }
         switch (model)
         {
             case ColorModel.RGB:

+ 26 - 10
Terminal.Gui/Views/ColorPicker.cs

@@ -15,6 +15,7 @@ public class ColorPicker : View
     public ColorPicker ()
     {
         CanFocus = true;
+        TabStop = TabBehavior.TabStop;
         Height = Dim.Auto ();
         Width = Dim.Auto ();
         ApplyStyleChanges ();
@@ -59,7 +60,7 @@ public class ColorPicker : View
                     Y = y,
                     Width = textFieldWidth
                 };
-                tfValue.Leave += UpdateSingleBarValueFromTextField;
+                tfValue.HasFocusChanged += UpdateSingleBarValueFromTextField;
                 _textFields.Add (bar, tfValue);
                 Add (tfValue);
             }
@@ -141,7 +142,7 @@ public class ColorPicker : View
         };
         _tfName.Autocomplete = auto;
 
-        _tfName.Leave += UpdateValueFromName;
+        _tfName.HasFocusChanged += UpdateValueFromName;
     }
 
     private void CreateTextField ()
@@ -164,13 +165,13 @@ public class ColorPicker : View
         {
             Y = y,
             X = 4,
-            Width = 8
+            Width = 8,
         };
 
         Add (_lbHex);
         Add (_tfHex);
 
-        _tfHex.Leave += UpdateValueFromTextField;
+        _tfHex.HasFocusChanged += UpdateValueFromTextField;
     }
 
     private void DisposeOldViews ()
@@ -181,7 +182,7 @@ public class ColorPicker : View
 
             if (_textFields.TryGetValue (bar, out TextField? tf))
             {
-                tf.Leave -= UpdateSingleBarValueFromTextField;
+                tf.HasFocusChanged -= UpdateSingleBarValueFromTextField;
                 Remove (tf);
                 tf.Dispose ();
             }
@@ -203,7 +204,7 @@ public class ColorPicker : View
         if (_tfHex != null)
         {
             Remove (_tfHex);
-            _tfHex.Leave -= UpdateValueFromTextField;
+            _tfHex.HasFocusChanged -= UpdateValueFromTextField;
             _tfHex.Dispose ();
             _tfHex = null;
         }
@@ -218,7 +219,7 @@ public class ColorPicker : View
         if (_tfName != null)
         {
             Remove (_tfName);
-            _tfName.Leave -= UpdateValueFromName;
+            _tfName.HasFocusChanged -= UpdateValueFromName;
             _tfName.Dispose ();
             _tfName = null;
         }
@@ -266,8 +267,13 @@ public class ColorPicker : View
         }
     }
 
-    private void UpdateSingleBarValueFromTextField (object? sender, FocusEventArgs e)
+    private void UpdateSingleBarValueFromTextField (object? sender, HasFocusEventArgs e)
     {
+        if (e.NewValue)
+        {
+            return;
+        }
+
         foreach (KeyValuePair<IColorBar, TextField> kvp in _textFields)
         {
             if (kvp.Value == sender)
@@ -280,8 +286,13 @@ public class ColorPicker : View
         }
     }
 
-    private void UpdateValueFromName (object? sender, FocusEventArgs e)
+    private void UpdateValueFromName (object? sender, HasFocusEventArgs e)
     {
+        if (e.NewValue)
+        {
+            return;
+        }
+
         if (_tfName == null)
         {
             return;
@@ -298,8 +309,13 @@ public class ColorPicker : View
         }
     }
 
-    private void UpdateValueFromTextField (object? sender, FocusEventArgs e)
+    private void UpdateValueFromTextField (object? sender, HasFocusEventArgs e)
     {
+        if (e.NewValue)
+        {
+            return;
+        }
+
         if (_tfHex == null)
         {
             return;

+ 4 - 1
Terminal.Gui/Views/Label.cs

@@ -28,7 +28,10 @@ public class Label : View
 
     private void Label_MouseClick (object sender, MouseEventEventArgs e)
     {
-        e.Handled = InvokeCommand (Command.HotKey) == true;
+        if (!CanFocus)
+        {
+            e.Handled = InvokeCommand (Command.HotKey) == true;
+        }
     }
 
     private void Label_TitleChanged (object sender, EventArgs<string> e)

+ 15 - 25
Terminal.Gui/Views/Menu/ContextMenu.cs

@@ -105,35 +105,25 @@ public sealed class ContextMenu : IDisposable
     /// <summary>Disposes the context menu object.</summary>
     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;
         }
     }
 
     /// <summary>Hides (closes) the ContextMenu.</summary>
-    public void Hide (bool dispose = false)
+    public void Hide ()
     {
-        if (_menuBar is { })
-        {
-            //_menuBar?.CleanUp ();
-            _menuBar.Enabled = false;
-
-            if (dispose)
-            {
-                _menuBar.Dispose ();
-                _menuBar = null;
-            }
-        }
+        _menuBar?.CleanUp ();
+        IsShow = false;
     }
 
     /// <summary>Event invoked when the <see cref="ContextMenu.Key"/> is changed.</summary>
@@ -148,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;
 
@@ -228,11 +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 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 (true);
-    }
+    private void MenuBar_MenuAllClosed (object sender, EventArgs e) { Hide (); }
 }

+ 137 - 134
Terminal.Gui/Views/Menu/MenuBar.cs

@@ -68,7 +68,6 @@ public class MenuBar : View, IDesignable
     {
         MenuItem._menuBar = this;
 
-        CanFocus = true;
         TabStop = TabBehavior.NoStop;
         X = 0;
         Y = 0;
@@ -86,10 +85,45 @@ public class MenuBar : View, IDesignable
         Added += MenuBar_Added;
 
         // Things this view knows how to do
-        AddCommand (Command.Left, () => MoveLeft ());
-        AddCommand (Command.Right, () => MoveRight ());
-        AddCommand (Command.Cancel, () => CloseMenuBar ());
-        AddCommand (Command.Accept, () => _selected >= 0 && ProcessMenu (_selected, Menus [_selected]));
+        AddCommand (
+                    Command.Left,
+                    () =>
+                    {
+                        MoveLeft ();
+
+                        return true;
+                    }
+                   );
+
+        AddCommand (
+                    Command.Right,
+                    () =>
+                    {
+                        MoveRight ();
+
+                        return true;
+                    }
+                   );
+
+        AddCommand (
+                    Command.Cancel,
+                    () =>
+                    {
+                        CloseMenuBar ();
+
+                        return true;
+                    }
+                   );
+
+        AddCommand (
+                    Command.Accept,
+                    () =>
+                    {
+                        ProcessMenu (_selected, Menus [_selected]);
+
+                        return true;
+                    }
+                   );
         AddCommand (Command.ToggleExpandCollapse, ctx => Select (Menus.IndexOf (ctx.KeyBinding?.Context)));
         AddCommand (Command.Select, ctx => Run ((ctx.KeyBinding?.Context as MenuItem)?.Action));
 
@@ -115,6 +149,9 @@ public class MenuBar : View, IDesignable
     /// <summary><see langword="true"/> if the menu is open; otherwise <see langword="true"/>.</summary>
     public bool IsMenuOpen { get; protected set; }
 
+    /// <summary>Gets the view that was last focused before opening the menu.</summary>
+    public View LastFocused { get; private set; }
+
     /// <summary>
     ///     Gets or sets the array of <see cref="MenuBarItem"/>s for the menu. Only set this after the
     ///     <see cref="MenuBar"/> is visible.
@@ -187,6 +224,7 @@ public class MenuBar : View, IDesignable
             if (value && UseKeysUpDownAsKeysLeftRight)
             {
                 _useKeysUpDownAsKeysLeftRight = false;
+                SetNeedsDisplay ();
             }
         }
     }
@@ -336,7 +374,7 @@ public class MenuBar : View, IDesignable
     }
 
     /// <summary>Opens the Menu programatically, as though the F9 key were pressed.</summary>
-    public bool OpenMenu ()
+    public void OpenMenu ()
     {
         MenuBar mbar = GetMouseGrabViewInstance (this);
 
@@ -347,12 +385,13 @@ public class MenuBar : View, IDesignable
 
         if (!Enabled || _openMenu is { })
         {
-            return false;
+            return;
         }
 
         _selected = 0;
+        SetNeedsDisplay ();
 
-        _previousFocused = Application.Navigation?.GetFocused ();
+        _previousFocused = SuperView is null ? Application.Current?.Focused : SuperView.Focused;
         OpenMenu (_selected);
 
         if (!SelectEnabledItem (
@@ -362,17 +401,15 @@ public class MenuBar : View, IDesignable
                                )
             && !CloseMenu (false))
         {
-            return IsMenuOpen;
+            return;
         }
 
         if (!OpenCurrentMenu.CheckSubMenu ())
         {
-            return IsMenuOpen;
+            return;
         }
 
         Application.GrabMouse (this);
-
-        return IsMenuOpen;
     }
 
     /// <inheritdoc/>
@@ -408,17 +445,18 @@ public class MenuBar : View, IDesignable
 
     // Activates the menu, handles either first focus, or activating an entry when it was already active
     // For mouse events.
-    internal bool Activate (int idx, int sIdx = -1, MenuBarItem subMenu = null)
+    internal void Activate (int idx, int sIdx = -1, MenuBarItem subMenu = null)
     {
         _selected = idx;
         _selectedSub = sIdx;
 
         if (_openMenu is null)
         {
-            _previousFocused = Application.Navigation?.GetFocused ();
+            _previousFocused = SuperView is null ? Application.Current?.Focused ?? null : SuperView.Focused;
         }
 
-        return OpenMenu (idx, sIdx, subMenu);
+        OpenMenu (idx, sIdx, subMenu);
+        SetNeedsDisplay ();
     }
 
     internal void CleanUp ()
@@ -440,6 +478,17 @@ public class MenuBar : View, IDesignable
             _lastFocused.SetFocus ();
         }
 
+        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;
     }
@@ -458,23 +507,16 @@ public class MenuBar : View, IDesignable
                 return;
             }
 
-#if DEBUG_IDISPOSABLE
-            if (WasDisposed)
+            if (LastFocused is { } && LastFocused != this)
             {
-                throw new ObjectDisposedException (GetType ().FullName);
+                _selected = -1;
             }
-#endif
-
-            _selected = -1;
-            Enabled = false;
 
             Application.UngrabMouse ();
         }
 
-        if (OpenCurrentMenu is { } && OpenCurrentMenu is Menu)
+        if (OpenCurrentMenu is { })
         {
-            OpenCurrentMenu.HasFocus = false;
-            OpenCurrentMenu.Dispose ();
             OpenCurrentMenu = null;
         }
 
@@ -517,42 +559,63 @@ public class MenuBar : View, IDesignable
                     Application.Current?.Remove (_openMenu);
                 }
 
-                if (_previousFocused is Menu && _openMenu is { } && (OpenCurrentMenu is {} && _previousFocused.ToString () != OpenCurrentMenu.ToString ()))
+                SetNeedsDisplay ();
+
+                if (_previousFocused is Menu && _openMenu is { } && _previousFocused.ToString () != OpenCurrentMenu.ToString ())
                 {
                     _previousFocused.SetFocus ();
                 }
 
                 _openMenu?.Dispose ();
-                if (_openMenu == OpenCurrentMenu)
+                _openMenu = null;
+
+                if (_lastFocused is Menu or MenuBar)
                 {
-                    OpenCurrentMenu = null;
+                    _lastFocused = null;
                 }
-                _openMenu = null;
 
-                if (_openSubMenu is null || _openSubMenu.Count == 0)
+                LastFocused = _lastFocused;
+                _lastFocused = null;
+
+                if (LastFocused is { CanFocus: true })
                 {
-                    CloseAllMenus ();
+                    if (!reopen)
+                    {
+                        _selected = -1;
+                    }
 
-                    return false;
+                    if (_openSubMenu is { })
+                    {
+                        _openSubMenu = null;
+                    }
+
+                    if (OpenCurrentMenu is { })
+                    {
+                        Application.Current?.Remove (OpenCurrentMenu);
+                        OpenCurrentMenu.Dispose ();
+                        OpenCurrentMenu = null;
+                    }
+
+                    LastFocused.SetFocus ();
+                }
+                else if (_openSubMenu is null || _openSubMenu.Count == 0)
+                {
+                    CloseAllMenus ();
                 }
                 else
                 {
                     SetFocus ();
                 }
 
-#if DEBUG_IDISPOSABLE
-                if (WasDisposed)
-                {
-                    throw new ObjectDisposedException (GetType ().FullName);
-                }
-#endif
                 IsMenuOpen = false;
+
                 break;
 
             case true:
                 _selectedSub = -1;
+                SetNeedsDisplay ();
                 RemoveAllOpensSubMenus ();
-                OpenCurrentMenu?._previousSubFocused.SetFocus ();
+                OpenCurrentMenu._previousSubFocused.SetFocus ();
                 _openSubMenu = null;
                 IsMenuOpen = true;
 
@@ -562,12 +625,6 @@ public class MenuBar : View, IDesignable
         _reopen = false;
         _isMenuClosing = false;
 
-#if DEBUG_IDISPOSABLE
-        if (WasDisposed)
-        {
-            throw new ObjectDisposedException (GetType ().FullName);
-        }
-#endif
         return true;
     }
 
@@ -597,7 +654,7 @@ public class MenuBar : View, IDesignable
                    );
     }
 
-    internal bool NextMenu (bool isSubMenu = false, bool ignoreUseSubMenusSingleFrame = false)
+    internal void NextMenu (bool isSubMenu = false, bool ignoreUseSubMenusSingleFrame = false)
     {
         switch (isSubMenu)
         {
@@ -617,7 +674,7 @@ public class MenuBar : View, IDesignable
 
                 if (_selected > -1 && !CloseMenu (true, ignoreUseSubMenusSingleFrame))
                 {
-                    return false;
+                    return;
                 }
 
                 OpenMenu (_selected);
@@ -649,7 +706,7 @@ public class MenuBar : View, IDesignable
                     {
                         if (_openSubMenu is { } && !CloseMenu (false, true))
                         {
-                            return IsMenuOpen;
+                            return;
                         }
 
                         NextMenu (false, ignoreUseSubMenusSingleFrame);
@@ -667,12 +724,14 @@ public class MenuBar : View, IDesignable
                     {
                         if (CloseMenu (false, true, ignoreUseSubMenusSingleFrame))
                         {
-                            return NextMenu (false, ignoreUseSubMenusSingleFrame);
+                            NextMenu (false, ignoreUseSubMenusSingleFrame);
                         }
 
-                        return IsMenuOpen;
+                        return;
                     }
 
+                    SetNeedsDisplay ();
+
                     if (UseKeysUpDownAsKeysLeftRight)
                     {
                         OpenCurrentMenu.CheckSubMenu ();
@@ -681,11 +740,9 @@ public class MenuBar : View, IDesignable
 
                 break;
         }
-
-        return IsMenuOpen;
     }
 
-    internal bool OpenMenu (int index, int sIndex = -1, MenuBarItem subMenu = null)
+    internal void OpenMenu (int index, int sIndex = -1, MenuBarItem subMenu = null)
     {
         _isMenuOpening = true;
         MenuOpeningEventArgs newMenu = OnMenuOpening (Menus [index]);
@@ -694,7 +751,7 @@ public class MenuBar : View, IDesignable
         {
             _isMenuOpening = false;
 
-            return false;
+            return;
         }
 
         if (newMenu.NewMenuBarItem is { })
@@ -708,22 +765,17 @@ public class MenuBar : View, IDesignable
         {
             case null:
                 // Open a submenu below a MenuBar
-                _lastFocused = null;//Application.Navigation.GetFocused();// ??= SuperView is null ? Application.Current?.MostFocused : SuperView.MostFocused;
+                _lastFocused ??= SuperView is null ? Application.Current?.MostFocused : SuperView.MostFocused;
 
                 if (_openSubMenu is { } && !CloseMenu (false, true))
                 {
-                    return IsMenuOpen;
+                    return;
                 }
 
                 if (_openMenu is { })
                 {
                     Application.Current?.Remove (_openMenu);
                     _openMenu.Dispose ();
-
-                    if (_openMenu == OpenCurrentMenu)
-                    {
-                        OpenCurrentMenu = null;
-                    }
                     _openMenu = null;
                 }
 
@@ -853,11 +905,9 @@ public class MenuBar : View, IDesignable
 
         _isMenuOpening = false;
         IsMenuOpen = true;
-
-        return true;
     }
 
-    internal bool PreviousMenu (bool isSubMenu = false, bool ignoreUseSubMenusSingleFrame = false)
+    internal void PreviousMenu (bool isSubMenu = false, bool ignoreUseSubMenusSingleFrame = false)
     {
         switch (isSubMenu)
         {
@@ -873,10 +923,10 @@ public class MenuBar : View, IDesignable
 
                 if (_selected > -1 && !CloseMenu (true, false, ignoreUseSubMenusSingleFrame))
                 {
-                    return true;
+                    return;
                 }
 
-                bool opened = OpenMenu (_selected);
+                OpenMenu (_selected);
 
                 if (!SelectEnabledItem (
                                         OpenCurrentMenu.BarItems.Children,
@@ -888,23 +938,21 @@ public class MenuBar : View, IDesignable
                     OpenCurrentMenu._currentChild = 0;
                 }
 
-                return opened;
-
+                break;
             case true:
                 if (_selectedSub > -1)
                 {
                     _selectedSub--;
                     RemoveSubMenu (_selectedSub, ignoreUseSubMenusSingleFrame);
+                    SetNeedsDisplay ();
                 }
                 else
                 {
-                    return PreviousMenu ();
+                    PreviousMenu ();
                 }
 
                 break;
         }
-
-        return true;
     }
 
     internal void RemoveAllOpensSubMenus ()
@@ -914,11 +962,6 @@ public class MenuBar : View, IDesignable
             foreach (Menu item in _openSubMenu)
             {
                 Application.Current.Remove (item);
-
-                if (item == OpenCurrentMenu)
-                {
-                    OpenCurrentMenu = null;
-                }
                 item.Dispose ();
             }
         }
@@ -1030,19 +1073,20 @@ public class MenuBar : View, IDesignable
         return Run (item?.Action);
     }
 
-    private bool CloseMenuBar ()
+    private void CloseMenuBar ()
     {
         if (!CloseMenu (false))
         {
-            return false;
+            return;
         }
 
         if (_openedByAltKey)
         {
             _openedByAltKey = false;
+            LastFocused?.SetFocus ();
         }
 
-        return true;
+        SetNeedsDisplay ();
     }
 
     private Point GetLocationOffset ()
@@ -1061,7 +1105,7 @@ public class MenuBar : View, IDesignable
         Added -= MenuBar_Added;
     }
 
-    private bool MoveLeft ()
+    private void MoveLeft ()
     {
         _selected--;
 
@@ -1070,30 +1114,22 @@ public class MenuBar : View, IDesignable
             _selected = Menus.Length - 1;
         }
 
-        if (_selected < 0)
-        {
-            return false;
-        }
-
-        return OpenMenu (_selected);
+        OpenMenu (_selected);
+        SetNeedsDisplay ();
     }
 
-    private bool MoveRight ()
+    private void MoveRight ()
     {
-        if (Menus.Length == 0)
-        {
-            return false;
-        }
         _selected = (_selected + 1) % Menus.Length;
-
-        return OpenMenu (_selected);
+        OpenMenu (_selected);
+        SetNeedsDisplay ();
     }
 
-    private bool ProcessMenu (int i, MenuBarItem mi)
+    private void ProcessMenu (int i, MenuBarItem mi)
     {
         if (_selected < 0 && IsMenuOpen)
         {
-            return false;
+            return;
         }
 
         if (mi.IsTopLevel)
@@ -1101,18 +1137,13 @@ public class MenuBar : View, IDesignable
             Point screen = ViewportToScreen (new Point (0, i));
             var menu = new Menu { Host = this, X = screen.X, Y = screen.Y, BarItems = mi };
             menu.Run (mi.Action);
-
-            if (menu == OpenCurrentMenu)
-            {
-                OpenCurrentMenu = null;
-            }
             menu.Dispose ();
         }
         else
         {
             Application.GrabMouse (this);
             _selected = i;
-            bool opened = OpenMenu (i);
+            OpenMenu (i);
 
             if (!SelectEnabledItem (
                                     OpenCurrentMenu.BarItems.Children,
@@ -1121,17 +1152,16 @@ public class MenuBar : View, IDesignable
                                    )
                 && !CloseMenu (false))
             {
-                return false;
+                return;
             }
 
             if (!OpenCurrentMenu.CheckSubMenu ())
             {
-                return true;
+                return;
             }
-            return opened;
         }
 
-        return true;
+        SetNeedsDisplay ();
     }
 
     private void RemoveSubMenu (int index, bool ignoreUseSubMenusSingleFrame = false)
@@ -1178,10 +1208,6 @@ public class MenuBar : View, IDesignable
                 }
 
                 menu.Dispose ();
-                if (menu == OpenCurrentMenu)
-                {
-                    OpenCurrentMenu = null;
-                }
             }
 
             RemoveSubMenu (i, ignoreUseSubMenusSingleFrame);
@@ -1240,6 +1266,7 @@ public class MenuBar : View, IDesignable
             if (value && UseSubMenusSingleFrame)
             {
                 UseSubMenusSingleFrame = false;
+                SetNeedsDisplay ();
             }
         }
     }
@@ -1325,22 +1352,11 @@ public class MenuBar : View, IDesignable
     /// <inheritdoc/>
     protected internal override bool OnMouseEvent (MouseEvent me)
     {
-        if (!HandleGrabView (me, this))
-        {
-            return false;
-        }
-
-        if (!IsMenuOpen)
-        {
-            return false;
-        }
-
-        if (!_handled)
+        if (!_handled && !HandleGrabView (me, this))
         {
             return false;
         }
 
-
         _handled = false;
 
         if (me.Flags == MouseFlags.Button1Pressed
@@ -1373,10 +1389,6 @@ public class MenuBar : View, IDesignable
                             var menu = new Menu { Host = this, X = screen.X, Y = screen.Y, BarItems = Menus [i] };
                             menu.Run (Menus [i].Action);
                             menu.Dispose ();
-                            if (menu == OpenCurrentMenu)
-                            {
-                                OpenCurrentMenu = null;
-                            }
                         }
                         else if (!IsMenuOpen)
                         {
@@ -1513,18 +1525,9 @@ public class MenuBar : View, IDesignable
                     CloseAllMenus ();
                 }
 
-//#if DEBUG_IDISPOSABLE
-//                if (WasDisposed)
-//                {
-//                    throw new ObjectDisposedException (GetType ().FullName);
-//                }
-//#endif
                 _handled = false;
 
-                Enabled = false;
                 return false;
-
-
             }
             else
             {

+ 1 - 0
Terminal.sln.DotSettings

@@ -406,6 +406,7 @@
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=langword/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Roslynator/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Toplevel/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=Ungrab/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=unsynchronized/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=BUGBUG/@EntryIndexedValue">True</s:Boolean>
 </wpf:ResourceDictionary>

+ 10 - 10
UICatalog/Scenarios/ColorPicker.cs

@@ -45,7 +45,7 @@ public class ColorPickers : Scenario
 
         // Foreground ColorPicker.
         foregroundColorPicker = new ColorPicker {
-            Title = "Foreground Color",
+            Title = "_Foreground Color",
             BorderStyle = LineStyle.Single,
             Width = Dim.Percent (50)
         };
@@ -61,7 +61,7 @@ public class ColorPickers : Scenario
         // Background ColorPicker.
         backgroundColorPicker = new ColorPicker
         {
-            Title = "Background Color",
+            Title = "_Background Color",
             X = Pos.AnchorEnd (),
             Width = Dim.Percent (50),
             BorderStyle = LineStyle.Single
@@ -86,7 +86,7 @@ public class ColorPickers : Scenario
         // Foreground ColorPicker 16.
         foregroundColorPicker16 = new ColorPicker16
         {
-            Title = "Foreground Color",
+            Title = "_Foreground Color",
             BorderStyle = LineStyle.Single,
             Width = Dim.Percent (50),
             Visible = false  // We default to HSV so hide old one
@@ -97,7 +97,7 @@ public class ColorPickers : Scenario
         // Background ColorPicker 16.
         backgroundColorPicker16 = new ColorPicker16
         {
-            Title = "Background Color",
+            Title = "_Background Color",
             X = Pos.AnchorEnd (),
             Width = Dim.Percent (50),
             BorderStyle = LineStyle.Single,
@@ -132,10 +132,10 @@ public class ColorPickers : Scenario
             Height = Dim.Auto (),
             RadioLabels = new []
             {
-                "RGB",
-                "HSV",
-                "HSL",
-                "16 Colors"
+                "_RGB",
+                "_HSV",
+                "H_SL",
+                "_16 Colors"
             },
             SelectedItem = (int)foregroundColorPicker.Style.ColorModel,
         };
@@ -180,7 +180,7 @@ public class ColorPickers : Scenario
         // Checkbox for switching show text fields on and off
         var cbShowTextFields = new CheckBox ()
         {
-            Text = "Show Text Fields",
+            Text = "Show _Text Fields",
             Y = Pos.Bottom (rgColorModel)+1,
             Width = Dim.Auto (),
             Height = Dim.Auto (),
@@ -199,7 +199,7 @@ public class ColorPickers : Scenario
         // Checkbox for switching show text fields on and off
         var cbShowName = new CheckBox ()
         {
-            Text = "Show Color Name",
+            Text = "Show Color _Name",
             Y = Pos.Bottom (cbShowTextFields) + 1,
             Width = Dim.Auto (),
             Height = Dim.Auto (),

+ 15 - 2
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 ();

+ 18 - 18
UnitTests/Views/ColorPickerTests.cs

@@ -97,7 +97,7 @@ public class ColorPickerTests
     [SetupFakeDriver]
     public void ColorPicker_RGB_MouseNavigation ()
     {
-        var cp = GetColorPicker (ColorModel.RGB,false);
+        var cp = GetColorPicker (ColorModel.RGB, false);
 
         cp.Draw ();
 
@@ -381,7 +381,7 @@ public class ColorPickerTests
         // Enter invalid hex value
         TextField hexField = cp.Subviews.OfType<TextField> ().First (tf => tf.Text == "#000000");
         hexField.Text = "#ZZZZZZ";
-        hexField.OnLeave (cp);
+        hexField.HasFocus = false;
 
         cp.Draw ();
 
@@ -550,7 +550,7 @@ public class ColorPickerTests
                     throw new NotSupportedException ("Corresponding Style option is not enabled");
                 }
 
-                return cp.Subviews.OfType<TextField> ().ElementAt (hasBarValueTextFields  ? (int)toGet : (int)toGet -3);
+                return cp.Subviews.OfType<TextField> ().ElementAt (hasBarValueTextFields ? (int)toGet : (int)toGet - 3);
             case ColorPickerPart.Hex:
 
                 int offset = hasBarValueTextFields ? 0 : 3;
@@ -608,7 +608,7 @@ public class ColorPickerTests
     [SetupFakeDriver]
     public void ColorPicker_DisposesOldViews_OnModelChange ()
     {
-        var cp = GetColorPicker (ColorModel.HSL,true);
+        var cp = GetColorPicker (ColorModel.HSL, true);
 
         var b1 = GetColorBar (cp, ColorPickerPart.Bar1);
         var b2 = GetColorBar (cp, ColorPickerPart.Bar2);
@@ -621,7 +621,7 @@ public class ColorPickerTests
         var hex = GetTextField (cp, ColorPickerPart.Hex);
 
 #if DEBUG_IDISPOSABLE
-        Assert.All (new View [] { b1, b2, b3, tf1, tf2, tf3,hex }, b => Assert.False (b.WasDisposed));
+        Assert.All (new View [] { b1, b2, b3, tf1, tf2, tf3, hex }, b => Assert.False (b.WasDisposed));
 #endif
         cp.Style.ColorModel = ColorModel.RGB;
         cp.ApplyStyleChanges ();
@@ -638,11 +638,11 @@ public class ColorPickerTests
 
         // Old bars should be disposed
 #if DEBUG_IDISPOSABLE
-        Assert.All (new View [] { b1, b2, b3, tf1, tf2, tf3,hex }, b => Assert.True (b.WasDisposed));
+        Assert.All (new View [] { b1, b2, b3, tf1, tf2, tf3, hex }, b => Assert.True (b.WasDisposed));
 #endif
-        Assert.NotSame (hex,hexAfter);
+        Assert.NotSame (hex, hexAfter);
 
-        Assert.NotSame (b1,b1After);
+        Assert.NotSame (b1, b1After);
         Assert.NotSame (b2, b2After);
         Assert.NotSame (b3, b3After);
 
@@ -654,9 +654,9 @@ public class ColorPickerTests
 
     [Fact]
     [SetupFakeDriver]
-    public void ColorPicker_TabCompleteColorName()
+    public void ColorPicker_TabCompleteColorName ()
     {
-        var cp = GetColorPicker (ColorModel.RGB, true,true);
+        var cp = GetColorPicker (ColorModel.RGB, true, true);
         cp.Draw ();
 
         var r = GetColorBar (cp, ColorPickerPart.Bar1);
@@ -665,7 +665,7 @@ public class ColorPickerTests
         var name = GetTextField (cp, ColorPickerPart.ColorName);
         var hex = GetTextField (cp, ColorPickerPart.Hex);
 
-        name.FocusFirst (TabBehavior.TabStop);
+        name.RestoreFocus (TabBehavior.TabStop);
 
         Assert.True (name.HasFocus);
         Assert.Same (name, cp.Focused);
@@ -676,7 +676,7 @@ public class ColorPickerTests
         Application.OnKeyDown (Key.A);
         Application.OnKeyDown (Key.Q);
 
-        Assert.Equal ("aq",name.Text);
+        Assert.Equal ("aq", name.Text);
 
 
         // Auto complete the color name
@@ -705,7 +705,7 @@ public class ColorPickerTests
         var name = GetTextField (cp, ColorPickerPart.ColorName);
         var hex = GetTextField (cp, ColorPickerPart.Hex);
 
-        hex.FocusFirst (TabBehavior.TabStop);
+        hex.RestoreFocus (TabBehavior.TabStop);
 
         Assert.True (hex.HasFocus);
         Assert.Same (hex, cp.Focused);
@@ -737,7 +737,7 @@ public class ColorPickerTests
 
         // Color name should be recognised as a known string and populated
         Assert.Equal ("#7FFFD4", hex.Text);
-        Assert.Equal("Aquamarine", name.Text);
+        Assert.Equal ("Aquamarine", name.Text);
 
         Application.Current?.Dispose ();
     }
@@ -747,7 +747,7 @@ public class ColorPickerTests
     {
         var colors = new W3CColors ();
         Assert.Contains ("Aquamarine", colors.GetColorNames ());
-        Assert.DoesNotContain ("Save as",colors.GetColorNames ());
+        Assert.DoesNotContain ("Save as", colors.GetColorNames ());
     }
     private ColorPicker GetColorPicker (ColorModel colorModel, bool showTextFields, bool showName = false)
     {
@@ -757,13 +757,13 @@ public class ColorPickerTests
         cp.Style.ShowColorName = showName;
         cp.ApplyStyleChanges ();
 
-        Application.Current = new Toplevel () { Width = 20 ,Height = 5};
+        Application.Current = new Toplevel () { Width = 20, Height = 5 };
         Application.Current.Add (cp);
-        Application.Current.FocusFirst (null);
+        Application.Current.RestoreFocus (null);
 
         Application.Current.LayoutSubviews ();
 
-        Application.Current.FocusFirst (null);
+        Application.Current.RestoreFocus (null);
         return cp;
     }
 }

+ 6 - 3
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 ();
     }
 
@@ -1396,7 +1397,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);
@@ -1406,7 +1408,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);

+ 45 - 0
UnitTests/Views/LabelTests.cs

@@ -1315,4 +1315,49 @@ e
         Assert.Equal (expectedLabelBounds, label.Viewport);
         super.Dispose ();
     }
+
+    [Fact]
+    [AutoInitShutdown]
+    public void Label_CanFocus_True_Get_Focus_By_Keyboard_And_Mouse ()
+    {
+        Label label = new () { Text = "label" };
+        View view = new () { Y = 2, Width = 10, Height = 1, Text = "view", CanFocus = true };
+        Toplevel top = new ();
+        top.Add (label, view);
+        Application.Begin (top);
+
+        Assert.Equal (new (0, 0, 5, 1), label.Frame);
+        Assert.Equal (new (0, 2, 10, 1), view.Frame);
+        Assert.Equal (view, top.MostFocused);
+        Assert.False (label.CanFocus);
+        Assert.False (label.HasFocus);
+        Assert.True (view.CanFocus);
+        Assert.True (view.HasFocus);
+
+        Assert.True (Application.OnKeyDown (Key.Tab));
+        Assert.False (label.HasFocus);
+        Assert.True (view.HasFocus);
+
+        Application.OnMouseEvent (new () { Position = new (0, 0), Flags = MouseFlags.Button1Clicked });
+        Assert.False (label.HasFocus);
+        Assert.True (view.HasFocus);
+
+        // Set label CanFocus to true
+        label.CanFocus = true;
+        Assert.True (Application.OnKeyDown (Key.Tab));
+        Assert.True (label.HasFocus);
+        Assert.False (view.HasFocus);
+
+        Assert.True (Application.OnKeyDown (Key.Tab));
+        Assert.False (label.HasFocus);
+        Assert.True (view.HasFocus);
+
+        Application.OnMouseEvent (new () { Position = new (0, 0), Flags = MouseFlags.Button1Clicked });
+        Assert.True (label.HasFocus);
+        Assert.False (view.HasFocus);
+
+        Application.OnMouseEvent (new () { Position = new (0, 2), Flags = MouseFlags.Button1Clicked });
+        Assert.False (label.HasFocus);
+        Assert.True (view.HasFocus);
+    }
 }

+ 1 - 1
UnitTests/Views/ScrollViewTests.cs

@@ -860,7 +860,7 @@ public class ScrollViewTests (ITestOutputHelper output)
 ";
 
         TestHelpers.AssertDriverContentsAre (expected, output);
-        
+
         top.Dispose ();
     }