From 66f83ad2e60960aa2b8e1b0a191dfcf4fcb197ae Mon Sep 17 00:00:00 2001 From: Tig Date: Thu, 25 Jul 2024 16:37:22 -0600 Subject: [PATCH] Reduced duplicated code by leverating Navigationdirection enum --- .../Application/Application.Navigation.cs | 37 ++-- ...ation .Screen.cs => Application.Screen.cs} | 0 Terminal.Gui/View/NavigationDirection.cs | 13 ++ Terminal.Gui/View/View.Navigation.cs | 192 ++++-------------- Terminal.Gui/Views/ComboBox.cs | 2 +- Terminal.Gui/Views/TabView.cs | 2 +- UICatalog/Scenarios/Notepad.cs | 2 +- UnitTests/View/NavigationTests.cs | 43 ++-- UnitTests/Views/DatePickerTests.cs | 10 +- 9 files changed, 100 insertions(+), 201 deletions(-) rename Terminal.Gui/Application/{Application .Screen.cs => Application.Screen.cs} (100%) create mode 100644 Terminal.Gui/View/NavigationDirection.cs diff --git a/Terminal.Gui/Application/Application.Navigation.cs b/Terminal.Gui/Application/Application.Navigation.cs index 6f80320b8..4d85c2ba1 100644 --- a/Terminal.Gui/Application/Application.Navigation.cs +++ b/Terminal.Gui/Application/Application.Navigation.cs @@ -36,7 +36,7 @@ internal static class ApplicationNavigation /// /// /// - internal static void FocusNearestView (IEnumerable? viewsInTabIndexes, View.NavigationDirection direction) + internal static void FocusNearestView (IEnumerable? 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 (); diff --git a/Terminal.Gui/Application/Application .Screen.cs b/Terminal.Gui/Application/Application.Screen.cs similarity index 100% rename from Terminal.Gui/Application/Application .Screen.cs rename to Terminal.Gui/Application/Application.Screen.cs diff --git a/Terminal.Gui/View/NavigationDirection.cs b/Terminal.Gui/View/NavigationDirection.cs new file mode 100644 index 000000000..b47995f9d --- /dev/null +++ b/Terminal.Gui/View/NavigationDirection.cs @@ -0,0 +1,13 @@ +namespace Terminal.Gui; + +/// +/// Indicates navigation direction. +/// +public enum NavigationDirection +{ + /// Navigate forward. + Forward, + + /// Navigate backwards. + Backward +} diff --git a/Terminal.Gui/View/View.Navigation.cs b/Terminal.Gui/View/View.Navigation.cs index 38e0481f6..5df5576a3 100644 --- a/Terminal.Gui/View/View.Navigation.cs +++ b/Terminal.Gui/View/View.Navigation.cs @@ -7,16 +7,6 @@ public partial class View // Focus and cross-view navigation management (TabStop /// Returns a value indicating if this View is currently on Top (Active) public bool IsCurrentTop => Application.Current == this; - /// Exposed as `internal` for unit tests. Indicates focus navigation direction. - public enum NavigationDirection - { - /// Navigate forward. - Forward, - - /// Navigate backwards. - Backward - } - // BUGBUG: The focus API is poorly defined and implemented. It deeply intertwines the view hierarchy with the tab order. /// Invoked when this view is gaining focus (entering). @@ -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 /// then the focus is set to the view itself. /// /// - /// If , only subviews where has set + /// If , only subviews where has + /// set /// will be considered. /// public void FocusFirst (bool overlappedOnly = false) @@ -493,7 +484,8 @@ public partial class View // Focus and cross-view navigation management (TabStop /// then the focus is set to the view itself. /// /// - /// If , only subviews where has set + /// If , only subviews where has + /// set /// will be considered. /// public void FocusLast (bool overlappedOnly = false) @@ -522,39 +514,28 @@ public partial class View // Focus and cross-view navigation management (TabStop } /// - /// Advances the focus to the next or previous view in , based on . + /// Advances the focus to the next or previous view in , based on + /// . /// itself. /// /// /// - /// 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. /// /// /// - /// if focus was changed to another subview (or stayed on this one), otherwise. + /// + /// if focus was changed to another subview (or stayed on this one), + /// otherwise. + /// public bool AdvanceFocus (NavigationDirection direction) - { - return direction switch - { - NavigationDirection.Forward => FocusNext (), - NavigationDirection.Backward => FocusPrev (), - _ => false - }; - } - - /// - /// Focuses the next view in . If there is no next view, the focus is set to the view - /// itself. - /// - /// if focus was changed to another subview (or stayed on this one), otherwise. - 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,10 +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 (); + + break; + case NavigationDirection.Backward: + w.FocusLast (); + + break; } SetFocus (w); @@ -621,114 +620,9 @@ public partial class View // Focus and cross-view navigation management (TabStop } } - // 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; - } - - /// - /// Focuses the previous view in . If there is no previous view, the focus is set to the - /// view itself. - /// - /// if previous was focused, otherwise. - public bool FocusPrev () - { - if (!CanBeVisible (this)) - { - return false; - } - - 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 (); - } - - SetFocus (w); - - return true; - } - } - - // 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; } diff --git a/Terminal.Gui/Views/ComboBox.cs b/Terminal.Gui/Views/ComboBox.cs index e4e71c97d..094983814 100644 --- a/Terminal.Gui/Views/ComboBox.cs +++ b/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; diff --git a/Terminal.Gui/Views/TabView.cs b/Terminal.Gui/Views/TabView.cs index 04f1773c1..2ea387047 100644 --- a/Terminal.Gui/Views/TabView.cs +++ b/Terminal.Gui/Views/TabView.cs @@ -95,7 +95,7 @@ public class TabView : View Command.PreviousView, () => { - SuperView?.FocusPrev (); + SuperView?.AdvanceFocus (NavigationDirection.Backward); return true; } diff --git a/UICatalog/Scenarios/Notepad.cs b/UICatalog/Scenarios/Notepad.cs index 2dbca0117..55dde6dee 100644 --- a/UICatalog/Scenarios/Notepad.cs +++ b/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); } diff --git a/UnitTests/View/NavigationTests.cs b/UnitTests/View/NavigationTests.cs index 1933c1662..0b80b758c 100644 --- a/UnitTests/View/NavigationTests.cs +++ b/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); diff --git a/UnitTests/Views/DatePickerTests.cs b/UnitTests/Views/DatePickerTests.cs index 410077c32..b81b15b76 100644 --- a/UnitTests/Views/DatePickerTests.cs +++ b/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));