Fixes #4167. Add Accepted event to View (#4452)

* Initial plan

* Add Accepted event to View and remove duplicate implementations

Co-authored-by: tig <[email protected]>

* Update RaiseAccepting to call RaiseAccepted and add comprehensive tests

Co-authored-by: tig <[email protected]>

* Fix code style violations - use explicit types and target-typed new

Co-authored-by: tig <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: tig <[email protected]>
Co-authored-by: Tig <[email protected]>

Terminal.Gui/ViewBase/View.Command.cs

@@ -141,6 +141,13 @@ public partial class View // Command APIs
             Accepting?.Invoke (this, args);
         }
 
+        // If Accepting was handled, raise Accepted (non-cancelable event)
+        if (args.Handled)
+        {
+            Logging.Debug ($"{Title} ({ctx?.Source?.Title}) - Calling RaiseAccepted");
+            RaiseAccepted (ctx);
+        }
+
         // Accept is a special case where if the event is not canceled, the event is
         //  - Invoked on any peer-View with IsDefault == true
         //  - bubbled up the SuperView hierarchy.
@@ -201,6 +208,48 @@ public partial class View // Command APIs
     /// </remarks>
     public event EventHandler<CommandEventArgs>? Accepting;
 
+    /// <summary>
+    ///     Raises the <see cref="OnAccepted"/>/<see cref="Accepted"/> event indicating the View has been accepted.
+    ///     This is called after <see cref="Accepting"/> has been raised and not cancelled.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         Unlike <see cref="Accepting"/>, this event cannot be cancelled. It is raised after the View has been accepted.
+    ///     </para>
+    /// </remarks>
+    /// <param name="ctx">The command context.</param>
+    protected void RaiseAccepted (ICommandContext? ctx)
+    {
+        CommandEventArgs args = new () { Context = ctx };
+
+        OnAccepted (args);
+        Accepted?.Invoke (this, args);
+    }
+
+    /// <summary>
+    ///     Called when the View has been accepted. This is called after <see cref="Accepting"/> has been raised and not cancelled.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         Unlike <see cref="OnAccepting"/>, this method is called after the View has been accepted and cannot cancel the operation.
+    ///     </para>
+    /// </remarks>
+    /// <param name="args">The event arguments.</param>
+    protected virtual void OnAccepted (CommandEventArgs args) { }
+
+    /// <summary>
+    ///     Event raised when the View has been accepted. This is raised after <see cref="Accepting"/> has been raised and not cancelled.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         Unlike <see cref="Accepting"/>, this event cannot be cancelled. It is raised after the View has been accepted.
+    ///     </para>
+    ///     <para>
+    ///         See <see cref="RaiseAccepted"/> for more information.
+    ///     </para>
+    /// </remarks>
+    public event EventHandler<CommandEventArgs>? Accepted;
+
     /// <summary>
     ///     Called when the user has performed an action (e.g. <see cref="Command.Select"/>) causing the View to change state.
     ///     Calls <see cref="OnSelecting"/> which can be cancelled; if not cancelled raises <see cref="Accepting"/>.

Terminal.Gui/Views/Menu/Menu.cs

@@ -136,40 +136,7 @@ public class Menu : Bar
         return false;
     }
 
-    // TODO: Consider moving Accepted to Bar?
 
-    /// <summary>
-    ///     Raises the <see cref="OnAccepted"/>/<see cref="Accepted"/> event indicating an item in this menu (or submenu)
-    ///     was accepted. This is used to determine when to hide the menu.
-    /// </summary>
-    /// <param name="ctx"></param>
-    /// <returns></returns>
-    protected void RaiseAccepted (ICommandContext? ctx)
-    {
-        //Logging.Trace ($"RaiseAccepted: {ctx}");
-        CommandEventArgs args = new () { Context = ctx };
-
-        OnAccepted (args);
-        Accepted?.Invoke (this, args);
-    }
-
-    /// <summary>
-    ///     Called when the user has accepted an item in this menu (or submenu). This is used to determine when to hide the menu.
-    /// </summary>
-    /// <remarks>
-    /// </remarks>
-    /// <param name="args"></param>
-    protected virtual void OnAccepted (CommandEventArgs args) { }
-
-    /// <summary>
-    ///     Raised when the user has accepted an item in this menu (or submenu). This is used to determine when to hide the menu.
-    /// </summary>
-    /// <remarks>
-    /// <para>
-    ///    See <see cref="RaiseAccepted"/> for more information.
-    /// </para>
-    /// </remarks>
-    public event EventHandler<CommandEventArgs>? Accepted;
 
     /// <inheritdoc />
     protected override void OnFocusedChanged (View? previousFocused, View? focused)

