Przeglądaj źródła

Fix AdvanceFocus not cycling properly through peers with nested subviews (#4267)

* Initial plan

* Implement fix for AdvanceFocus nested subview navigation

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

* Refactor ShouldBubbleUpForWrapping to use iteration instead of recursion

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]>
Copilot 2 miesięcy temu
rodzic
commit
c8147416b2

+ 40 - 1
Terminal.Gui/ViewBase/View.Navigation.cs

@@ -92,7 +92,7 @@ public partial class View // Focus and cross-view navigation management (TabStop
             if (SuperView is { })
             {
                 // If we are TabStop, and we have at least one other focusable peer, move to the SuperView's chain
-                if (TabStop == TabBehavior.TabStop && SuperView is { } && SuperView.GetFocusChain (direction, behavior).Length > 1)
+                if (TabStop == TabBehavior.TabStop && SuperView is { } && ShouldBubbleUpForWrapping (SuperView, direction, behavior))
                 {
                     return false;
                 }
@@ -171,6 +171,45 @@ public partial class View // Focus and cross-view navigation management (TabStop
         }
     }
 
+    /// <summary>
+    ///     Determines if focus should bubble up to a SuperView when wrapping would occur.
+    ///     Iteratively checks up the SuperView hierarchy to see if there are any focusable peers at any level.
+    /// </summary>
+    /// <param name="view">The SuperView to check.</param>
+    /// <param name="direction">The navigation direction.</param>
+    /// <param name="behavior">The tab behavior to filter by.</param>
+    /// <returns>
+    ///     <see langword="true"/> if there are focusable peers at this level or any ancestor level,
+    ///     <see langword="false"/> otherwise.
+    /// </returns>
+    private bool ShouldBubbleUpForWrapping (View? view, NavigationDirection direction, TabBehavior? behavior)
+    {
+        View? currentView = view;
+
+        while (currentView is { })
+        {
+            // If this parent has multiple focusable children, we should bubble up
+            View [] chain = currentView.GetFocusChain (direction, behavior);
+
+            if (chain.Length > 1)
+            {
+                return true;
+            }
+
+            // If parent has only 1 child but parent is also TabStop with a SuperView, continue checking up the hierarchy
+            if (currentView.TabStop == TabBehavior.TabStop && currentView.SuperView is { })
+            {
+                currentView = currentView.SuperView;
+            }
+            else
+            {
+                break;
+            }
+        }
+
+        return false;
+    }
+
     private bool RaiseAdvancingFocus (NavigationDirection direction, TabBehavior? behavior)
     {
         // Call the virtual method

+ 44 - 0
Tests/UnitTestsParallelizable/View/Navigation/AdvanceFocusTests.cs

@@ -644,4 +644,48 @@ public class AdvanceFocusTests ()
         Assert.Equal (canFocus, view.CanFocus);
         Assert.Equal (tabStop, view.TabStop);
     }
+
+    [Fact]
+    public void AdvanceFocus_Cycles_Through_Peers_And_All_Nested_SubViews_When_Multiple ()
+    {
+        var top = new View { Id = "top", CanFocus = true };
+
+        View peer1 = new View
+        {
+            CanFocus = true,
+            Id = "peer1",
+        };
+
+        var peer2 = new View
+        {
+            CanFocus = true,
+            Id = "peer2",
+        };
+        var peer2SubView = new View
+        {
+            Id = "peer2SubView", CanFocus = true
+        };
+        var v1 = new View { Id = "v1", CanFocus = true };
+        var v2 = new View { Id = "v2", CanFocus = true };
+        peer2SubView.Add (v1, v2);
+
+        peer2.Add (peer2SubView);
+
+        top.Add (peer1, peer2);
+        top.SetFocus ();
+
+        Assert.Equal (peer1, top.MostFocused);
+
+        top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop);
+        Assert.Equal (v1, top.MostFocused);
+
+        top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop);
+        Assert.Equal (v2, top.MostFocused);
+
+        // This should cycle to peer1 - previously it incorrectly cycled to v1
+        top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop);
+        Assert.Equal (peer1, top.MostFocused);
+
+        top.Dispose ();
+    }
 }