search

gui-cs / Terminal.Gui

Cross Platform Terminal UI toolkit for .NET
MIT License
9.56k stars 680 forks source link

`ContextMenu` pollutes `Toplevel.MenuBar`'s key bindings #3700

Closed tig closed 1 week ago

tig commented 2 weeks ago

To repro:

1) Run Graph View Example Scenario 2) PressCtrl-G`

Behavior:

Nothing happens

Expected:

Graph cycles to next graph.

image

Debugging this, I found the Keybindings for the Top.MenuBar are polluted with all of the key bindings for the context menu for the TextView used for the "about" frame.

As a result, since TextView uses Ctrl+G for Delete All, Ctrl+G was added as a key binding to the menu. Thus the StatusBar never sees it.

This code in TextView.cs is creating MenuItems.

image

MenuItem uses MenuBar._menuBar which is a static.

This was introduced in @bdisp's

BDisp commented 2 weeks ago

The issue doesn't had to do with ContextMenu _menuBar been static but due the fact ContextMenu been initialized with MenuItems before it's open. My idea is only create the ContextMenu's MenuItems during the call of the Show method. But when the ContextMenu is opened the ScrollBar shortcut Ctrl-G will be replaced by the same keybind of the ContextMenu. The only way to avoid this is not use keybings that are already being used by others views.

tig commented 2 weeks ago

FWIW, we're not too far from being able to replace Menu with Menuv2. I just mocked up a quick prototype in the Bars scenario using this branch as a base https://github.com/gui-cs/Terminal.Gui/pull/3705.

ZI4aRMC 1

So, investing too much energy in trying to fix ContextMenu may not be a good use of time.

BDisp commented 2 weeks ago

So, investing too much energy in trying to fix ContextMenu may not be a good use of time.

I almost finishing this. Can be used as prove of concept and then remove.

tig commented 2 weeks ago

please do checkout that prototype sometime. I just refined it a bit. Kinda cool me thinks.