diff --git a/Terminal.Gui/App/Application.Mouse.cs b/Terminal.Gui/App/Application.Mouse.cs index b92818d2d..6e06ab759 100644 --- a/Terminal.Gui/App/Application.Mouse.cs +++ b/Terminal.Gui/App/Application.Mouse.cs @@ -185,11 +185,7 @@ public static partial class Application // Mouse handling && Popover?.GetActivePopover () as View is { Visible: true } visiblePopover && View.IsInHierarchy (visiblePopover, deepestViewUnderMouse, includeAdornments: true) is false) { - // TODO: Build a use/test case for the popover not handling Quit - if (visiblePopover.InvokeCommand (Command.Quit) is true && visiblePopover.Visible) - { - visiblePopover.Visible = false; - } + ApplicationPopover.HideWithQuitCommand (visiblePopover); // Recurse once so the event can be handled below the popover RaiseMouseEvent (mouseEvent); @@ -205,10 +201,10 @@ public static partial class Application // Mouse handling if (Initialized) { WantContinuousButtonPressedView = deepestViewUnderMouse switch - { - { WantContinuousButtonPressed: true } => deepestViewUnderMouse, - _ => null - }; + { + { WantContinuousButtonPressed: true } => deepestViewUnderMouse, + _ => null + }; } // May be null before the prior condition or the condition may set it as null. diff --git a/Terminal.Gui/App/Application.Run.cs b/Terminal.Gui/App/Application.Run.cs index 860bed0fc..d20336e5f 100644 --- a/Terminal.Gui/App/Application.Run.cs +++ b/Terminal.Gui/App/Application.Run.cs @@ -577,11 +577,7 @@ public static partial class Application // Run (Begin, Run, End, Stop) if (Popover?.GetActivePopover () as View is { Visible: true } visiblePopover) { - // TODO: Build a use/test case for the popover not handling Quit - if (visiblePopover.InvokeCommand (Command.Quit) is true && visiblePopover.Visible) - { - visiblePopover.Visible = false; - } + ApplicationPopover.HideWithQuitCommand (visiblePopover); } runState.Toplevel.OnUnloaded (); diff --git a/Terminal.Gui/App/ApplicationPopover.cs b/Terminal.Gui/App/ApplicationPopover.cs index 061b10379..69430dbab 100644 --- a/Terminal.Gui/App/ApplicationPopover.cs +++ b/Terminal.Gui/App/ApplicationPopover.cs @@ -1,5 +1,6 @@ #nullable enable +using System.Diagnostics; namespace Terminal.Gui.App; @@ -36,12 +37,8 @@ public sealed class ApplicationPopover : IDisposable { if (popover is { } && !_popovers.Contains (popover)) { - // When created, set IPopover.Toplevel to the current Application.Top - if (popover.Toplevel is null) - { - popover.Toplevel = Application.Top; - } + popover.Toplevel ??= Application.Top; _popovers.Add (popover); } @@ -62,19 +59,20 @@ public sealed class ApplicationPopover : IDisposable /// public bool DeRegister (IPopover? popover) { - if (popover is { } && _popovers.Contains (popover)) + if (popover is null || !_popovers.Contains (popover)) { - if (GetActivePopover () == popover) - { - _activePopover = null; - } - - _popovers.Remove (popover); - - return true; + return false; } - return false; + if (GetActivePopover () == popover) + { + _activePopover = null; + } + + _popovers.Remove (popover); + + return true; + } private IPopover? _activePopover; @@ -111,6 +109,18 @@ public sealed class ApplicationPopover : IDisposable if (popover is View newPopover) { + if (!(newPopover.ViewportSettings.HasFlag (ViewportSettingsFlags.Transparent) && + newPopover.ViewportSettings.HasFlag (ViewportSettingsFlags.TransparentMouse))) + { + throw new InvalidOperationException ("Popovers must have ViewportSettings.Transparent and ViewportSettings.TransparentMouse set."); + } + + if (newPopover.KeyBindings.GetFirstFromCommands (Command.Quit) is null) + { + throw new InvalidOperationException ("Popovers must have a key binding for Command.Quit."); + } + + Register (popover); if (!newPopover.IsInitialized) @@ -127,7 +137,7 @@ public sealed class ApplicationPopover : IDisposable /// /// Causes the specified popover to be hidden. - /// If the popover is dervied from , this is the same as setting + /// If the popover is derived from , this is the same as setting /// to . /// /// @@ -142,6 +152,22 @@ public sealed class ApplicationPopover : IDisposable } } + /// + /// Hides a popover view if it supports the quit command and is currently visible. It checks for the command's + /// support before hiding. + /// + /// The view that is being checked and potentially hidden based on its visibility and command support. + internal static void HideWithQuitCommand (View visiblePopover) + { + if (visiblePopover.Visible + && (!visiblePopover.GetSupportedCommands ().Contains (Command.Quit) + || (visiblePopover.InvokeCommand (Command.Quit) is true && visiblePopover.Visible))) + { + visiblePopover.Visible = false; + } + } + + /// /// Called when the user presses a key. Dispatches the key to the active popover, if any, /// otherwise to the popovers in the order they were registered. Inactive popovers only get hotkeys. @@ -155,7 +181,7 @@ public sealed class ApplicationPopover : IDisposable if (activePopover is { Visible: true }) { - Logging.Debug ($"Active - Calling NewKeyDownEvent ({key}) on {activePopover.Title}"); + //Logging.Debug ($"Active - Calling NewKeyDownEvent ({key}) on {activePopover.Title}"); if (activePopover.NewKeyDownEvent (key)) { @@ -177,7 +203,7 @@ public sealed class ApplicationPopover : IDisposable } // hotKeyHandled = popoverView.InvokeCommandsBoundToHotKey (key); - Logging.Debug ($"Inactive - Calling NewKeyDownEvent ({key}) on {popoverView.Title}"); + //Logging.Debug ($"Inactive - Calling NewKeyDownEvent ({key}) on {popoverView.Title}"); hotKeyHandled = popoverView.NewKeyDownEvent (key); if (hotKeyHandled is true) diff --git a/Terminal.Gui/App/IPopover.cs b/Terminal.Gui/App/IPopover.cs index cbff58597..652ca4614 100644 --- a/Terminal.Gui/App/IPopover.cs +++ b/Terminal.Gui/App/IPopover.cs @@ -3,8 +3,32 @@ namespace Terminal.Gui.App; /// -/// Interface identifying a View as being capable of being a Popover. +/// Defines the contract for a popover view in Terminal.Gui. /// +/// +/// +/// A popover is a transient UI element that appears above other content to display contextual information or UI, +/// such as menus, tooltips, or dialogs. +/// Popovers are managed by and are typically shown using +/// . +/// +/// +/// Popovers are not modal; they do not block input to the rest of the application, but they do receive focus and +/// input events while visible. +/// When a popover is shown, it is responsible for handling its own layout and content. +/// +/// +/// Popovers are automatically hidden when: +/// +/// The user clicks outside the popover (unless occluded by a subview of the popover). +/// The user presses (typically Esc). +/// Another popover is shown. +/// +/// +/// +/// To implement a custom popover, inherit from or implement this interface directly. +/// +/// public interface IPopover { /// @@ -15,5 +39,5 @@ public interface IPopover /// When is called, the is set to the current /// if not already set. /// - public Toplevel? Toplevel { get; set; } + Toplevel? Toplevel { get; set; } } diff --git a/Terminal.Gui/App/PopoverBaseImpl.cs b/Terminal.Gui/App/PopoverBaseImpl.cs index 3800d886c..8c426a2db 100644 --- a/Terminal.Gui/App/PopoverBaseImpl.cs +++ b/Terminal.Gui/App/PopoverBaseImpl.cs @@ -3,24 +3,41 @@ namespace Terminal.Gui.App; /// -/// Abstract base class for Popover Views. +/// Abstract base class for popover views in Terminal.Gui. /// /// /// -/// To show a Popover, use . To hide a popover, -/// call with set to . +/// Popover Lifecycle:
+/// To display a popover, use . To hide a popover, either call +/// , +/// set to , or show another popover. /// /// -/// If the user clicks anywhere not occluded by a SubView of the Popover, presses , -/// or causes another popover to show, the Popover will be hidden. +/// Focus and Input:
+/// When visible, a popover receives focus and input events. If the user clicks outside the popover (and not on a +/// subview), +/// presses , or another popover is shown, the popover will be hidden +/// automatically. +/// +/// +/// Layout:
+/// When the popover becomes visible, it is automatically laid out to fill the screen by default. You can override +/// this behavior +/// by setting and in your derived class. +/// +/// +/// Custom Popovers:
+/// To create a custom popover, inherit from and add your own content and logic. /// /// public abstract class PopoverBaseImpl : View, IPopover { - /// - /// Creates a new PopoverBaseImpl. + /// Initializes a new instance of the class. /// + /// + /// By default, the popover fills the available screen area and is focusable. + /// protected PopoverBaseImpl () { Id = "popoverBaseImpl"; @@ -52,32 +69,43 @@ public abstract class PopoverBaseImpl : View, IPopover } } - /// + /// + public Toplevel? Toplevel { get; set; } + + /// + /// Called when the property is changing. + /// + /// + /// When becoming visible, the popover is laid out to fit the screen. + /// When becoming hidden, focus is restored to the previous view. + /// + /// + /// to cancel the visibility change; otherwise, . + /// protected override bool OnVisibleChanging () { bool ret = base.OnVisibleChanging (); - if (ret is not true) + + if (ret) { - if (!Visible) + return ret; + } + + if (!Visible) + { + // Whenever visible is changing to true, we need to resize; + // it's our only chance because we don't get laid out until we're visible + Layout (Application.Screen.Size); + } + else + { + // Whenever visible is changing to false, we need to reset the focus + if (ApplicationNavigation.IsInHierarchy (this, Application.Navigation?.GetFocused ())) { - // Whenever visible is changing to true, we need to resize; - // it's our only chance because we don't get laid out until we're visible - Layout (Application.Screen.Size); - } - else - { - // Whenever visible is changing to false, we need to reset the focus - if (ApplicationNavigation.IsInHierarchy(this, Application.Navigation?.GetFocused ())) - { - Application.Navigation?.SetFocused (Application.Top?.MostFocused); - } + Application.Navigation?.SetFocused (Application.Top?.MostFocused); } } return ret; } - - /// - public Toplevel? Toplevel { get; set; } - } diff --git a/Terminal.Gui/Input/InputBindings.cs b/Terminal.Gui/Input/InputBindings.cs index 9c2b446e1..75a4711e5 100644 --- a/Terminal.Gui/Input/InputBindings.cs +++ b/Terminal.Gui/Input/InputBindings.cs @@ -160,7 +160,7 @@ public abstract class InputBindings where TBinding : IInputBin /// The set of commands to search. /// /// The first matching bound to the set of commands specified by - /// . if the set of caommands was not found. + /// . if the set of commands was not found. /// public TEvent? GetFirstFromCommands (params Command [] commands) { return _bindings.FirstOrDefault (a => a.Value.Commands.SequenceEqual (commands)).Key; } @@ -169,7 +169,7 @@ public abstract class InputBindings where TBinding : IInputBin /// /// The s bound to the set of commands specified by . An empty list if /// the - /// set of caommands was not found. + /// set of commands was not found. /// public IEnumerable GetAllFromCommands (params Command [] commands) { diff --git a/Terminal.Gui/Views/Menu/PopoverMenu.cs b/Terminal.Gui/Views/Menu/PopoverMenu.cs index c2bd0d5ed..dcaef66ea 100644 --- a/Terminal.Gui/Views/Menu/PopoverMenu.cs +++ b/Terminal.Gui/Views/Menu/PopoverMenu.cs @@ -65,6 +65,8 @@ public class PopoverMenu : PopoverBaseImpl, IDesignable AddCommand (Command.Left, MoveLeft); KeyBindings.Add (Key.CursorLeft, Command.Left); + // PopoverBaseImpl sets a key binding for Quit, so we + // don't need to do it here. AddCommand (Command.Quit, Quit); return; diff --git a/Tests/UnitTests/Application/ApplicationPopoverTests.cs b/Tests/UnitTests/Application/ApplicationPopoverTests.cs index 422b670e8..66395840a 100644 --- a/Tests/UnitTests/Application/ApplicationPopoverTests.cs +++ b/Tests/UnitTests/Application/ApplicationPopoverTests.cs @@ -21,6 +21,7 @@ public class ApplicationPopoverTests [Fact] public void Application_Shutdown_Resets_PopoverManager () { + Application.ResetState (true); // Arrange Assert.Null (Application.Popover); Application.Init (new FakeDriver ()); @@ -37,6 +38,7 @@ public class ApplicationPopoverTests [Fact] public void Application_End_Does_Not_Reset_PopoverManager () { + Application.ResetState (true); // Arrange Assert.Null (Application.Popover); Application.Init (new FakeDriver ()); @@ -59,6 +61,7 @@ public class ApplicationPopoverTests [Fact] public void Application_End_Hides_Active () { + Application.ResetState (true); // Arrange Assert.Null (Application.Popover); Application.Init (new FakeDriver ()); @@ -87,6 +90,7 @@ public class ApplicationPopoverTests [Fact] public void Application_Shutdown_Disposes_Registered_Popovers () { + Application.ResetState (true); // Arrange Assert.Null (Application.Popover); Application.Init (new FakeDriver ()); @@ -104,6 +108,7 @@ public class ApplicationPopoverTests [Fact] public void Application_Shutdown_Does_Not_Dispose_DeRegistered_Popovers () { + Application.ResetState (true); // Arrange Assert.Null (Application.Popover); Application.Init (new FakeDriver ()); @@ -126,6 +131,7 @@ public class ApplicationPopoverTests [Fact] public void Application_Shutdown_Does_Not_Dispose_ActiveNotRegistered_Popover () { + Application.ResetState (true); // Arrange Assert.Null (Application.Popover); Application.Init (new FakeDriver ()); @@ -148,6 +154,7 @@ public class ApplicationPopoverTests [Fact] public void Register_SetsTopLevel () { + Application.ResetState (true); // Arrange Assert.Null (Application.Popover); Application.Init (new FakeDriver ()); @@ -166,6 +173,7 @@ public class ApplicationPopoverTests [Fact] public void Keyboard_Events_Go_Only_To_Popover_Associated_With_Toplevel () { + Application.ResetState (true); // Arrange Assert.Null (Application.Popover); Application.Init (new FakeDriver ()); diff --git a/Tests/UnitTestsParallelizable/Application/ApplicationPopoverTests.cs b/Tests/UnitTestsParallelizable/Application/ApplicationPopoverTests.cs index ab9746ca4..c34ddd7a5 100644 --- a/Tests/UnitTestsParallelizable/Application/ApplicationPopoverTests.cs +++ b/Tests/UnitTestsParallelizable/Application/ApplicationPopoverTests.cs @@ -144,10 +144,28 @@ public class ApplicationPopoverTests public PopoverTestClass () { + ViewportSettings = ViewportSettingsFlags.Transparent | ViewportSettingsFlags.TransparentMouse; CanFocus = true; AddCommand (Command.New, NewCommandHandler!); HotKeyBindings.Add (Key.N.WithCtrl, Command.New); + AddCommand (Command.Quit, Quit); + KeyBindings.Add (Application.QuitKey, Command.Quit); + + return; + + bool? Quit (ICommandContext? ctx) + { + if (!Visible) + { + return false; + } + + Visible = false; + + return true; + } + bool? NewCommandHandler (ICommandContext ctx) { NewCommandInvokeCount++; diff --git a/Tests/UnitTestsParallelizable/Application/PopoverBaseImplTests.cs b/Tests/UnitTestsParallelizable/Application/PopoverBaseImplTests.cs new file mode 100644 index 000000000..7d8df46a8 --- /dev/null +++ b/Tests/UnitTestsParallelizable/Application/PopoverBaseImplTests.cs @@ -0,0 +1,68 @@ +using System; +using Terminal.Gui; +using Terminal.Gui.App; +using Xunit; + +public class PopoverBaseImplTests +{ + // Minimal concrete implementation for testing + private class TestPopover : PopoverBaseImpl { } + + [Fact] + public void Constructor_SetsDefaults () + { + var popover = new TestPopover (); + + Assert.Equal ("popoverBaseImpl", popover.Id); + Assert.True (popover.CanFocus); + Assert.Equal (Dim.Fill (), popover.Width); + Assert.Equal (Dim.Fill (), popover.Height); + Assert.True (popover.ViewportSettings.HasFlag (ViewportSettingsFlags.Transparent)); + Assert.True (popover.ViewportSettings.HasFlag (ViewportSettingsFlags.TransparentMouse)); + } + + [Fact] + public void Toplevel_Property_CanBeSetAndGet () + { + var popover = new TestPopover (); + var top = new Toplevel (); + popover.Toplevel = top; + Assert.Same (top, popover.Toplevel); + } + + [Fact] + public void Show_ThrowsIfPopoverMissingRequiredFlags () + { + var popover = new TestPopover (); + + // Popover missing Transparent flags + popover.ViewportSettings = ViewportSettingsFlags.None; // Remove required flags + + var popoverManager = new ApplicationPopover (); + // Test missing Transparent flags + Assert.ThrowsAny (() => popoverManager.Show (popover)); + + } + + + [Fact] + public void Show_ThrowsIfPopoverMissingQuitCommand () + { + var popover = new TestPopover (); + + // Popover missing Command.Quit binding + popover.KeyBindings.Clear (); // Remove all key bindings + + var popoverManager = new ApplicationPopover (); + Assert.ThrowsAny (() => popoverManager.Show (popover)); + } + + [Fact] + public void Show_DoesNotThrow_BasePopoverImpl () + { + var popover = new TestPopover (); + + var popoverManager = new ApplicationPopover (); + popoverManager.Show (popover); + } +}