Fixes #4074 - Popover eats Key.Space

Terminal.Gui/View/View.Command.cs

@@ -30,6 +30,8 @@ private void SetupCommands ()
 
                         SetFocus ();
 
+                        // Always return true on hotkey, even if SetFocus fails because 
+                        // hotkeys are always handled by the View (unless RaiseHandlingHotKey cancels).
                         return true;
                     });
 
@@ -45,9 +47,9 @@ private void SetupCommands ()
 
                         if (CanFocus)
                         {
-                            SetFocus ();
-
-                            return true;
+                            // For Select, if the view is focusable and SetFocus succeeds, by defition,
+                            // the event is handled. So return what SetFocus returns.
+                            return SetFocus ();
                         }
 
                         return false;

Terminal.Gui/Views/Label.cs

@@ -25,7 +25,7 @@ public Label ()
         Width = Dim.Auto (DimAutoStyle.Text);
 
         // On HoKey, pass it to the next view
-        AddCommand (Command.HotKey, InvokeHotKeyOnNext);
+        AddCommand (Command.HotKey, InvokeHotKeyOnNextPeer);
 
         TitleChanged += Label_TitleChanged;
         MouseClick += Label_MouseClick;
@@ -59,7 +59,7 @@ public override Rune HotKeySpecifier
         set => TextFormatter.HotKeySpecifier = base.HotKeySpecifier = value;
     }
 
-    private bool? InvokeHotKeyOnNext (ICommandContext commandContext)
+    private bool? InvokeHotKeyOnNextPeer (ICommandContext commandContext)
     {
         if (RaiseHandlingHotKey () == true)
         {
@@ -70,16 +70,19 @@ public override Rune HotKeySpecifier
         {
             SetFocus ();
 
+            // Always return true on hotkey, even if SetFocus fails because
+            // hotkeys are always handled by the View (unless RaiseHandlingHotKey cancels).
+            // This is the same behavior as the base (View).
             return true;
         }
 
         if (HotKey.IsValid)
         {
+            // If the Label has a hotkey, we need to find the next view in the subview list
             int me = SuperView?.SubViews.IndexOf (this) ?? -1;
 
             if (me != -1 && me < SuperView?.SubViews.Count - 1)
             {
-
                 return SuperView?.SubViews.ElementAt (me + 1).InvokeCommand (Command.HotKey) == true;
             }
         }

Tests/IntegrationTests/FluentTests/MenuBarv2Tests.cs

@@ -506,4 +506,79 @@ public void MenuBarItem_Without_QuitKey_Open_QuitKey_Does_Not_Quit_MenuBar_Super
                                      .WriteOutLogs (_out)
                                      .Stop ();
     }
+
+    [Theory]
+    [ClassData (typeof (V2TestDrivers))]
+    public void MenuBar_Not_Active_DoesNotEat_Space (V2TestDriver d)
+    {
+        int spaceKeyDownCount = 0;
+        View testView = new View ()
+        {
+            CanFocus = true,
+            Id = "testView",
+        };
+
+        testView.KeyDown += (sender, key) =>
+                            {
+                                if (key == Key.Space)
+                                {
+                                    spaceKeyDownCount++;
+                                }
+                            };
+
+        using GuiTestContext c = With.A<Window> (50, 20, d)
+                                     .Then (
+                                            () =>
+                                            {
+                                                var menuBar = new MenuBarv2 ();
+                                                Toplevel top = Application.Top!;
+                                                menuBar.EnableForDesign (ref top);
+                                                Application.Top!.Add (menuBar);
+                                            })
+                                     .Add (testView)
+                                     .WaitIteration ()
+                                     .Focus (testView)
+                                     .RaiseKeyDownEvent (Key.Space)
+                                     .Then (() => Assert.Equal (1, spaceKeyDownCount))
+                                     .WriteOutLogs (_out)
+                                     .Stop ();
+    }
+
+    [Theory]
+    [ClassData (typeof (V2TestDrivers))]
+    public void MenuBar_Not_Active_DoesNotEat_Enter (V2TestDriver d)
+    {
+        int enterKeyDownCount = 0;
+        View testView = new View ()
+        {
+            CanFocus = true,
+            Id = "testView",
+        };
+
+        testView.KeyDown += (sender, key) =>
+                            {
+                                if (key == Key.Enter)
+                                {
+                                    enterKeyDownCount++;
+                                }
+                            };
+
+        using GuiTestContext c = With.A<Window> (50, 20, d)
+                                     .Then (
+                                            () =>
+                                            {
+                                                var menuBar = new MenuBarv2 ();
+                                                Toplevel top = Application.Top!;
+                                                menuBar.EnableForDesign (ref top);
+                                                Application.Top!.Add (menuBar);
+                                            })
+                                     .Add (testView)
+                                     .WaitIteration ()
+                                     .Focus (testView)
+                                     .RaiseKeyDownEvent (Key.Enter)
+                                     .Then (() => Assert.Equal (1, enterKeyDownCount))
+                                     .WriteOutLogs (_out)
+                                     .Stop ();
+    }
+
 }

Tests/IntegrationTests/FluentTests/PopverMenuTests.cs