Terminal.Gui/Views/Menu/MenuItem.cs

@@ -143,15 +143,10 @@ public class MenuItem : Shortcut
         {
             // Logging.Debug ($"{Title} - calling base.DispatchCommand...");
             // Base will Raise Selected, then Accepting, then invoke the Action, if any
+            // Note: base.DispatchCommand will call RaiseAccepted via RaiseAccepting when handled
             ret = base.DispatchCommand (commandContext);
         }
 
-        if (ret is true)
-        {
-            // Logging.Debug ($"{Title} - Calling RaiseAccepted");
-            RaiseAccepted (commandContext);
-        }
-
         return ret;
     }
 
@@ -205,42 +200,7 @@ public class MenuItem : Shortcut
         return base.OnMouseEnter (eventArgs);
     }
 
-    // TODO: Consider moving Accepted to Shortcut?
-
-    /// <summary>
-    ///     Raises the <see cref="OnAccepted"/>/<see cref="Accepted"/> event indicating this item (or submenu)
-    ///     was accepted. This is used to determine when to hide the menu.
-    /// </summary>
-    /// <param name="ctx"></param>
-    /// <returns></returns>
-    protected void RaiseAccepted (ICommandContext? ctx)
-    {
-        //Logging.Trace ($"RaiseAccepted: {ctx}");
-        CommandEventArgs args = new () { Context = ctx };
-
-        OnAccepted (args);
-        Accepted?.Invoke (this, args);
-    }
-
-    /// <summary>
-    ///     Called when the user has accepted an item in this menu (or submenu). This is used to determine when to hide the
-    ///     menu.
-    /// </summary>
-    /// <remarks>
-    /// </remarks>
-    /// <param name="args"></param>
-    protected virtual void OnAccepted (CommandEventArgs args) { }
 
-    /// <summary>
-    ///     Raised when the user has accepted an item in this menu (or submenu). This is used to determine when to hide the
-    ///     menu.
-    /// </summary>
-    /// <remarks>
-    ///     <para>
-    ///         See <see cref="RaiseAccepted"/> for more information.
-    ///     </para>
-    /// </remarks>
-    public event EventHandler<CommandEventArgs>? Accepted;
 
     /// <inheritdoc/>
     protected override void Dispose (bool disposing)

Terminal.Gui/Views/Menu/PopoverMenu.cs

@@ -560,40 +560,7 @@ public class PopoverMenu : PopoverBaseImpl, IDesignable
         return false;
     }
 
-    /// <summary>
-    ///     Raises the <see cref="OnAccepted"/>/<see cref="Accepted"/> event indicating a menu (or submenu)
-    ///     was accepted and the Menus in the PopoverMenu were hidden. Use this to determine when to hide the PopoverMenu.
-    /// </summary>
-    /// <param name="ctx"></param>
-    /// <returns></returns>
-    protected void RaiseAccepted (ICommandContext? ctx)
-    {
-        // Logging.Debug ($"{Title} - RaiseAccepted: {ctx}");
-        CommandEventArgs args = new () { Context = ctx };
-
-        OnAccepted (args);
-        Accepted?.Invoke (this, args);
-    }
 
