Skip to content

Commit

Permalink
Fixes gui-cs#3700. ContextMenu pollutes Toplevel.MenuBar's key bindings
Browse files Browse the repository at this point in the history
  • Loading branch information
@BDisp
BDisp committed Aug 30, 2024
1 parent 56d9c59 commit 224260b
Show file tree
Hide file tree
Showing 14 changed files with 766 additions and 458 deletions.
24 changes: 12 additions & 12 deletions Terminal.Gui/Views/FileDialog.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1272,19 +1272,19 @@ private void ShowCellContextMenu (Point? clickedCell, MouseEventEventArgs e)

var contextMenu = new ContextMenu
{
Position = new Point (e.MouseEvent.Position.X + 1, e.MouseEvent.Position.Y + 1),
MenuItems = new MenuBarItem (
Position = new Point (e.MouseEvent.Position.X + 1, e.MouseEvent.Position.Y + 1)
};

var menuItems = new MenuBarItem (
[
new MenuItem (Strings.fdCtxNew, string.Empty, New),
new MenuItem (Strings.fdCtxRename, string.Empty, Rename),
new MenuItem (Strings.fdCtxDelete, string.Empty, Delete)
]
)
};

);
_tableView.SetSelection (clickedCell.Value.X, clickedCell.Value.Y, false);

contextMenu.Show ();
contextMenu.Show (menuItems);
}

private void ShowHeaderContextMenu (int clickedCol, MouseEventEventArgs e)
Expand All @@ -1293,8 +1293,10 @@ private void ShowHeaderContextMenu (int clickedCol, MouseEventEventArgs e)

var contextMenu = new ContextMenu
{
Position = new Point (e.MouseEvent.Position.X + 1, e.MouseEvent.Position.Y + 1),
MenuItems = new MenuBarItem (
Position = new Point (e.MouseEvent.Position.X + 1, e.MouseEvent.Position.Y + 1)
};

var menuItems = new MenuBarItem (
[
new MenuItem (
string.Format (
Expand All @@ -1309,10 +1311,8 @@ private void ShowHeaderContextMenu (int clickedCol, MouseEventEventArgs e)
string.Empty,
() => SortColumn (clickedCol, isAsc))
]
)
};

contextMenu.Show ();
);
contextMenu.Show (menuItems);
}

private void SortColumn (int clickedCol)
Expand Down
84 changes: 58 additions & 26 deletions Terminal.Gui/Views/Menu/ContextMenu.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
namespace Terminal.Gui;
#nullable enable

namespace Terminal.Gui;

