Fixes #5366 - BREAKING CHANGE - Make View.Text notifications CWP-compliant and non-virtual

Terminal.Gui/ViewBase/View.Text.cs

@@ -1,14 +1,11 @@
+using System.ComponentModel;
+
 namespace Terminal.Gui.ViewBase;
 
 public partial class View // Text Property APIs
 {
     private string _text = string.Empty;
 
-    /// <summary>
-    ///     Called when the <see cref="Text"/> has changed. Fires the <see cref="TextChanged"/> event.
-    /// </summary>
-    public void OnTextChanged () => TextChanged?.Invoke (this, EventArgs.Empty);
-
     /// <summary>
     ///     Gets or sets whether trailing spaces at the end of word-wrapped lines are preserved
     ///     or not when <see cref="Text.TextFormatter.WordWrap"/> is enabled.
@@ -50,7 +47,17 @@ public bool PreserveTrailingSpaces
     ///         If <see cref="View.Width"/> or <see cref="View.Height"/> are using <see cref="DimAutoStyle.Text"/>,
     ///         the <see cref="GetContentSize ()"/> will be adjusted to fit the text.
     ///     </para>
-    ///     <para>When the text changes, the <see cref="TextChanged"/> is fired.</para>
+    ///     <para>
+    ///         Setting <see cref="Text"/> to the same value as the current value is a no-op; neither
+    ///         <see cref="TextChanging"/> nor <see cref="TextChanged"/> will be raised.
+    ///     </para>
+    ///     <para>
+    ///         Before the text is changed, the <see cref="TextChanging"/> CWP hook is invoked. If cancelled,
+    ///         the text remains unchanged and <see cref="TextChanged"/> is not raised.
+    ///     </para>
+    ///     <para>
+    ///         After the text is changed, the <see cref="TextChanged"/> event is raised.
+    ///     </para>
     /// </remarks>
     public virtual string Text
     {
@@ -62,14 +69,79 @@ public virtual string Text
                 return;
             }
 
+            if (OnTextChanging ())
+            {
+                return;
+            }
+
+            CancelEventArgs args = new ();
+            TextChanging?.Invoke (this, args);
+
+            if (args.Cancel)
+            {
+                return;
+            }
+
             _text = value;
 
             UpdateTextFormatterText ();
             SetNeedsLayout ();
+
             OnTextChanged ();
         }
     }
 
+    /// <summary>
+    ///     Called before the <see cref="Text"/> changes. Invokes the <see cref="TextChanging"/> event, which can
+    ///     be cancelled.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         This is a signal-only notification. It does not carry old or new text values because
+    ///         <see cref="View.Text"/> semantics vary across derived views.
+    ///     </para>
+    /// </remarks>
+    /// <returns><see langword="true"/> if the text change should be cancelled; otherwise <see langword="false"/>.</returns>
+    protected virtual bool OnTextChanging () => false;
+
+    /// <summary>
+    ///     Raised when the <see cref="Text"/> is about to change. Set <see cref="CancelEventArgs.Cancel"/> to
+    ///     <see langword="true"/> to prevent the change.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         This is a signal-only notification at the <see cref="View"/> level. It does not carry old or new
+    ///         text values. Derived controls that need richer text-edit semantics may expose their own specific events.
+    ///     </para>
+    /// </remarks>
+    public event EventHandler<CancelEventArgs>? TextChanging;
+
+    /// <summary>
+    ///     Called after the <see cref="Text"/> has been changed. Raises the <see cref="TextChanged"/> event.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         This is a signal-only notification. It does not carry old or new text values because
+    ///         <see cref="View.Text"/> semantics vary across derived views.
+    ///     </para>
+    ///     <para>
+    ///         Derived views that override <see cref="Text"/> and do not call <c>base.Text</c> should call
+    ///         this method after mutating text to participate in the CWP workflow.
+    ///     </para>
+    /// </remarks>
+    protected virtual void OnTextChanged () => TextChanged?.Invoke (this, EventArgs.Empty);
+
+    /// <summary>
+    ///     Raised after the <see cref="Text"/> has been changed.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         This is a signal-only notification at the <see cref="View"/> level. It does not carry old or new
+    ///         text values. Derived controls that need richer text-edit semantics may expose their own specific events.
+    ///     </para>
+    /// </remarks>
+    public event EventHandler? TextChanged;
+
     /// <summary>
     ///     Gets or sets how the View's <see cref="Text"/> is aligned horizontally when drawn. Changing this property will
     ///     redisplay the <see cref="View"/>.
@@ -92,11 +164,6 @@ public Alignment TextAlignment
         }
     }
 
-    /// <summary>
-    ///     Text changed event, raised when the text has changed.
-    /// </summary>
-    public event EventHandler? TextChanged;
-
     /// <summary>
     ///     Gets or sets the direction of the View's <see cref="Text"/>. Changing this property will redisplay the
     ///     <see cref="View"/>.

Terminal.Gui/Views/TextInput/TextField/TextField.Text.cs

