From ccec0eec1191140c945c6419a03e57126793f910 Mon Sep 17 00:00:00 2001
From: Tig <tig@users.noreply.github.com>
Date: Thu, 25 Jul 2024 12:16:10 -0600
Subject: [PATCH] Documenting focus code

---
 Terminal.Gui/View/View.Navigation.cs    | 21 ++++++++++++++++-----
 Terminal.Gui/Views/Toplevel.cs          |  2 +-
 UnitTests/Views/TextFieldTests.cs       |  6 +++---
 UnitTests/Views/TreeTableSourceTests.cs |  2 +-
 4 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/Terminal.Gui/View/View.Navigation.cs b/Terminal.Gui/View/View.Navigation.cs
index 3cc0c1f11..edbe5eabc 100644
--- a/Terminal.Gui/View/View.Navigation.cs
+++ b/Terminal.Gui/View/View.Navigation.cs
@@ -1,3 +1,5 @@
+using System.Diagnostics;
+
 namespace Terminal.Gui;
 
 public partial class View // Focus and cross-view navigation management (TabStop, TabIndex, etc...)
@@ -603,7 +605,7 @@ public partial class View // Focus and cross-view navigation management (TabStop
     ///     Focuses the next view in <see cref="View.TabIndexes"/>. If there is no next view, the focus is set to the view
     ///     itself.
     /// </summary>
-    /// <returns><see langword="true"/> if next was focused, <see langword="false"/> otherwise.</returns>
+    /// <returns><see langword="true"/> if focus was changed to another subview (or stayed on this one), <see langword="false"/> otherwise.</returns>
     public bool FocusNext ()
     {
         if (!CanBeVisible (this))
@@ -622,7 +624,7 @@ public partial class View // Focus and cross-view navigation management (TabStop
         {
             FocusFirst ();
 
-            return Focused != null;
+            return Focused is { };
         }
 
         int focusedIdx = -1;
@@ -633,18 +635,25 @@ public partial class View // Focus and cross-view navigation management (TabStop
 
             if (w.HasFocus)
             {
+                // A subview has focus, tell *it* to FocusNext
                 if (w.FocusNext ())
                 {
+                    // The subview changed which of it's subviews had focus
                     return true;
                 }
 
+                Debug.Assert (w.HasFocus);
+
+                // The subview has no subviews that can be next. Cache that we found a focused subview.
                 focusedIdx = i;
 
                 continue;
             }
 
-            if (w.CanFocus && focusedIdx != -1 && w._tabStop && w.Visible && w.Enabled)
+            // The subview does not have focus, but at least one other that can. Can this one be focused?
+            if (focusedIdx != -1 && w.CanFocus && w._tabStop && w.Visible && w.Enabled)
             {
+                // Make w Leave
                 Focused.SetHasFocus (false, w);
 
                 //// If the focused view is overlapped don't focus on the next if it's not overlapped.
@@ -659,6 +668,7 @@ public partial class View // Focus and cross-view navigation management (TabStop
                 //    continue;
                 //}
 
+                // QUESTION: Why do we check these again here?
                 if (w.CanFocus && w._tabStop && w.Visible && w.Enabled)
                 {
                     w.FocusFirst ();
@@ -673,7 +683,7 @@ public partial class View // Focus and cross-view navigation management (TabStop
         // There's no next view in tab indexes.
         if (Focused is { })
         {
-            // Leave Focused
+            // Leave
             Focused.SetHasFocus (false, this);
 
             //if (Focused.Arrangement.HasFlag (ViewArrangement.Overlapped))
@@ -682,7 +692,8 @@ public partial class View // Focus and cross-view navigation management (TabStop
             //    return true;
             //}
 
-            // Signal to caller no next view was found
+            // Signal to caller no next view was found; this will enable it to make a peer
+            // or view up the superview hierarchy have focus.
             Focused = null;
         }
 
diff --git a/Terminal.Gui/Views/Toplevel.cs b/Terminal.Gui/Views/Toplevel.cs
index 26fe724c8..435bd6fc7 100644
--- a/Terminal.Gui/Views/Toplevel.cs
+++ b/Terminal.Gui/Views/Toplevel.cs
@@ -430,7 +430,7 @@ public partial class Toplevel : View
         {
             if (Focused is null)
             {
-                EnsureFocus ();
+                FocusFirstOrLast ();
             }
 
             return null;
diff --git a/UnitTests/Views/TextFieldTests.cs b/UnitTests/Views/TextFieldTests.cs
index ed3a35775..4096ee404 100644
--- a/UnitTests/Views/TextFieldTests.cs
+++ b/UnitTests/Views/TextFieldTests.cs
@@ -78,7 +78,7 @@ public class TextFieldTests (ITestOutputHelper output)
     public void Cancel_TextChanging_ThenBackspace ()
     {
         var tf = new TextField ();
-        tf.EnsureFocus ();
+        tf.FocusFirstOrLast ();
         tf.NewKeyDownEvent (Key.A.WithShift);
         Assert.Equal ("A", tf.Text);
 
@@ -929,7 +929,7 @@ public class TextFieldTests (ITestOutputHelper output)
     public void Backspace_From_End ()
     {
         var tf = new TextField { Text = "ABC" };
-        tf.EnsureFocus ();
+        tf.FocusFirstOrLast ();
         Assert.Equal ("ABC", tf.Text);
         tf.BeginInit ();
         tf.EndInit ();
@@ -956,7 +956,7 @@ public class TextFieldTests (ITestOutputHelper output)
     public void Backspace_From_Middle ()
     {
         var tf = new TextField { Text = "ABC" };
-        tf.EnsureFocus ();
+        tf.FocusFirstOrLast ();
         tf.CursorPosition = 2;
         Assert.Equal ("ABC", tf.Text);
 
diff --git a/UnitTests/Views/TreeTableSourceTests.cs b/UnitTests/Views/TreeTableSourceTests.cs
index a1a319b1b..02625b6cf 100644
--- a/UnitTests/Views/TreeTableSourceTests.cs
+++ b/UnitTests/Views/TreeTableSourceTests.cs
@@ -289,7 +289,7 @@ public class TreeTableSourceTests : IDisposable
 
         var top = new Toplevel ();
         top.Add (tableView);
-        top.EnsureFocus ();
+        top.FocusFirstOrLast ();
         Assert.Equal (tableView, top.MostFocused);
 
         return tableView;