-    /// <summary>
-    ///     Called when the user has accepted an item in this menu (or submenu. This is used to determine when to hide the
-    ///     menu.
-    /// </summary>
-    /// <remarks>
-    /// </remarks>
-    /// <param name="args"></param>
-    protected virtual void OnAccepted (CommandEventArgs args) { }
-
-    /// <summary>
-    ///     Raised when the user has accepted an item in this menu (or submenu. This is used to determine when to hide the
-    ///     menu.
-    /// </summary>
-    /// <remarks>
-    ///     <para>
-    ///         See <see cref="RaiseAccepted"/> for more information.
-    ///     </para>
-    /// </remarks>
-    public event EventHandler<CommandEventArgs>? Accepted;
 
     private void MenuOnSelectedMenuItemChanged (object? sender, MenuItem? e)
     {

Tests/UnitTestsParallelizable/ViewBase/ViewCommandTests.cs

@@ -124,9 +124,124 @@ public class ViewCommandTests
         Assert.Equal (0, view.OnAcceptedCount);
     }
 
-
     #endregion OnAccept/Accept tests
 
+    #region Accepted tests
+
+    [Fact]
+    public void Accepted_Event_Is_Raised_After_Accepting_When_Handled ()
+    {
+        View view = new ();
+        var acceptingInvoked = false;
+        var acceptedInvoked = false;
+
+        view.Accepting += (sender, e) =>
+                          {
+                              acceptingInvoked = true;
+                              e.Handled = true;
+                          };
+
+        view.Accepted += (sender, e) =>
+                         {
+                             acceptedInvoked = true;
+                             Assert.True (acceptingInvoked); // Accepting should be raised first
+                         };
+
+        bool? ret = view.InvokeCommand (Command.Accept);
+        Assert.True (ret);
+        Assert.True (acceptingInvoked);
+        Assert.True (acceptedInvoked);
+    }
+
+    [Fact]
+    public void Accepted_Event_Not_Raised_When_Accepting_Not_Handled ()
+    {
+        View view = new ();
+        var acceptingInvoked = false;
+        var acceptedInvoked = false;
+
+        view.Accepting += (sender, e) =>
+                          {
+                              acceptingInvoked = true;
+                              e.Handled = false;
+                          };
+
+        view.Accepted += (sender, e) =>
+                         {
+                             acceptedInvoked = true;
+                         };
+
+        // When not handled, Accept bubbles to SuperView, so returns false (no superview)
+        bool? ret = view.InvokeCommand (Command.Accept);
+        Assert.False (ret);
+        Assert.True (acceptingInvoked);
+        Assert.False (acceptedInvoked); // Should not be invoked when not handled
+    }
+
+    [Fact]
+    public void Accepted_Event_Cannot_Be_Cancelled ()
+    {
+        View view = new ();
+        var acceptedInvoked = false;
+
+        view.Accepting += (sender, e) =>
+                          {
+                              e.Handled = true;
+                          };
+
+        view.Accepted += (sender, e) =>
+                         {
+                             acceptedInvoked = true;
+                             // Accepted event has Handled property but it doesn't affect flow
+                             e.Handled = false;
+                         };
+
+        bool? ret = view.InvokeCommand (Command.Accept);
+        Assert.True (ret);
+        Assert.True (acceptedInvoked);
+    }
+
+    [Fact]
+    public void OnAccepted_Called_When_Accepting_Handled ()
+    {
+        OnAcceptedTestView view = new ();
+
+        view.Accepting += (sender, e) =>
+                          {
+                              e.Handled = true;
+                          };
+
+        view.InvokeCommand (Command.Accept);
+        Assert.Equal (1, view.OnAcceptedCallCount);
+    }
+
+    [Fact]
+    public void OnAccepted_Not_Called_When_Accepting_Not_Handled ()
+    {
+        OnAcceptedTestView view = new ();
+
+        view.Accepting += (sender, e) =>
+                          {
+                              e.Handled = false;
+                          };
+
+        view.InvokeCommand (Command.Accept);
+        Assert.Equal (0, view.OnAcceptedCallCount);
+    }
+
+    private class OnAcceptedTestView : View
+    {
+        public int OnAcceptedCallCount { get; private set; }
+
+        protected override void OnAccepted (CommandEventArgs args)
+        {
+            OnAcceptedCallCount++;
+            base.OnAccepted (args);
+        }
+    }
+
+    #endregion Accepted tests
+
     #region OnSelect/Select tests
 
     [Theory]