From 1359e43066f3244f4b5385e80b8ab15af30540d5 Mon Sep 17 00:00:00 2001 From: Tig Date: Wed, 28 Aug 2024 23:47:43 -0700 Subject: [PATCH] Fixed nested tabgroup nav! --- .../Application/Application.Keyboard.cs | 32 +++++- Terminal.Gui/View/View.Navigation.cs | 63 ++++++++++-- Terminal.Gui/Views/DatePicker.cs | 1 - UICatalog/Scenarios/Navigation.cs | 10 +- .../View/Navigation/AdvanceFocusTests.cs | 97 ++++++++++++++++--- UnitTests/View/Navigation/NavigationTests.cs | 47 +-------- 6 files changed, 179 insertions(+), 71 deletions(-) diff --git a/Terminal.Gui/Application/Application.Keyboard.cs b/Terminal.Gui/Application/Application.Keyboard.cs index dc11d02c8..f468a045a 100644 --- a/Terminal.Gui/Application/Application.Keyboard.cs +++ b/Terminal.Gui/Application/Application.Keyboard.cs @@ -300,7 +300,21 @@ public static partial class Application // Keyboard handling // TODO: This OverlapppedTop tomfoolery goes away in addressing #2491 if (ApplicationOverlapped.OverlappedTop is null && Current is { }) { - return Current.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabGroup); + if (Current.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabGroup)) + { + return true; + }; + + //// Go back down the focus chain and focus the first TabGroup + //View []? views = Current.GetSubviewFocusChain (NavigationDirection.Forward, TabBehavior.TabGroup); + + //if (views.Length > 0) + //{ + // View []? subViews = views [0].GetSubviewFocusChain (NavigationDirection.Forward, TabBehavior.TabStop); + // return subViews? [0].SetFocus (); + //} + + return false; } ApplicationOverlapped.OverlappedMoveNext (); @@ -316,7 +330,21 @@ public static partial class Application // Keyboard handling // TODO: This OverlapppedTop tomfoolery goes away in addressing #2491 if (ApplicationOverlapped.OverlappedTop is null && Current is { }) { - return Current.AdvanceFocus (NavigationDirection.Backward, TabBehavior.TabGroup); + if (Current.AdvanceFocus (NavigationDirection.Backward, TabBehavior.TabGroup)) + { + return true; + }; + + //// Go back down the focus chain and focus the first TabGroup + //View []? views = Current.GetSubviewFocusChain (NavigationDirection.Backward, TabBehavior.TabGroup); + + //if (views.Length > 0) + //{ + // View []? subViews = views [0].GetSubviewFocusChain (NavigationDirection.Backward, TabBehavior.TabStop); + // return subViews? [0].SetFocus (); + //} + + return false; } ApplicationOverlapped.OverlappedMovePrevious (); diff --git a/Terminal.Gui/View/View.Navigation.cs b/Terminal.Gui/View/View.Navigation.cs index 046026afe..1ff5decde 100644 --- a/Terminal.Gui/View/View.Navigation.cs +++ b/Terminal.Gui/View/View.Navigation.cs @@ -37,6 +37,7 @@ public partial class View // Focus and cross-view navigation management (TabStop return true; } + // AdvanceFocus did not advance View [] index = GetSubviewFocusChain (direction, behavior); if (index.Length == 0) @@ -44,6 +45,49 @@ public partial class View // Focus and cross-view navigation management (TabStop return false; } + if (behavior == TabBehavior.TabGroup) + { + if (direction == NavigationDirection.Forward && focused == index [^1] && SuperView is null) + { + // We're at the top of the focus chain. Go back down the focus chain and focus the first TabGroup + View [] views = GetSubviewFocusChain (NavigationDirection.Forward, TabBehavior.TabGroup); + + if (views.Length > 0) + { + View [] subViews = views [0].GetSubviewFocusChain (NavigationDirection.Forward, TabBehavior.TabStop); + + if (subViews.Length > 0) + { + if (subViews [0].SetFocus ()) + { + return true; + } + } + } + } + + if (direction == NavigationDirection.Backward && focused == index [0]) + { + // We're at the bottom of the focus chain + View [] views = GetSubviewFocusChain (NavigationDirection.Forward, TabBehavior.TabGroup); + + if (views.Length > 0) + { + View [] subViews = views [^1].GetSubviewFocusChain (NavigationDirection.Forward, TabBehavior.TabStop); + + if (subViews.Length > 0) + { + if (subViews [0].SetFocus ()) + { + return true; + } + } + } + } + + } + + int focusedIndex = index.IndexOf (Focused); // Will return -1 if Focused can't be found or is null var next = 0; @@ -57,21 +101,20 @@ public partial class View // Focus and cross-view navigation management (TabStop // We're moving beyond the last subview // Determine if focus should remain in this focus chain, or move to the superview's focus chain - // BUGBUG: The logic below is sketchy and barely works. In fact, it doesn't work propertly for all nested TabGroups. - // - If we are TabStop and our SuperView has at least one other TabStop subview, move to the SuperView's chain + + // If we are TabStop and our SuperView has at least one other TabStop subview, move to the SuperView's chain if (TabStop == TabBehavior.TabStop && SuperView is { } && SuperView.GetSubviewFocusChain (direction, behavior).Length > 1) { return false; } - // - If we are TabGroup and our SuperView has at least one other TabGroup subview, move to the SuperView's chain - if (TabStop == TabBehavior.TabGroup && SuperView is { TabStop: TabBehavior.TabGroup }) + // TabGroup is special-cased. + if (focused?.TabStop == TabBehavior.TabGroup) { - if (behavior == TabBehavior.TabGroup) + if (SuperView?.GetSubviewFocusChain (direction, TabBehavior.TabGroup)?.Length > 0) { - // Wrap to first focusable views - // BUGBUG: This should do a Restore Focus instead - index = GetSubviewFocusChain (direction, null); + // Our superview has a TabGroup subview; signal we couldn't move so we nav out to it + return false; } } } @@ -560,7 +603,7 @@ public partial class View // Focus and cross-view navigation management (TabStop if (appFocused is { } || appFocused == this) { - Application.Navigation.SetFocused (newFocusedView ?? SuperView); + Application.Navigation.SetFocused (newFocusedView ?? SuperView); } } @@ -635,7 +678,7 @@ public partial class View // Focus and cross-view navigation management (TabStop /// /// /// GetScopedTabIndexes - private View [] GetSubviewFocusChain (NavigationDirection direction, TabBehavior? behavior) + internal View [] GetSubviewFocusChain (NavigationDirection direction, TabBehavior? behavior) { IEnumerable? fitleredSubviews; diff --git a/Terminal.Gui/Views/DatePicker.cs b/Terminal.Gui/Views/DatePicker.cs index 6d25a915a..1676f6df2 100644 --- a/Terminal.Gui/Views/DatePicker.cs +++ b/Terminal.Gui/Views/DatePicker.cs @@ -189,7 +189,6 @@ public class DatePicker : View Date = date; _dateLabel = new Label { X = 0, Y = 0, Text = "Date: " }; CanFocus = true; - TabStop = TabBehavior.TabGroup; _calendar = new TableView { diff --git a/UICatalog/Scenarios/Navigation.cs b/UICatalog/Scenarios/Navigation.cs index 71be6e285..404124b84 100644 --- a/UICatalog/Scenarios/Navigation.cs +++ b/UICatalog/Scenarios/Navigation.cs @@ -64,15 +64,19 @@ public class Navigation : Scenario View overlappedView2 = CreateOverlappedView (3, Pos.Center () + 10, Pos.Center () + 5); - // BUGBUG: F6 through nested tab groups doesn't work yet. -#if NESTED_TABGROUPS var overlappedInOverlapped1 = CreateOverlappedView (4, 1, 4); overlappedView2.Add (overlappedInOverlapped1); var overlappedInOverlapped2 = CreateOverlappedView (5, 10, 7); overlappedView2.Add (overlappedInOverlapped2); -#endif + CheckBox cb = new () + { + X = Pos.AnchorEnd (), + Y = Pos.AnchorEnd (), + Title = "Checkbo_x" + }; + overlappedView2.Add (cb); testFrame.Add (overlappedView1); testFrame.Add (overlappedView2); diff --git a/UnitTests/View/Navigation/AdvanceFocusTests.cs b/UnitTests/View/Navigation/AdvanceFocusTests.cs index b3a2e00da..63e7843cc 100644 --- a/UnitTests/View/Navigation/AdvanceFocusTests.cs +++ b/UnitTests/View/Navigation/AdvanceFocusTests.cs @@ -73,33 +73,35 @@ public class AdvanceFocusTests () } [Fact] - public void AdvanceFocus_Compound_Subview () + public void AdvanceFocus_Compound_Subview_TabStop () { + TabBehavior behavior = TabBehavior.TabStop; var top = new View { Id = "top", CanFocus = true }; var compoundSubview = new View { CanFocus = true, - Id = "compoundSubview" + Id = "compoundSubview", + TabStop = behavior }; - var v1 = new View { Id = "v1", CanFocus = true }; - var v2 = new View { Id = "v2", CanFocus = true }; - var v3 = new View { Id = "v3", CanFocus = false }; + var v1 = new View { Id = "v1", CanFocus = true, TabStop = behavior }; + var v2 = new View { Id = "v2", CanFocus = true, TabStop = behavior }; + var v3 = new View { Id = "v3", CanFocus = false, TabStop = behavior }; compoundSubview.Add (v1, v2, v3); top.Add (compoundSubview); // Cycle through v1 & v2 - top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + top.AdvanceFocus (NavigationDirection.Forward, behavior); Assert.True (v1.HasFocus); Assert.False (v2.HasFocus); Assert.False (v3.HasFocus); - top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + top.AdvanceFocus (NavigationDirection.Forward, behavior); Assert.False (v1.HasFocus); Assert.True (v2.HasFocus); Assert.False (v3.HasFocus); - top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + top.AdvanceFocus (NavigationDirection.Forward, behavior); Assert.True (v1.HasFocus); Assert.False (v2.HasFocus); Assert.False (v3.HasFocus); @@ -108,6 +110,7 @@ public class AdvanceFocusTests () View otherSubview = new () { CanFocus = true, + TabStop = behavior, Id = "otherSubview" }; @@ -118,15 +121,15 @@ public class AdvanceFocusTests () Assert.False (v1.HasFocus); // Cycle through v1 & v2 - top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + top.AdvanceFocus (NavigationDirection.Forward, behavior); Assert.True (v1.HasFocus); Assert.False (v2.HasFocus); Assert.False (v3.HasFocus); - top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + top.AdvanceFocus (NavigationDirection.Forward, behavior); Assert.False (v1.HasFocus); Assert.True (v2.HasFocus); Assert.False (v3.HasFocus); - top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + top.AdvanceFocus (NavigationDirection.Forward, behavior); Assert.False (v1.HasFocus); Assert.False (v2.HasFocus); Assert.False (v3.HasFocus); @@ -134,7 +137,7 @@ public class AdvanceFocusTests () Assert.True (otherSubview.HasFocus); // v2 was previously focused down the compoundSubView focus chain - top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + top.AdvanceFocus (NavigationDirection.Forward, behavior); Assert.False (v1.HasFocus); Assert.True (v2.HasFocus); Assert.False (v3.HasFocus); @@ -142,6 +145,76 @@ public class AdvanceFocusTests () top.Dispose (); } + [Fact] + public void AdvanceFocus_Compound_Subview_TabGroup () + { + var top = new View { Id = "top", CanFocus = true, TabStop = TabBehavior.TabGroup }; + + var compoundSubview = new View + { + CanFocus = true, + Id = "compoundSubview", + TabStop = TabBehavior.TabGroup + }; + var tabStopView = new View { Id = "tabStop", CanFocus = true, TabStop = TabBehavior.TabStop }; + var tabGroupView1 = new View { Id = "tabGroup1", CanFocus = true, TabStop = TabBehavior.TabGroup }; + var tabGroupView2 = new View { Id = "tabGroup2", CanFocus = true, TabStop = TabBehavior.TabGroup }; + + compoundSubview.Add (tabStopView, tabGroupView1, tabGroupView2); + + top.Add (compoundSubview); + top.SetFocus (); + Assert.True (tabStopView.HasFocus); + + // TabGroup should cycle to tabGroup1 then tabGroup2 + top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabGroup); + Assert.False (tabStopView.HasFocus); + Assert.True (tabGroupView1.HasFocus); + Assert.False (tabGroupView2.HasFocus); + top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabGroup); + Assert.False (tabStopView.HasFocus); + Assert.False (tabGroupView1.HasFocus); + Assert.True (tabGroupView2.HasFocus); + + // Add another TabGroup subview + View otherTabGroupSubview = new () + { + CanFocus = true, + TabStop = TabBehavior.TabGroup, + Id = "otherTabGroupSubview" + }; + + top.Add (otherTabGroupSubview); + + // Adding a focusable subview causes advancefocus + Assert.True (otherTabGroupSubview.HasFocus); + Assert.False (tabStopView.HasFocus); + + // TagBroup navs to the other subview + top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabGroup); + Assert.Equal (compoundSubview, top.Focused); + Assert.True (tabStopView.HasFocus); + + top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabGroup); + Assert.Equal (compoundSubview, top.Focused); + Assert.True (tabGroupView1.HasFocus); + + top.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabGroup); + Assert.Equal (compoundSubview, top.Focused); + Assert.True (tabGroupView2.HasFocus); + + // Now go backwards + top.AdvanceFocus (NavigationDirection.Backward, TabBehavior.TabGroup); + Assert.Equal (compoundSubview, top.Focused); + Assert.True (tabGroupView1.HasFocus); + + top.AdvanceFocus (NavigationDirection.Backward, TabBehavior.TabGroup); + Assert.Equal (otherTabGroupSubview, top.Focused); + Assert.True (otherTabGroupSubview.HasFocus); + + top.Dispose (); + } + [Fact] public void AdvanceFocus_NoStop_And_CanFocus_True_No_Focus () { diff --git a/UnitTests/View/Navigation/NavigationTests.cs b/UnitTests/View/Navigation/NavigationTests.cs index 2e4b56170..306e7739e 100644 --- a/UnitTests/View/Navigation/NavigationTests.cs +++ b/UnitTests/View/Navigation/NavigationTests.cs @@ -206,7 +206,10 @@ public class NavigationTests (ITestOutputHelper _output) : TestsAllViews break; case TabBehavior.TabGroup: - Application.OnKeyDown (Key.F6); + if (!Application.OnKeyDown (Key.F6)) + { + view.SetFocus (); + } break; case null: @@ -316,7 +319,6 @@ public class NavigationTests (ITestOutputHelper _output) : TestsAllViews // View.Focused - No subviews [Fact] - [Trait ("BUGBUG", "Fix in Issue #3444")] public void Focused_NoSubviews () { var view = new View (); @@ -324,47 +326,6 @@ public class NavigationTests (ITestOutputHelper _output) : TestsAllViews view.CanFocus = true; view.SetFocus (); - Assert.True (view.HasFocus); - Assert.Null (view.Focused); // BUGBUG: Should be view - } - - [Fact] - public void FocusNearestView_Ensure_Focus_Ordered () - { - Application.Top = Application.Current = new Toplevel (); - - var win = new Window (); - var winSubview = new View { CanFocus = true, Text = "WindowSubview" }; - win.Add (winSubview); - Application.Current.Add (win); - - var frm = new FrameView (); - var frmSubview = new View { CanFocus = true, Text = "FrameSubview" }; - frm.Add (frmSubview); - Application.Current.Add (frm); - Application.Current.SetFocus (); - - Assert.Equal (winSubview, Application.Current.MostFocused); - - Application.OnKeyDown (Key.Tab); // Move to the next TabStop. There is none. So we should stay. - Assert.Equal (winSubview, Application.Current.MostFocused); - - Application.OnKeyDown (Key.F6); - Assert.Equal (frmSubview, Application.Current.MostFocused); - - Application.OnKeyDown (Key.Tab); - Assert.Equal (frmSubview, Application.Current.MostFocused); - - Application.OnKeyDown (Key.F6); - Assert.Equal (winSubview, Application.Current.MostFocused); - - Application.OnKeyDown (Key.F6.WithShift); - Assert.Equal (frmSubview, Application.Current.MostFocused); - - Application.OnKeyDown (Key.F6.WithShift); - Assert.Equal (winSubview, Application.Current.MostFocused); - - Application.Current.Dispose (); } [Fact]