@@ -9,7 +9,7 @@ public partial class TextField
     private string? _lastPastedText;
 
     /// <summary>Raised before <see cref="Text"/> changes. The change can be canceled the text adjusted.</summary>
-    public event EventHandler<ResultEventArgs<string>>? TextChanging;
+    public new event EventHandler<ResultEventArgs<string>>? TextChanging;
 
     /// <summary>
     ///     Tracks whether the text field should be considered "used", that is, that the user has moved in the entry, so

Tests/UnitTestsParallelizable/ViewBase/TextCwpTests.cs

@@ -0,0 +1,151 @@
+// Copilot
+
+using System.ComponentModel;
+
+namespace ViewBaseTests;
+
+/// <summary>
+///     Tests for the CWP-compliant <see cref="View.Text"/> notifications:
+///     <see cref="View.TextChanging"/> and <see cref="View.TextChanged"/>.
+/// </summary>
+public class TextCwpTests
+{
+    [Fact]
+    public void Text_SetSameValue_NoEventsRaised ()
+    {
+        View view = new () { Text = "hello" };
+        bool changingRaised = false;
+        bool changedRaised = false;
+        view.TextChanging += (_, _) => changingRaised = true;
+        view.TextChanged += (_, _) => changedRaised = true;
+
+        view.Text = "hello";
+
+        Assert.False (changingRaised);
+        Assert.False (changedRaised);
+    }
+
+    [Fact]
+    public void Text_SetDifferentValue_BothEventsRaised ()
+    {
+        View view = new () { Text = "old" };
+        bool changingRaised = false;
+        bool changedRaised = false;
+        view.TextChanging += (_, _) => changingRaised = true;
+        view.TextChanged += (_, _) => changedRaised = true;
+
+        view.Text = "new";
+
+        Assert.True (changingRaised);
+        Assert.True (changedRaised);
+    }
+
+    [Fact]
+    public void TextChanging_Cancel_PreventsTextChange ()
+    {
+        View view = new () { Text = "original" };
+        view.TextChanging += (_, e) => e.Cancel = true;
+
+        view.Text = "modified";
+
+        Assert.Equal ("original", view.Text);
+    }
+
+    [Fact]
+    public void TextChanging_Cancel_SuppressesTextChanged ()
+    {
+        View view = new () { Text = "original" };
+        bool changedRaised = false;
+        view.TextChanging += (_, e) => e.Cancel = true;
+        view.TextChanged += (_, _) => changedRaised = true;
+
+        view.Text = "modified";
+
+        Assert.False (changedRaised);
+    }
+
+    [Fact]
+    public void TextChanging_RaisedBeforeMutation ()
+    {
+        View view = new () { Text = "before" };
+        string? textDuringChanging = null;
+        view.TextChanging += (sender, _) => textDuringChanging = ((View)sender!).Text;
+
+        view.Text = "after";
+
+        Assert.Equal ("before", textDuringChanging);
+    }
+
+    [Fact]
+    public void TextChanged_RaisedAfterMutation ()
+    {
+        View view = new () { Text = "before" };
+        string? textDuringChanged = null;
+        view.TextChanged += (sender, _) => textDuringChanged = ((View)sender!).Text;
+
+        view.Text = "after";
+
+        Assert.Equal ("after", textDuringChanged);
+    }
+
+    [Fact]
+    public void OnTextChanging_Override_CanCancel ()
+    {
+        CancellingView view = new ();
+        // Set initial text before enabling cancellation
+        view.AllowChange = true;
+        view.Text = "initial";
+        view.AllowChange = false;
+
+        view.Text = "blocked";
+
+        Assert.Equal ("initial", view.Text);
+    }
+
+    [Fact]
+    public void OnTextChanged_Override_CalledAfterChange ()
+    {
+        TrackingView view = new () { Text = "start" };
+
+        view.Text = "end";
+
+        Assert.True (view.OnTextChangedCalled);
+        Assert.Equal ("end", view.TextAtOnTextChanged);
+    }
+
+    [Fact]
+    public void TextChanging_EventOrder_ChangingBeforeChanged ()
+    {
+        View view = new () { Text = "a" };
+        List<string> order = [];
+        view.TextChanging += (_, _) => order.Add ("changing");
+        view.TextChanged += (_, _) => order.Add ("changed");
+
+        view.Text = "b";
+
+        Assert.Equal (["changing", "changed"], order);
+    }
+
+    /// <summary>A test subclass that cancels text changes via <see cref="View.OnTextChanging"/>.</summary>
+    private class CancellingView : View
+    {
+        public bool AllowChange { get; set; }
+
+        protected override bool OnTextChanging () => !AllowChange;
+    }
+
+    /// <summary>A test subclass that tracks calls to <see cref="View.OnTextChanged"/>.</summary>
+    private class TrackingView : View
+    {
+        public bool OnTextChangedCalled { get; private set; }
+        public string? TextAtOnTextChanged { get; private set; }
+
+        protected override void OnTextChanged ()
+        {
+            OnTextChangedCalled = true;
+            TextAtOnTextChanged = Text;
+
+            base.OnTextChanged ();
+        }
+    }
+}