From f2e5b08a88a23d2c431a47ec9f0b893b0f2759a2 Mon Sep 17 00:00:00 2001 From: Fabian R Date: Sat, 23 May 2020 18:41:41 -0500 Subject: [PATCH 1/2] + Added IsCurrentTop read-only property to Gui.Toplevel class to allow a more convenient checking if a view's instance is currently on top (active and displayed). - Fixed an elusive crash that may occur in the Application.RunLoop method due to a null'ed Toplevel.NeedDisplay property. This issue appears to be caused by a race condition that may occur when switching Views (TopLevel) too fast. Extended to all other NeedDisplay checks too. - ListView control now displays empty rows for Null items in its Items collection, instead of crashing with a NullReferenceException. - Improved MenuBarItem constructor behaviour by performing an additional sanity check on the MenuItem[] children parameter to ensure its not null. If so, raise an ArgumentNullException. Using an empty array of type MenuItem for this parameter still displays an empty menu as intended. --- Terminal.Gui/Core.cs | 21 +++++++++++++++------ Terminal.Gui/Views/ListView.cs | 16 ++++++++++------ Terminal.Gui/Views/Menu.cs | 3 +++ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/Terminal.Gui/Core.cs b/Terminal.Gui/Core.cs index fa0b41354..397f9f462 100644 --- a/Terminal.Gui/Core.cs +++ b/Terminal.Gui/Core.cs @@ -350,6 +350,15 @@ namespace Terminal.Gui { /// The identifier. public ustring Id { get; set; } = ""; + /// + /// Returns a value indicating if this View is currently on Top (Active) + /// + public bool IsCurrentTop { + get { + return Application.Current == this; + } + } + /// /// Gets or sets a value indicating whether this want mouse position reports. /// @@ -531,7 +540,7 @@ namespace Terminal.Gui { /// The region that must be flagged for repaint. public void SetNeedsDisplay (Rect region) { - if (NeedDisplay.IsEmpty) + if (NeedDisplay == null || NeedDisplay.IsEmpty) NeedDisplay = region; else { var x = Math.Min (NeedDisplay.X, region.X); @@ -1015,7 +1024,7 @@ namespace Terminal.Gui { if (subviews != null) { foreach (var view in subviews) { - if (!view.NeedDisplay.IsEmpty || view.childNeedsDisplay) { + if (view.NeedDisplay != null && (!view.NeedDisplay.IsEmpty || view.childNeedsDisplay)) { if (view.Frame.IntersectsWith (clipRect) && view.Frame.IntersectsWith (region)) { // FIXED: optimize this by computing the intersection of region and view.Bounds @@ -1704,8 +1713,8 @@ namespace Terminal.Gui { { Application.CurrentView = this; - if (this == Application.Top || this == Application.Current) { - if (!NeedDisplay.IsEmpty) { + if (IsCurrentTop) { + if (NeedDisplay != null && !NeedDisplay.IsEmpty) { Driver.SetAttribute (Colors.TopLevel.Normal); Clear (region); Driver.SetAttribute (Colors.Base.Normal); @@ -1887,7 +1896,7 @@ namespace Terminal.Gui { { Application.CurrentView = this; - if (!NeedDisplay.IsEmpty) { + if (NeedDisplay != null && !NeedDisplay.IsEmpty) { DrawFrameWindow (); } contentView.Redraw (contentView.Bounds); @@ -2487,7 +2496,7 @@ namespace Terminal.Gui { Iteration?.Invoke (null, EventArgs.Empty); } else if (wait == false) return; - if (!state.Toplevel.NeedDisplay.IsEmpty || state.Toplevel.childNeedsDisplay) { + if (state.Toplevel.NeedDisplay != null && (!state.Toplevel.NeedDisplay.IsEmpty || state.Toplevel.childNeedsDisplay)) { state.Toplevel.Redraw (state.Toplevel.Bounds); if (DebugDrawBounds) DrawBounds (state.Toplevel); diff --git a/Terminal.Gui/Views/ListView.cs b/Terminal.Gui/Views/ListView.cs index e3c53557b..c325485cb 100644 --- a/Terminal.Gui/Views/ListView.cs +++ b/Terminal.Gui/Views/ListView.cs @@ -593,12 +593,16 @@ namespace Terminal.Gui { { container.Move (col, line); var t = src [item]; - if (t is ustring) { - RenderUstr (driver, (ustring)t, col, line, width); - } else if (t is string) { - RenderUstr (driver, (string)t, col, line, width); - } else - RenderUstr (driver, t.ToString (), col, line, width); + if (t == null) { + RenderUstr (driver, ustring.Make(""), col, line, width); + } else { + if (t is ustring) { + RenderUstr (driver, (ustring)t, col, line, width); + } else if (t is string) { + RenderUstr (driver, (string)t, col, line, width); + } else + RenderUstr (driver, t.ToString (), col, line, width); + } } /// diff --git a/Terminal.Gui/Views/Menu.cs b/Terminal.Gui/Views/Menu.cs index 0a920a8ed..688bea039 100644 --- a/Terminal.Gui/Views/Menu.cs +++ b/Terminal.Gui/Views/Menu.cs @@ -162,6 +162,9 @@ namespace Terminal.Gui { /// The items in the current menu. public MenuBarItem (ustring title, MenuItem [] children) { + if (children == null) + throw new ArgumentNullException (nameof (children), "The parameter cannot be null. Use an empty array instead."); + SetTitle (title ?? ""); Children = children; } From 814821b93342d7fdd413ba2a9379434773c005e6 Mon Sep 17 00:00:00 2001 From: Fabian R Date: Sat, 23 May 2020 20:22:11 -0500 Subject: [PATCH 2/2] Removed redundant properties on MenuBar View -- Redundant members isMenuClosed and MenuOpen were replaced with a more consistent and intuitive single IsMenuOpen property. - CloseMenu method is now Public. - StartMenu has been renamed to OpenMenu and made Public. + Added missing XmlDoc to OpenMenu and CloseMenu, and fixed an small typo to MenuOpen/IsMenuOpen This commit breaks compatibility with previous versions. Changes required to host apps: MenuBar.MenuOpen property renamed to IsMenuOpen MenuBar.StartMenu method renamed to OpenMenu, use it to open the Menu Use MenuBar.CloseMenu to close the active menu. --- Terminal.Gui/Views/Menu.cs | 61 +++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/Terminal.Gui/Views/Menu.cs b/Terminal.Gui/Views/Menu.cs index 688bea039..da16a08ef 100644 --- a/Terminal.Gui/Views/Menu.cs +++ b/Terminal.Gui/Views/Menu.cs @@ -323,7 +323,7 @@ namespace Terminal.Gui { public override void PositionCursor () { - if (host == null || !host.isMenuClosed) + if (host == null || host.IsMenuOpen) if (barItems.IsTopLevel) { host.PositionCursor (); } else @@ -406,11 +406,11 @@ namespace Terminal.Gui { var item = barItems.Children [current]; if (item == null || !item.IsEnabled ()) disabled = true; if (host.UseKeysUpDownAsKeysLeftRight && barItems.Children [current]?.SubMenu != null && - !disabled && !host.isMenuClosed) { + !disabled && host.IsMenuOpen) { CheckSubMenu (); break; } - if (host.isMenuClosed) + if (!host.IsMenuOpen) host.OpenMenu (host.selected); } while (barItems.Children [current] == null || disabled); SetNeedsDisplay (); @@ -566,7 +566,7 @@ namespace Terminal.Gui { selectedSub = -1; ColorScheme = Colors.Menu; WantMousePositionReports = true; - isMenuClosed = true; + IsMenuOpen = false; } bool openedByAltKey; @@ -587,14 +587,14 @@ namespace Terminal.Gui { { if (keyEvent.IsAlt) { // User pressed Alt - this may be a precursor to a menu accelerator (e.g. Alt-F) - if (!keyEvent.IsCtrl && openedByAltKey && isMenuClosed && openMenu == null && ((uint)keyEvent.Key & (uint)Key.CharMask) == 0) { + if (!keyEvent.IsCtrl && openedByAltKey && !IsMenuOpen && openMenu == null && ((uint)keyEvent.Key & (uint)Key.CharMask) == 0) { // There's no open menu, the first menu item should be highlight. // The right way to do this is to SetFocus(MenuBar), but for some reason // that faults. //Activate (0); //StartMenu (); - isMenuClosed = false; + IsMenuOpen = true; selected = 0; CanFocus = true; lastFocused = SuperView.MostFocused; @@ -609,7 +609,7 @@ namespace Terminal.Gui { if (openMenu != null) CloseAllMenus (); openedByAltKey = false; - isMenuClosed = true; + IsMenuOpen = false; selected = -1; CanFocus = false; if (lastFocused != null) @@ -661,13 +661,13 @@ namespace Terminal.Gui { for (int i = 0; i < Menus.Length; i++) { if (i == selected) { pos++; - if (!isMenuClosed) + if (IsMenuOpen) Move (pos + 1, 0); else Move (pos + 1, 0); return; } else { - if (!isMenuClosed) + if (IsMenuOpen) pos += 1 + Menus [i].TitleLength + 2; else pos += 2 + Menus [i].TitleLength + 1; @@ -698,12 +698,11 @@ namespace Terminal.Gui { View previousFocused; internal bool isMenuOpening; internal bool isMenuClosing; - internal bool isMenuClosed; /// - /// True of the menu is open; otherwise false. + /// True if the menu is open; otherwise false. /// - public bool MenuOpen { get; set; } + public bool IsMenuOpen { get; protected set; } View lastFocused; @@ -751,12 +750,13 @@ namespace Terminal.Gui { break; } isMenuOpening = false; - isMenuClosed = false; - MenuOpen = true; + IsMenuOpen = true; } - // Starts the menu from a hotkey - void StartMenu () + /// + /// Opens the current Menu programatically. + /// + public void OpenMenu () { if (openMenu != null) return; @@ -781,6 +781,13 @@ namespace Terminal.Gui { SetNeedsDisplay (); } + /// + /// Closes the current Menu programatically, if open. + /// + public void CloseMenu() + { + CloseMenu (false, false); + } internal void CloseMenu (bool reopen = false, bool isSubMenu = false) { isMenuClosing = true; @@ -802,12 +809,12 @@ namespace Terminal.Gui { if (!reopen) selected = -1; LastFocused.SuperView?.SetFocus (LastFocused); + IsMenuOpen = false; } else { SuperView.SetFocus (this); - isMenuClosed = true; + IsMenuOpen = false; PositionCursor (); } - isMenuClosed = true; break; case true: @@ -819,7 +826,7 @@ namespace Terminal.Gui { break; } isMenuClosing = false; - MenuOpen = false; + IsMenuOpen = false; } void RemoveSubMenu (int index) @@ -882,7 +889,7 @@ namespace Terminal.Gui { if (LastFocused != null && LastFocused != this) selected = -1; } - isMenuClosed = true; + IsMenuOpen = false; openedByHotKey = false; openedByAltKey = false; } @@ -996,8 +1003,8 @@ namespace Terminal.Gui { public override bool ProcessHotKey (KeyEvent kb) { if (kb.Key == Key.F9) { - if (isMenuClosed) - StartMenu (); + if (!IsMenuOpen) + OpenMenu (); else CloseAllMenus (); return true; @@ -1088,7 +1095,7 @@ namespace Terminal.Gui { for (int i = 0; i < Menus.Length; i++) { if (cx > pos && me.X < pos + 1 + Menus [i].TitleLength) { if (selected == i && (me.Flags == MouseFlags.Button1Pressed || me.Flags == MouseFlags.Button1DoubleClicked) && - !isMenuClosed) { + IsMenuOpen) { Application.UngrabMouse (); if (Menus [i].IsTopLevel) { var menu = new Menu (this, i, 0, Menus [i]); @@ -1097,7 +1104,7 @@ namespace Terminal.Gui { CloseMenu (); } } else if ((me.Flags == MouseFlags.Button1Pressed || me.Flags == MouseFlags.Button1DoubleClicked) && - isMenuClosed) { + !IsMenuOpen) { if (Menus [i].IsTopLevel) { var menu = new Menu (this, i, 0, Menus [i]); menu.Run (Menus [i].Action); @@ -1105,12 +1112,12 @@ namespace Terminal.Gui { Activate (i); } } else if (selected != i && selected > -1 && me.Flags == MouseFlags.ReportMousePosition) { - if (!isMenuClosed) { + if (IsMenuOpen) { CloseMenu (); Activate (i); } } else { - if (!isMenuClosed) + if (IsMenuOpen) Activate (i); } return true; @@ -1142,7 +1149,7 @@ namespace Terminal.Gui { handled = false; return false; } - } else if (isMenuClosed && (me.Flags == MouseFlags.Button1Pressed || me.Flags == MouseFlags.Button1DoubleClicked || + } else if (!IsMenuOpen && (me.Flags == MouseFlags.Button1Pressed || me.Flags == MouseFlags.Button1DoubleClicked || me.Flags.HasFlag (MouseFlags.Button1Pressed | MouseFlags.ReportMousePosition))) { Application.GrabMouse (current); } else {