`PopoverMenu` eats `Key.Space` inputs

[Error-String-Expected-Got-Nil]

Describe the bug
I have a TextView for getting some user input. When I try to input into it, is seems to work fine, except for the space key, which doesn't do anything.

After quite a bit of investigation with a debugger, I have managed to determine the root cause is that the space input is getting eaten by a popover menu created by a menu bar I added. Apparently menu bar popovers are by default configured to have space as a hotkey for 'select'. However, popovers can eat key inputs even when unfocused, so now my TextView isn't able to accept space inputs.

To Reproduce
Steps to reproduce the behavior:

• Create a new window
• Add a MenuBarv2 to it with at least one MenuBarItemv2 with at least one MenuItemv2 (unsure if necessary, but it's what mine has)
• Create a TextField or TextView in the same window
• Run the application, enter the text input, and attempt to enter a space key
• Observe that nothing happens

Expected behavior
You should be able to enter spaces when you have a menu bar.

An actual text character like space probably shouldn't be a global hotkey like this. Or at the very least, it should be possible to make MenuBarv2 popover menus non-global, or remove the space key from their hotkeys. If that is possible already, I would like to know how, but I also still think this shouldn't be the default behavior.

[tig]

I'll look at this asap. MenuBarv2 is definitely not fully baked and my apologies about that!

[tznind]

Here is an integration test

Btw we should probably make a menu equivellent so we don't have to use Then - see .WithContextMenu () for example. We could call it WithTopMenu or WithMenuBarV2

[Theory]
[ClassData (typeof (V2TestDrivers))]
public void MenuBar_DoesNotPreventSpace (V2TestDriver d)
{
    TextField tf;

    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 (tf = new TextField ())
                                 .WaitIteration ()
                                 .Focus <TextField>(_=>true)
                                 .ScreenShot ("Initial state", _out)
                                 .RaiseKeyDownEvent (Key.A)
                                 .RaiseKeyDownEvent (Key.Space)
                                 .RaiseKeyDownEvent (Key.B)
                                 .ScreenShot ("After typing",_out)
                                 .AssertEqual ("a b",tf.Text)
                                 .WriteOutLogs (_out)
                                 .Stop ();

Fails with

Xunit.Sdk.EqualException
Assert.Equal() Failure: Strings differ
            ↓ (pos 1)
Expected: "a b"
Actual:   "ab"
            ↑ (pos 1)

[Error-String-Expected-Got-Nil]

I have discovered that there is a way to remove command keybindings, popover.KeyBindings.Remove(). Only found it by accident while looking through TextView for something else. Still probably shouldn't be the default behavior, though.

[tznind]

I have discovered that there is a way to remove command keybindings, popover.KeyBindings.Remove(). Only found it by accident while looking through TextView for something else. Still probably shouldn't be the default behavior, though.

Indeed, the problem is not the keybinding - but rather the priority order of which view gets to handle the space first evenwhen menu is closed. This will be fixed in library :)

[tig]

This is actually the proper, low-level unit test:

    [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 ();
    }

The bug is actually in View.Command:

        // Space or single-click - Raise Selecting
        AddCommand (Command.Select, ctx =>
                                    {
                                        if (RaiseSelecting (ctx) is true)
                                        {
                                            return true;
                                        }

                                        if (CanFocus)
                                        {
-                                             SetFocus ();
- 
+                                            return SetFocus ();
                                        }

                                        return false;
                                    });

[tig]

Please review PR:

• Fixes #4074 - Popover eats Key.Space #4075

[tig]

More info: by default View sets focus on Command.Select which is bound to Key.Space. However, it ignores if the View is actually visible or not.

MenuBarv2 creates a PopoverMenu for each MenuBarItemv2. These are Visible = false until active.

When Key.Space is pressed, Command.Select gets routed to the first MenuBarItem2 which is not visible. The old Command.Select handler blindly returned true, indicating the event was handled.

The fix is to actually use the result of SetFocus, which will return false if the view is not Visible.