@@ -47,8 +47,6 @@ public void EnableForDesign_CreatesMenuItems (V2TestDriver d)
     [ClassData (typeof (V2TestDrivers))]
     public void Activate_Sets_Application_Navigation_Correctly (V2TestDriver d)
     {
-        MenuBarv2? menuBar = null;
-
         using GuiTestContext c = With.A<Window> (50, 20, d)
                                      .Then (
                                             () =>
@@ -93,8 +91,6 @@ public void Activate_Sets_Application_Navigation_Correctly (V2TestDriver d)
     [ClassData (typeof (V2TestDrivers))]
     public void QuitKey_Hides (V2TestDriver d)
     {
-        MenuBarv2? menuBar = null;
-
         using GuiTestContext c = With.A<Window> (50, 20, d)
                                      .Then (
                                             () =>
@@ -145,8 +141,6 @@ public void QuitKey_Hides (V2TestDriver d)
     [ClassData (typeof (V2TestDrivers))]
     public void QuitKey_Restores_Focus_Correctly (V2TestDriver d)
     {
-        MenuBarv2? menuBar = null;
-
         using GuiTestContext c = With.A<Window> (50, 20, d)
                                      .Then (
                                             () =>
@@ -195,8 +189,6 @@ public void QuitKey_Restores_Focus_Correctly (V2TestDriver d)
     [ClassData (typeof (V2TestDrivers))]
     public void MenuBarItem_With_QuitKey_Open_QuitKey_Does_Not_Quit_App (V2TestDriver d)
     {
-        MenuBarv2? menuBar = null;
-
         using GuiTestContext c = With.A<Window> (50, 20, d)
                                      .Then (
                                             () =>
@@ -240,4 +232,116 @@ public void MenuBarItem_With_QuitKey_Open_QuitKey_Does_Not_Quit_App (V2TestDrive
                                      .WriteOutLogs (_out)
                                      .Stop ();
     }
+
+
+    [Theory]
+    [ClassData (typeof (V2TestDrivers))]
+    public void Not_Active_DoesNotEat_Space (V2TestDriver d)
+    {
+        int spaceKeyDownCount = 0;
+        View testView = new View ()
+        {
+            CanFocus = true,
+            Id = "testView",
+        };
+
+        testView.KeyDown += (sender, key) =>
+        {
+            if (key == Key.Space)
+            {
+                spaceKeyDownCount++;
+            }
+        };
+
+        using GuiTestContext c = With.A<Window> (50, 20, d)
+                                     .Then (
+                                            () =>
+                                            {
+                                                var popoverMenu = new PopoverMenu();
+                                                Toplevel top = Application.Top!;
+                                                popoverMenu.EnableForDesign (ref top);
+                                                Application.Popover!.Register (popoverMenu);
+                                            })
+                                     .Add (testView)
+                                     .WaitIteration ()
+                                     .Focus (testView)
+                                     .RaiseKeyDownEvent (Key.Space)
+                                     .Then (() => Assert.Equal (1, spaceKeyDownCount))
+                                     .WriteOutLogs (_out)
+                                     .Stop ();
+    }
+
+    [Theory]
+    [ClassData (typeof (V2TestDrivers))]
+    public void Not_Active_DoesNotEat_Enter (V2TestDriver d)
+    {
+        int enterKeyDownCount = 0;
+        View testView = new View ()
+        {
+            CanFocus = true,
+            Id = "testView",
+        };
+
+        testView.KeyDown += (sender, key) =>
+        {
+            if (key == Key.Enter)
+            {
+                enterKeyDownCount++;
+            }
+        };
+
+        using GuiTestContext c = With.A<Window> (50, 20, d)
+                                     .Then (
+                                            () =>
+                                            {
+                                                var popoverMenu = new PopoverMenu ();
+                                                Toplevel top = Application.Top!;
+                                                popoverMenu.EnableForDesign (ref top);
+                                                Application.Popover!.Register (popoverMenu);
+                                            })
+                                     .Add (testView)
+                                     .WaitIteration ()
+                                     .Focus (testView)
+                                     .RaiseKeyDownEvent (Key.Enter)
+                                     .Then (() => Assert.Equal (1, enterKeyDownCount))
+                                     .WriteOutLogs (_out)
+                                     .Stop ();
+    }
+
+    [Theory]
+    [ClassData (typeof (V2TestDrivers))]
+    public void Not_Active_DoesNotEat_QuitKey (V2TestDriver d)
+    {
+        int quitKeyDownCount = 0;
+        View testView = new View ()
+        {
+            CanFocus = true,
+            Id = "testView",
+        };
+
+        testView.KeyDown += (sender, key) =>
+                            {
+                                if (key == Application.QuitKey)
+                                {
+                                    quitKeyDownCount++;
+                                }
+                            };
+
+        using GuiTestContext c = With.A<Window> (50, 20, d)
+                                     .Then (
+                                            () =>
+                                            {
+                                                var popoverMenu = new PopoverMenu ();
+                                                Toplevel top = Application.Top!;
+                                                popoverMenu.EnableForDesign (ref top);
+                                                Application.Popover!.Register (popoverMenu);
+                                            })
+                                     .Add (testView)
+                                     .WaitIteration ()
+                                     .Focus (testView)
+                                     .RaiseKeyDownEvent (Application.QuitKey)
+                                     .Then (() => Assert.Equal (1, quitKeyDownCount))
+                                     .WriteOutLogs (_out)
+                                     .Stop ();
+    }
 }

Tests/UnitTests/View/ViewCommandTests.cs

@@ -100,7 +100,7 @@ public void Button_CanFocus_False_Raises_Accepted_Correctly ()
                        };
 
         int btnAcceptedCount = 0;
-        bool btnCancelAccepting = false;
+        bool btnCancelAccepting = true;
         var btn = new Button ()
         {
             Title = "Button",
@@ -148,7 +148,7 @@ public void Button_CanFocus_False_Raises_Accepted_Correctly ()
                                      });
 
         Assert.Equal (1, btnAcceptedCount);
-        Assert.Equal (2, wAcceptedCount);
+        Assert.Equal (0, wAcceptedCount);
 
         Application.ResetState (true);
     }