/// <summary>
/// ContextMenu provides a pop-up menu that can be positioned anywhere within a <see cref="View"/>. ContextMenu is
Expand All @@ -16,15 +18,15 @@
/// </para>
/// <para>
/// Callers can cause the ContextMenu to be activated on a right-mouse click (or other interaction) by calling
/// <see cref="Show()"/>.
/// <see cref="Show"/>.
/// </para>
/// <para>ContextMenus are located using screen coordinates and appear above all other Views.</para>
/// </summary>
public sealed class ContextMenu : IDisposable
{
private static MenuBar _menuBar;
private static MenuBar? _menuBar;

private Toplevel _container;
private Toplevel? _container;
private Key _key = DefaultKey;
private MouseFlags _mouseFlags = MouseFlags.Button3Clicked;

Expand All @@ -33,15 +35,9 @@ public ContextMenu ()
{
if (IsShow)
{
if (_menuBar.SuperView is { })
{
Hide ();
}

Hide ();
IsShow = false;
}

MenuItems = new MenuBarItem ();
}

/// <summary>The default shortcut key for activating the context menu.</summary>
Expand All @@ -56,13 +52,13 @@ public ContextMenu ()
public bool ForceMinimumPosToZero { get; set; } = true;

/// <summary>The host <see cref="View "/> which position will be used, otherwise if it's null the container will be used.</summary>
public View Host { get; set; }
public View? Host { get; set; }

/// <summary>Gets whether the ContextMenu is showing or not.</summary>
public static bool IsShow { get; private set; }

/// <summary>Specifies the key that will activate the context menu.</summary>
public Key Key
public new Key Key
{
get => _key;
set
Expand All @@ -74,10 +70,10 @@ public Key Key
}

/// <summary>Gets the <see cref="MenuBar"/> that is hosting this context menu.</summary>
public MenuBar MenuBar => _menuBar;
public MenuBar? MenuBar => _menuBar;

/// <summary>Gets or sets the menu items for this context menu.</summary>
public MenuBarItem MenuItems { get; set; }
public MenuBarItem? MenuItems { get; private set; }

/// <summary><see cref="Gui.MouseFlags"/> specifies the mouse action used to activate the context menu by mouse.</summary>
public MouseFlags MouseFlags
Expand Down Expand Up @@ -105,11 +101,10 @@ public MouseFlags MouseFlags
/// <summary>Disposes the context menu object.</summary>
public void Dispose ()
{
if (_menuBar is null)
if (_menuBar is { })
{
return;
_menuBar.MenuAllClosed -= MenuBar_MenuAllClosed;
}
_menuBar.MenuAllClosed -= MenuBar_MenuAllClosed;
Application.UngrabMouse ();
_menuBar?.Dispose ();
_menuBar = null;
Expand All @@ -126,25 +121,62 @@ public void Dispose ()
/// <summary>Hides (closes) the ContextMenu.</summary>
public void Hide ()
{
RemoveKeyBindings (MenuItems);
_menuBar?.CleanUp ();
IsShow = false;
}

private void RemoveKeyBindings (MenuBarItem? menuBarItem)
{
if (menuBarItem is null)
{
return;
}

foreach (var menuItem in menuBarItem.Children!)
{
// ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract
if (menuItem is null)
{
continue;
}

if (menuItem is MenuBarItem barItem)
{
RemoveKeyBindings (barItem);
}
else
{
if (menuItem.ShortcutKey != Key.Empty)
{
// Remove an existent ShortcutKey
_menuBar?.KeyBindings.Remove (menuItem.ShortcutKey);
}
}
}
}

/// <summary>Event invoked when the <see cref="ContextMenu.Key"/> is changed.</summary>
public event EventHandler<KeyChangedEventArgs> KeyChanged;
public event EventHandler<KeyChangedEventArgs>? KeyChanged;

/// <summary>Event invoked when the <see cref="ContextMenu.MouseFlags"/> is changed.</summary>
public event EventHandler<MouseFlagsChangedEventArgs> MouseFlagsChanged;
public event EventHandler<MouseFlagsChangedEventArgs>? MouseFlagsChanged;

/// <summary>Shows (opens) the ContextMenu, displaying the <see cref="MenuItem"/>s it contains.</summary>
public void Show ()
public void Show (MenuBarItem? menuItems)
{
if (_menuBar is { })
{
Hide ();
Dispose ();
}

if (menuItems is null || menuItems.Children.Length == 0)
{
return;
}

MenuItems = menuItems;
_container = Application.Current;
_container!.Closing += Container_Closing;
_container.Deactivate += Container_Deactivate;
Expand All @@ -155,7 +187,7 @@ public void Show ()
if (Host is { })
{
Point pos = Host.ViewportToScreen (frame).Location;
pos.Y += Host.Frame.Height - 1;
pos.Y += Host.Frame.Height > 0 ? Host.Frame.Height - 1 : 0;

if (position != pos)
{
Expand Down Expand Up @@ -224,9 +256,9 @@ public void Show ()
_menuBar.OpenMenu ();
}

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 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 (); }
private void MenuBar_MenuAllClosed (object? sender, EventArgs e) { Hide (); }
}
55 changes: 38 additions & 17 deletions Terminal.Gui/Views/Menu/Menu.cs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#nullable enable

namespace Terminal.Gui;

/// <summary>
Expand All @@ -11,7 +13,7 @@ internal sealed class Menu : View
internal int _currentChild;
internal View _previousSubFocused;

internal static Rectangle MakeFrame (int x, int y, MenuItem [] items, Menu parent = null)
internal static Rectangle MakeFrame (int x, int y, MenuItem [] items, Menu? parent = null)
{
if (items is null || items.Length == 0)
{
Expand Down Expand Up @@ -68,6 +70,25 @@ public override void BeginInit ()

Frame = MakeFrame (Frame.X, Frame.Y, _barItems?.Children, Parent);

if (_barItems?.Children is { })
{
foreach (MenuItem menuItem in _barItems!.Children)
{
if (menuItem is { })
{
menuItem._menuBar = Host;

if (menuItem.ShortcutKey != Key.Empty)
{
KeyBinding keyBinding = new ([Command.Select], KeyBindingScope.HotKey, menuItem);
// Remove an existent ShortcutKey
menuItem._menuBar?.KeyBindings.Remove (menuItem.ShortcutKey);
menuItem._menuBar?.KeyBindings.Add (menuItem.ShortcutKey, keyBinding);
}
}
}
}

if (_barItems is { IsTopLevel: true })
{
// This is a standalone MenuItem on a MenuBar
Expand Down Expand Up @@ -166,9 +187,9 @@ public Menu ()
return true;
}
);
AddCommand (Command.Select, ctx => _host?.SelectItem (ctx.KeyBinding?.Context as MenuItem));
AddCommand (Command.ToggleExpandCollapse, ctx => ExpandCollapse (ctx.KeyBinding?.Context as MenuItem));
AddCommand (Command.HotKey, ctx => _host?.SelectItem (ctx.KeyBinding?.Context as MenuItem));
AddCommand (Command.Select, ctx => _host?.SelectItem ((ctx.KeyBinding?.Context as MenuItem)!));
AddCommand (Command.ToggleExpandCollapse, ctx => ExpandCollapse ((ctx.KeyBinding?.Context as MenuItem)!));
AddCommand (Command.HotKey, ctx => _host?.SelectItem ((ctx.KeyBinding?.Context as MenuItem)!));

// Default key bindings for this view
KeyBindings.Add (Key.CursorUp, Command.LineUp);
Expand All @@ -179,7 +200,7 @@ public Menu ()
KeyBindings.Add (Key.Enter, Command.Accept);
}

private void AddKeyBindingsHotKey (MenuBarItem menuBarItem)
private void AddKeyBindingsHotKey (MenuBarItem? menuBarItem)
{
if (menuBarItem is null || menuBarItem.Children is null)
{
Expand All @@ -200,7 +221,7 @@ private void AddKeyBindingsHotKey (MenuBarItem menuBarItem)
}
}

private void RemoveKeyBindingsHotKey (MenuBarItem menuBarItem)
private void RemoveKeyBindingsHotKey (MenuBarItem? menuBarItem)
{
if (menuBarItem is null || menuBarItem.Children is null)
{
Expand All @@ -219,7 +240,7 @@ private void RemoveKeyBindingsHotKey (MenuBarItem menuBarItem)

/// <summary>Called when a key bound to Command.ToggleExpandCollapse is pressed. This means a hot key was pressed.</summary>
/// <returns></returns>
private bool ExpandCollapse (MenuItem menuItem)
private bool ExpandCollapse (MenuItem? menuItem)
{
if (!IsInitialized || !Visible)
{
Expand All @@ -243,7 +264,7 @@ private bool ExpandCollapse (MenuItem menuItem)

if (m?.Children?.Length > 0)
{
MenuItem item = _barItems.Children [_currentChild];
MenuItem? item = _barItems.Children [_currentChild];

if (item is null)
{
Expand Down Expand Up @@ -297,7 +318,7 @@ private bool ExpandCollapse (MenuItem menuItem)
return _host.OnInvokingKeyBindings (keyEvent, scope);
}

private void Current_TerminalResized (object sender, SizeChangedEventArgs e)
private void Current_TerminalResized (object? sender, SizeChangedEventArgs e)
{
if (_host.IsMenuOpen)
{
Expand All @@ -320,7 +341,7 @@ public override void OnVisibleChanged ()
}
}

private void Application_RootMouseEvent (object sender, MouseEvent a)
private void Application_RootMouseEvent (object? sender, MouseEvent a)
{
if (a.View is { } and (MenuBar or not Menu))
{
Expand Down Expand Up @@ -350,7 +371,7 @@ private void Application_RootMouseEvent (object sender, MouseEvent a)
}
}

internal Attribute DetermineColorSchemeFor (MenuItem item, int index)
internal Attribute DetermineColorSchemeFor (MenuItem? item, int index)
{
if (item is null)
{
Expand Down Expand Up @@ -456,7 +477,7 @@ public override void OnDrawContent (Rectangle viewport)
continue;
}

string textToDraw = null;
string? textToDraw = null;
Rune nullCheckedChar = Glyphs.CheckStateNone;
Rune checkChar = Glyphs.Selected;
Rune uncheckedChar = Glyphs.UnSelected;
Expand All @@ -468,7 +489,7 @@ public override void OnDrawContent (Rectangle viewport)
}

// Support Checked even though CheckType wasn't set
if (item.CheckType == MenuItemCheckStyle.Checked && item.Checked is null)
if (item is { CheckType: MenuItemCheckStyle.Checked, Checked: null })
{
textToDraw = $"{nullCheckedChar} {item.Title}";
}
Expand Down Expand Up @@ -548,7 +569,7 @@ public override void OnDrawContent (Rectangle viewport)
// PositionCursor ();
}

private void Current_DrawContentComplete (object sender, DrawEventArgs e)
private void Current_DrawContentComplete (object? sender, DrawEventArgs e)
{
if (Visible)
{
Expand All @@ -573,9 +594,9 @@ private void Current_DrawContentComplete (object sender, DrawEventArgs e)
return _host?.PositionCursor ();
}

public void Run (Action action)
public void Run (Action? action)
{
if (action is null || _host is null)
if (action is null)
{
return;
}
Expand Down Expand Up @@ -900,7 +921,7 @@ internal bool CheckSubMenu ()

if (pos == -1
&& this != _host.OpenCurrentMenu
&& subMenu.Children != _host.OpenCurrentMenu._barItems.Children
&& subMenu.Children != _host.OpenCurrentMenu!._barItems.Children
&& !_host.CloseMenu (false, true))
{
return false;
Expand Down
Loading

0 comments on commit 224260b

Please sign in to comment.