Reduced duplicated code by leverating Navigationdirection enum

Terminal.Gui/Application/Application.Navigation.cs

@@ -36,7 +36,7 @@ internal static class ApplicationNavigation
     /// </summary>
     /// <param name="viewsInTabIndexes"></param>
     /// <param name="direction"></param>
-    internal static void FocusNearestView (IEnumerable<View>? viewsInTabIndexes, View.NavigationDirection direction)
+    internal static void FocusNearestView (IEnumerable<View>? viewsInTabIndexes, NavigationDirection direction)
     {
         if (viewsInTabIndexes is null)
         {
@@ -56,14 +56,7 @@ internal static class ApplicationNavigation
 
             if (found && v != Application.Current)
             {
-                if (direction == View.NavigationDirection.Forward)
-                {
-                    Application.Current!.SuperView?.FocusNext ();
-                }
-                else
-                {
-                    Application.Current!.SuperView?.FocusPrev ();
-                }
+                Application.Current!.SuperView?.AdvanceFocus (direction);
 
                 focusProcessed = true;
 
@@ -89,9 +82,9 @@ internal static class ApplicationNavigation
     {
         View? old = GetDeepestFocusedSubview (Application.Current!.Focused);
 
-        if (!Application.Current.FocusNext ())
+        if (!Application.Current.AdvanceFocus (NavigationDirection.Forward))
         {
-            Application.Current.FocusNext ();
+            Application.Current.AdvanceFocus (NavigationDirection.Forward);
         }
 
         if (old != Application.Current.Focused && old != Application.Current.Focused?.Focused)
@@ -101,7 +94,7 @@ internal static class ApplicationNavigation
         }
         else
         {
-            FocusNearestView (Application.Current.SuperView?.TabIndexes, View.NavigationDirection.Forward);
+            FocusNearestView (Application.Current.SuperView?.TabIndexes, NavigationDirection.Forward);
         }
     }
 
@@ -114,9 +107,9 @@ internal static class ApplicationNavigation
         {
             Toplevel? top = Application.Current!.Modal ? Application.Current : Application.Top;
 
-            if (!Application.Current.FocusNext ())
+            if (!Application.Current.AdvanceFocus (NavigationDirection.Forward))
             {
-                Application.Current.FocusNext ();
+                Application.Current.AdvanceFocus (NavigationDirection.Forward);
             }
 
             if (top != Application.Current.Focused && top != Application.Current.Focused?.Focused)
@@ -126,16 +119,16 @@ internal static class ApplicationNavigation
             }
             else
             {
-                FocusNearestView (Application.Current.SuperView?.TabIndexes, View.NavigationDirection.Forward);
+                FocusNearestView (Application.Current.SuperView?.TabIndexes, NavigationDirection.Forward);
             }
 
 
 
-            //top!.FocusNext ();
+            //top!.AdvanceFocus (NavigationDirection.Forward);
 
             //if (top.Focused is null)
             //{
-            //    top.FocusNext ();
+            //    top.AdvanceFocus (NavigationDirection.Forward);
             //}
 
             //top.SetNeedsDisplay ();
@@ -155,9 +148,9 @@ internal static class ApplicationNavigation
     {
         View? old = GetDeepestFocusedSubview (Application.Current!.Focused);
 
-        if (!Application.Current.FocusPrev ())
+        if (!Application.Current.AdvanceFocus (NavigationDirection.Backward))
         {
-            Application.Current.FocusPrev ();
+            Application.Current.AdvanceFocus (NavigationDirection.Backward);
         }
 
         if (old != Application.Current.Focused && old != Application.Current.Focused?.Focused)
@@ -167,7 +160,7 @@ internal static class ApplicationNavigation
         }
         else
         {
-            FocusNearestView (Application.Current.SuperView?.TabIndexes?.Reverse (), View.NavigationDirection.Backward);
+            FocusNearestView (Application.Current.SuperView?.TabIndexes?.Reverse (), NavigationDirection.Backward);
         }
     }
 
@@ -176,11 +169,11 @@ internal static class ApplicationNavigation
         if (ApplicationOverlapped.OverlappedTop is null)
         {
             Toplevel? top = Application.Current!.Modal ? Application.Current : Application.Top;
-            top!.FocusPrev ();
+            top!.AdvanceFocus (NavigationDirection.Backward);
 
             if (top.Focused is null)
             {
-                top.FocusPrev ();
+                top.AdvanceFocus (NavigationDirection.Backward);
             }
 
             top.SetNeedsDisplay ();

Terminal.Gui/View/NavigationDirection.cs

@@ -0,0 +1,13 @@
+namespace Terminal.Gui;
+
+/// <summary>
+///     Indicates navigation direction.
+/// </summary>
+public enum NavigationDirection
+{
+    /// <summary>Navigate forward.</summary>
+    Forward,
+
+    /// <summary>Navigate backwards.</summary>
+    Backward
+}

Terminal.Gui/View/View.Navigation.cs

@@ -7,16 +7,6 @@ public partial class View // Focus and cross-view navigation management (TabStop
     /// <summary>Returns a value indicating if this View is currently on Top (Active)</summary>
     public bool IsCurrentTop => Application.Current == this;
 
-    /// <summary>Exposed as `internal` for unit tests. Indicates focus navigation direction.</summary>
-    public enum NavigationDirection
-    {
-        /// <summary>Navigate forward.</summary>
-        Forward,
-
-        /// <summary>Navigate backwards.</summary>
-        Backward
-    }
-
     // BUGBUG: The focus API is poorly defined and implemented. It deeply intertwines the view hierarchy with the tab order.
 
     /// <summary>Invoked when this view is gaining focus (entering).</summary>
@@ -240,11 +230,11 @@ public partial class View // Focus and cross-view navigation management (TabStop
                 // If EnsureFocus () didn't set focus to a view, focus the next focusable view in the application
                 if (SuperView is { Focused: null })
                 {
-                    SuperView.FocusNext ();
+                    SuperView.AdvanceFocus (NavigationDirection.Forward);
 
                     if (SuperView.Focused is null && Application.Current is { })
                     {
-                        Application.Current.FocusNext ();
+                        Application.Current.AdvanceFocus (NavigationDirection.Forward);
                     }
 
                     ApplicationOverlapped.BringOverlappedTopToFront ();
@@ -460,7 +450,8 @@ public partial class View // Focus and cross-view navigation management (TabStop
     ///     <see cref="View.TabIndexes"/> then the focus is set to the view itself.
     /// </summary>
     /// <param name="overlappedOnly">
-    ///     If <see langword="true"/>, only subviews where <see cref="Arrangement"/> has <see cref="ViewArrangement.Overlapped"/> set
+    ///     If <see langword="true"/>, only subviews where <see cref="Arrangement"/> has
+    ///     <see cref="ViewArrangement.Overlapped"/> set
     ///     will be considered.
     /// </param>
     public void FocusFirst (bool overlappedOnly = false)
@@ -493,7 +484,8 @@ public partial class View // Focus and cross-view navigation management (TabStop
     ///     <see cref="View.TabIndexes"/> then the focus is set to the view itself.
     /// </summary>
     /// <param name="overlappedOnly">
-    ///     If <see langword="true"/>, only subviews where <see cref="Arrangement"/> has <see cref="ViewArrangement.Overlapped"/> set
+    ///     If <see langword="true"/>, only subviews where <see cref="Arrangement"/> has
+    ///     <see cref="ViewArrangement.Overlapped"/> set
     ///     will be considered.
     /// </param>
     public void FocusLast (bool overlappedOnly = false)
@@ -522,39 +514,28 @@ public partial class View // Focus and cross-view navigation management (TabStop
     }
 
     /// <summary>
-    ///     Advances the focus to the next or previous view in <see cref="View.TabIndexes"/>, based on <paramref name="direction"/>.
+    ///     Advances the focus to the next or previous view in <see cref="View.TabIndexes"/>, based on
+    ///     <paramref name="direction"/>.
     ///     itself.
     /// </summary>
     /// <remarks>
     ///     <para>
-    ///          If there is no next/previous view, the focus is set to the view itself.
+    ///         If there is no next/previous view, the focus is set to the view itself.
     ///     </para>
     /// </remarks>
     /// <param name="direction"></param>
-    /// <returns><see langword="true"/> if focus was changed to another subview (or stayed on this one), <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 AdvanceFocus (NavigationDirection direction)
-    {
-        return direction switch
-        {
-            NavigationDirection.Forward => FocusNext (),
-            NavigationDirection.Backward => FocusPrev (),
-            _ => false
-        };
-    }
-
-    /// <summary>
-    ///     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 focus was changed to another subview (or stayed on this one), <see langword="false"/> otherwise.</returns>
-    public bool FocusNext ()
     {
         if (!CanBeVisible (this))
         {
             return false;
         }
 
-        FocusDirection = NavigationDirection.Forward;
+        FocusDirection = direction;
 
         if (TabIndexes is null || TabIndexes.Count == 0)
         {
@@ -563,21 +544,33 @@ public partial class View // Focus and cross-view navigation management (TabStop
 
         if (Focused is null)
         {
-            FocusFirst ();
+            switch (direction)
+            {
+                case NavigationDirection.Forward:
+                    FocusFirst ();
+
+                    break;
+                case NavigationDirection.Backward:
+                    FocusLast ();
+
+                    break;
+                default:
+                    throw new ArgumentOutOfRangeException (nameof (direction), direction, null);
+            }
 
             return Focused is { };
         }
 
-        int focusedIdx = -1;
+        var focusedFound = false;
 
-        for (var i = 0; i < TabIndexes.Count; i++)
+        foreach (View w in direction == NavigationDirection.Forward
+                               ? TabIndexes.ToArray ()
+                               : TabIndexes.ToArray ().Reverse ())
         {
-            View w = TabIndexes [i];
-
             if (w.HasFocus)
             {
                 // A subview has focus, tell *it* to FocusNext
-                if (w.FocusNext ())
+                if (w.AdvanceFocus (direction))
                 {
                     // The subview changed which of it's subviews had focus
                     return true;
@@ -586,13 +579,13 @@ public partial class View // Focus and cross-view navigation management (TabStop
                 Debug.Assert (w.HasFocus);
 
                 // The subview has no subviews that can be next. Cache that we found a focused subview.
-                focusedIdx = i;
+                focusedFound = true;
 
                 continue;
             }
 
             // 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)
+            if (focusedFound && w.CanFocus && w._tabStop && w.Visible && w.Enabled)
             {
                 // Make Focused Leave
                 Focused.SetHasFocus (false, w);
@@ -609,104 +602,16 @@ 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)
+                switch (direction)
                 {
-                    w.FocusFirst ();
-                }
+                    case NavigationDirection.Forward:
+                        w.FocusFirst ();
 
-                SetFocus (w);
-
-                return true;
-            }
-        }
-
-        // There's no next view in tab indexes.
-        if (Focused is { })
-        {
-            // Leave
-            Focused.SetHasFocus (false, this);
-
-            //if (Focused.Arrangement.HasFlag (ViewArrangement.Overlapped))
-            //{
-            //    FocusFirst (true);
-            //    return true;
-            //}
-
-            // 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;
-        }
-
-        return false;
-    }
-
-    /// <summary>
-    ///     Focuses the previous view in <see cref="View.TabIndexes"/>. If there is no previous view, the focus is set to the
-    ///     view itself.
-    /// </summary>
-    /// <returns><see langword="true"/> if previous was focused, <see langword="false"/> otherwise.</returns>
-    public bool FocusPrev ()
-    {
-        if (!CanBeVisible (this))
-        {
-            return false;
-        }
+                        break;
+                    case NavigationDirection.Backward:
+                        w.FocusLast ();
 
-        FocusDirection = NavigationDirection.Backward;
-
-        if (TabIndexes is null || TabIndexes.Count == 0)
-        {
-            return false;
-        }
-
-        if (Focused is null)
-        {
-            FocusLast ();
-
-            return Focused != null;
-        }
-
-        int focusedIdx = -1;
-
-        for (int i = TabIndexes.Count; i > 0;)
-        {
-            i--;
-            View w = TabIndexes [i];
-
-            if (w.HasFocus)
-            {
-                if (w.FocusPrev ())
-                {
-                    return true;
-                }
-
-                focusedIdx = i;
-
-                continue;
-            }
-
-            if (w.CanFocus && focusedIdx != -1 && w._tabStop && w.Visible && w.Enabled)
-            {
-                Focused.SetHasFocus (false, w);
-
-                // If the focused view is overlapped don't focus on the next if it's not overlapped.
-                if (Focused.Arrangement.HasFlag (ViewArrangement.Overlapped) && !w.Arrangement.HasFlag (ViewArrangement.Overlapped))
-                {
-                    FocusLast (true);
-
-                    return true;
-                }
-
-                // If the focused view is not overlapped and the next is, skip it
-                if (!Focused.Arrangement.HasFlag (ViewArrangement.Overlapped) && w.Arrangement.HasFlag (ViewArrangement.Overlapped))
-                {
-                    continue;
-                }
-
-                if (w.CanFocus && w._tabStop && w.Visible && w.Enabled)
-                {
-                    w.FocusLast ();
+                        break;
                 }
 
                 SetFocus (w);
@@ -715,20 +620,9 @@ public partial class View // Focus and cross-view navigation management (TabStop
             }
         }
 
-        // There's no prev view in tab indexes.
         if (Focused is { })
         {
-            // Leave Focused
             Focused.SetHasFocus (false, this);
-
-            if (Focused.Arrangement.HasFlag (ViewArrangement.Overlapped))
-            {
-                FocusLast (true);
-
-                return true;
-            }
-
-            // Signal to caller no next view was found
             Focused = null;
         }
 

Terminal.Gui/Views/ComboBox.cs

@@ -520,7 +520,7 @@ public class ComboBox : View, IDesignable
             else
             {
                 _listview.TabStop = false;
-                SuperView?.FocusNext ();
+                SuperView?.AdvanceFocus (NavigationDirection.Forward);
             }
 
             return true;

Terminal.Gui/Views/TabView.cs

@@ -95,7 +95,7 @@ public class TabView : View
                     Command.PreviousView,
                     () =>
                     {
-                        SuperView?.FocusPrev ();
+                        SuperView?.AdvanceFocus (NavigationDirection.Backward);
 
                         return true;
                     }

UICatalog/Scenarios/Notepad.cs

@@ -310,7 +310,7 @@ public class Notepad : Scenario
         newTile.ContentView.Add (newTabView);
 
         newTabView.FocusFirst ();
-        newTabView.FocusNext ();
+        newTabView.AdvanceFocus (NavigationDirection.Forward);
     }
 
     private void SplitDown (TabView sender, OpenedFile tab) { Split (1, Orientation.Horizontal, sender, tab); }

UnitTests/View/NavigationTests.cs

@@ -614,8 +614,7 @@ public class NavigationTests (ITestOutputHelper output) : TestsAllViews
         Assert.True (removed);
         Assert.NotNull (view3);
 
-        Exception exception =
-            Record.Exception (() => Application.OnKeyDown (Key.Tab.WithCtrl));
+        Exception exception = Record.Exception (() => Application.OnKeyDown (Key.Tab.WithCtrl));
         Assert.Null (exception);
         Assert.True (removed);
         Assert.Null (view3);
@@ -1380,23 +1379,23 @@ public class NavigationTests (ITestOutputHelper output) : TestsAllViews
 
         r.Add (v1, v2, v3);
 
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
 
         v1.TabStop = true;
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.True (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
         v2.TabStop = true;
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.True (v2.HasFocus);
         Assert.False (v3.HasFocus);
         v3.TabStop = true;
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.True (v3.HasFocus);
@@ -1413,23 +1412,23 @@ public class NavigationTests (ITestOutputHelper output) : TestsAllViews
 
         r.Add (v1, v2, v3);
 
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
 
         v1.CanFocus = true;
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.True (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
         v2.CanFocus = true;
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.True (v2.HasFocus);
         Assert.False (v3.HasFocus);
         v3.CanFocus = true;
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.True (v3.HasFocus);
@@ -1446,15 +1445,15 @@ public class NavigationTests (ITestOutputHelper output) : TestsAllViews
 
         r.Add (v1, v2, v3);
 
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.True (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.True (v2.HasFocus);
         Assert.False (v3.HasFocus);
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.True (v3.HasFocus);
@@ -1471,15 +1470,15 @@ public class NavigationTests (ITestOutputHelper output) : TestsAllViews
 
         r.Add (v1, v2, v3);
 
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
@@ -1496,15 +1495,15 @@ public class NavigationTests (ITestOutputHelper output) : TestsAllViews
 
         r.Add (v1, v2, v3);
 
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
@@ -1521,15 +1520,15 @@ public class NavigationTests (ITestOutputHelper output) : TestsAllViews
 
         r.Add (v1, v2, v3);
 
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);
-        r.FocusNext ();
+        r.AdvanceFocus (NavigationDirection.Forward);
         Assert.False (v1.HasFocus);
         Assert.False (v2.HasFocus);
         Assert.False (v3.HasFocus);

UnitTests/Views/DatePickerTests.cs

@@ -54,9 +54,9 @@ public class DatePickerTests
         Application.Begin (top);
 
         // Set focus to next month button
-        datePicker.FocusNext ();
-        datePicker.FocusNext ();
-        datePicker.FocusNext ();
+        datePicker.AdvanceFocus (NavigationDirection.Forward);
+        datePicker.AdvanceFocus (NavigationDirection.Forward);
+        datePicker.AdvanceFocus (NavigationDirection.Forward);
 
         // Change month to December
         Assert.True (datePicker.NewKeyDownEvent (Key.Enter));
@@ -81,8 +81,8 @@ public class DatePickerTests
         Application.Begin (top);
 
         // set focus to the previous month button
-        datePicker.FocusNext ();
-        datePicker.FocusNext ();
+        datePicker.AdvanceFocus (NavigationDirection.Forward);
+        datePicker.AdvanceFocus (NavigationDirection.Forward);
 
         // Change month to January 
         Assert.True (datePicker.NewKeyDownEvent (Key.Enter));