From 27234b7bf9240c59043b576babab6c68db19349c Mon Sep 17 00:00:00 2001 From: Tig Date: Sat, 31 Aug 2024 17:17:55 -0400 Subject: [PATCH] Fixes #3707 - Wizard focus (#3708) * Initial commit. * More fixes * Fixed AdvanceFocus issue with nested subviews. E.g. Wizard->WizardStep->_contentView->subview * Fixed DatePicker tests * Fixed DatePicker tests --- Terminal.Gui/View/View.Navigation.cs | 62 +++++---- Terminal.Gui/Views/DatePicker.cs | 4 + Terminal.Gui/Views/Wizard/Wizard.cs | 13 +- Terminal.Gui/Views/Wizard/WizardStep.cs | 28 ++-- UICatalog/Scenarios/Wizards.cs | 40 ++++-- .../View/Navigation/AdvanceFocusTests.cs | 131 +++++++++++++++++- UnitTests/Views/DatePickerTests.cs | 26 +++- 7 files changed, 238 insertions(+), 66 deletions(-) diff --git a/Terminal.Gui/View/View.Navigation.cs b/Terminal.Gui/View/View.Navigation.cs index ed8e2b844..65ae2a7bf 100644 --- a/Terminal.Gui/View/View.Navigation.cs +++ b/Terminal.Gui/View/View.Navigation.cs @@ -14,7 +14,7 @@ public partial class View // Focus and cross-view navigation management (TabStop /// /// /// - /// If there is no next/previous view, the focus is set to the view itself. + /// If there is no next/previous view to advance to, the focus is set to the view itself. /// /// /// @@ -37,17 +37,19 @@ public partial class View // Focus and cross-view navigation management (TabStop return true; } - // AdvanceFocus did not advance - View [] index = GetSubviewFocusChain (direction, behavior); + // AdvanceFocus did not advance - do we wrap, or move up to the superview? - if (index.Length == 0) + View [] focusChain = GetSubviewFocusChain (direction, behavior); + + if (focusChain.Length == 0) { return false; } + // Special case TabGroup if (behavior == TabBehavior.TabGroup) { - if (direction == NavigationDirection.Forward && focused == index [^1] && SuperView is null) + if (direction == NavigationDirection.Forward && focused == focusChain [^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); @@ -66,7 +68,7 @@ public partial class View // Focus and cross-view navigation management (TabStop } } - if (direction == NavigationDirection.Backward && focused == index [0]) + if (direction == NavigationDirection.Backward && focused == focusChain [0]) { // We're at the bottom of the focus chain View [] views = GetSubviewFocusChain (NavigationDirection.Forward, TabBehavior.TabGroup); @@ -86,38 +88,38 @@ public partial class View // Focus and cross-view navigation management (TabStop } } - int focusedIndex = index.IndexOf (Focused); // Will return -1 if Focused can't be found or is null - int next = 0; + int focusedIndex = focusChain.IndexOf (Focused); // Will return -1 if Focused can't be found or is null + var next = 0; // Assume we wrap to start of the focus chain - if (focusedIndex < index.Length - 1) + if (focusedIndex < focusChain.Length - 1) { // We're moving w/in the subviews next = focusedIndex + 1; } else { - // We're moving beyond the last subview - // Determine if focus should remain in this focus chain, or move to the superview's focus 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) + if (SuperView is { }) { - return false; - } - - // TabGroup is special-cased. - if (focused?.TabStop == TabBehavior.TabGroup) - { - if (SuperView?.GetSubviewFocusChain (direction, TabBehavior.TabGroup)?.Length > 0) + // 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.GetSubviewFocusChain (direction, behavior).Length > 1) { - // Our superview has a TabGroup subview; signal we couldn't move so we nav out to it return false; } + + // TabGroup is special-cased. + if (focused?.TabStop == TabBehavior.TabGroup) + { + if (SuperView?.GetSubviewFocusChain (direction, TabBehavior.TabGroup)?.Length > 0) + { + // Our superview has a TabGroup subview; signal we couldn't move so we nav out to it + return false; + } + } } } - View view = index [next]; + View view = focusChain [next]; if (view.HasFocus) { @@ -259,7 +261,7 @@ public partial class View // Focus and cross-view navigation management (TabStop { if (Focused is null && _subviews?.Count > 0) { - if (_previouslyMostFocused is { } /* && (behavior is null || _previouslyMostFocused.TabStop == behavior)*/) + if (_previouslyMostFocused is { }) { return _previouslyMostFocused.SetFocus (); } @@ -355,6 +357,11 @@ public partial class View // Focus and cross-view navigation management (TabStop return focusSet; } + /// + /// Caches the most focused subview when this view is losing focus. This is used by . + /// + private View? _previouslyMostFocused; + /// /// INTERNAL: Called when focus is going to change to this view. This method is called by and /// other methods that @@ -625,7 +632,7 @@ public partial class View // Focus and cross-view navigation management (TabStop if (SuperView is { }) { - SuperView._previouslyMostFocused = focusedPeer; + //SuperView._previouslyMostFocused = focusedPeer; } // Post-conditions - prove correctness @@ -637,11 +644,6 @@ public partial class View // Focus and cross-view navigation management (TabStop SetNeedsDisplay (); } - /// - /// Caches the most focused subview when this view is losing focus. This is used by . - /// - private View? _previouslyMostFocused; - private void NotifyFocusChanged (bool newHasFocus, View? previousFocusedView, View? focusedVew) { // Call the virtual method diff --git a/Terminal.Gui/Views/DatePicker.cs b/Terminal.Gui/Views/DatePicker.cs index 1676f6df2..c4ac8951e 100644 --- a/Terminal.Gui/Views/DatePicker.cs +++ b/Terminal.Gui/Views/DatePicker.cs @@ -192,6 +192,7 @@ public class DatePicker : View _calendar = new TableView { + Id = "_calendar", X = 0, Y = Pos.Bottom (_dateLabel), Height = 11, @@ -206,6 +207,7 @@ public class DatePicker : View _dateField = new DateField (DateTime.Now) { + Id = "_dateField", X = Pos.Right (_dateLabel), Y = 0, Width = Dim.Width (_calendar) - Dim.Width (_dateLabel), @@ -215,6 +217,7 @@ public class DatePicker : View _previousMonthButton = new Button { + Id = "_previousMonthButton", X = Pos.Center () - 2, Y = Pos.Bottom (_calendar) - 1, Width = 2, @@ -234,6 +237,7 @@ public class DatePicker : View _nextMonthButton = new Button { + Id = "_nextMonthButton", X = Pos.Right (_previousMonthButton) + 2, Y = Pos.Bottom (_calendar) - 1, Width = 2, diff --git a/Terminal.Gui/Views/Wizard/Wizard.cs b/Terminal.Gui/Views/Wizard/Wizard.cs index 70aeeeb5f..262558d12 100644 --- a/Terminal.Gui/Views/Wizard/Wizard.cs +++ b/Terminal.Gui/Views/Wizard/Wizard.cs @@ -351,14 +351,11 @@ public class Wizard : Dialog UpdateButtonsAndTitle (); - // Set focus to the nav buttons - if (BackButton.HasFocus) + + // Set focus on the contentview + if (newStep is { }) { - BackButton.SetFocus (); - } - else - { - NextFinishButton.SetFocus (); + newStep.Subviews.ToArray () [0].SetFocus (); } if (OnStepChanged (oldStep, _currentStep)) @@ -543,7 +540,7 @@ public class Wizard : Dialog SetNeedsLayout (); LayoutSubviews (); - Draw (); + //Draw (); } private void Wizard_Closing (object sender, ToplevelClosingEventArgs obj) diff --git a/Terminal.Gui/Views/Wizard/WizardStep.cs b/Terminal.Gui/Views/Wizard/WizardStep.cs index f6ebac473..36f88e5d3 100644 --- a/Terminal.Gui/Views/Wizard/WizardStep.cs +++ b/Terminal.Gui/Views/Wizard/WizardStep.cs @@ -13,7 +13,7 @@ /// the step is active; see also: . To enable or disable a step from being shown to the /// user, set . /// -public class WizardStep : FrameView +public class WizardStep : View { ///// ///// The title of the . @@ -36,19 +36,32 @@ public class WizardStep : FrameView //private string title = string.Empty; // The contentView works like the ContentView in FrameView. - private readonly View _contentView = new () { Id = "WizardContentView" }; - private readonly TextView _helpTextView = new (); + private readonly View _contentView = new () + { + CanFocus = true, + TabStop = TabBehavior.TabStop, + Id = "WizardStep._contentView" + }; + private readonly TextView _helpTextView = new () + { + CanFocus = true, + TabStop = TabBehavior.TabStop, + ReadOnly = true, + WordWrap = true, + AllowsTab = false, + Id = "WizardStep._helpTextView" + }; /// /// Initializes a new instance of the class. /// public WizardStep () { + TabStop = TabBehavior.TabStop; + CanFocus = true; BorderStyle = LineStyle.None; base.Add (_contentView); - _helpTextView.ReadOnly = true; - _helpTextView.WordWrap = true; base.Add (_helpTextView); // BUGBUG: v2 - Disabling scrolling for now @@ -144,11 +157,6 @@ public class WizardStep : FrameView /// public override View Remove (View view) { - if (view is null) - { - return view; - } - SetNeedsDisplay (); View container = view?.SuperView; diff --git a/UICatalog/Scenarios/Wizards.cs b/UICatalog/Scenarios/Wizards.cs index c909ccd40..f3c03c82c 100644 --- a/UICatalog/Scenarios/Wizards.cs +++ b/UICatalog/Scenarios/Wizards.cs @@ -26,7 +26,7 @@ public class Wizards : Scenario }; win.Add (frame); - var label = new Label { X = 0, Y = 0, TextAlignment = Alignment.End, Text = "Width:" }; + var label = new Label { X = 0, Y = 0, TextAlignment = Alignment.End, Text = "_Width:", Width = 10 }; frame.Add (label); var widthEdit = new TextField @@ -39,7 +39,7 @@ public class Wizards : Scenario }; frame.Add (widthEdit); - label = new() + label = new () { X = 0, Y = Pos.Bottom (label), @@ -47,7 +47,7 @@ public class Wizards : Scenario Width = Dim.Width (label), Height = 1, TextAlignment = Alignment.End, - Text = "Height:" + Text = "_Height:" }; frame.Add (label); @@ -61,7 +61,7 @@ public class Wizards : Scenario }; frame.Add (heightEdit); - label = new() + label = new () { X = 0, Y = Pos.Bottom (label), @@ -69,7 +69,7 @@ public class Wizards : Scenario Width = Dim.Width (label), Height = 1, TextAlignment = Alignment.End, - Text = "Title:" + Text = "_Title:" }; frame.Add (label); @@ -91,7 +91,7 @@ public class Wizards : Scenario win.Loaded += Win_Loaded; - label = new() + label = new () { X = Pos.Center (), Y = Pos.AnchorEnd (1), TextAlignment = Alignment.End, Text = "Action:" }; @@ -105,7 +105,7 @@ public class Wizards : Scenario var showWizardButton = new Button { - X = Pos.Center (), Y = Pos.Bottom (frame) + 2, IsDefault = true, Text = "Show Wizard" + X = Pos.Center (), Y = Pos.Bottom (frame) + 2, IsDefault = true, Text = "_Show Wizard" }; showWizardButton.Accept += (s, e) => @@ -162,6 +162,13 @@ public class Wizards : Scenario firstStep.HelpText = "This is the End User License Agreement.\n\n\n\n\n\nThis is a test of the emergency broadcast system. This is a test of the emergency broadcast system.\nThis is a test of the emergency broadcast system.\n\n\nThis is a test of the emergency broadcast system.\n\nThis is a test of the emergency broadcast system.\n\n\n\nThe end of the EULA."; + + RadioGroup radioGroup = new () + { + RadioLabels = ["_One", "_Two", "_3"] + }; + firstStep.Add (radioGroup); + wizard.AddStep (firstStep); // Add 2nd step @@ -178,6 +185,13 @@ public class Wizards : Scenario Text = "Press Me to Rename Step", X = Pos.Right (buttonLbl), Y = Pos.Top (buttonLbl) }; + RadioGroup radioGroup2 = new () + { + RadioLabels = ["_A", "_B", "_C"], + Orientation = Orientation.Horizontal + }; + secondStep.Add (radioGroup2); + button.Accept += (s, e) => { secondStep.Title = "2nd Step"; @@ -193,7 +207,7 @@ public class Wizards : Scenario var firstNameField = new TextField { Text = "Number", Width = 30, X = Pos.Right (lbl), Y = Pos.Top (lbl) }; secondStep.Add (lbl, firstNameField); - lbl = new() { Text = "Last Name: ", X = 1, Y = Pos.Bottom (lbl) }; + lbl = new () { Text = "Last Name: ", X = 1, Y = Pos.Bottom (lbl) }; var lastNameField = new TextField { Text = "Six", Width = 30, X = Pos.Right (lbl), Y = Pos.Top (lbl) }; secondStep.Add (lbl, lastNameField); @@ -213,7 +227,8 @@ public class Wizards : Scenario Y = Pos.Bottom (thirdStepEnabledCeckBox) + 2, Width = Dim.Fill (), Height = 4, - Title = "A Broken Frame (by Depeche Mode)" + Title = "A Broken Frame (by Depeche Mode)", + TabStop = TabBehavior.NoStop }; frame.Add (new TextField { Text = "This is a TextField inside of the frame." }); secondStep.Add (frame); @@ -286,7 +301,7 @@ public class Wizards : Scenario } }; fourthStep.Add (hideHelpBtn); - fourthStep.NextButtonText = "Go To Last Step"; + fourthStep.NextButtonText = "_Go To Last Step"; var scrollBar = new ScrollBarView (someText, true); scrollBar.ChangedPosition += (s, e) => @@ -355,4 +370,9 @@ public class Wizards : Scenario win.Dispose (); Application.Shutdown (); } + + private void Wizard_StepChanged (object sender, StepChangeEventArgs e) + { + throw new NotImplementedException (); + } } diff --git a/UnitTests/View/Navigation/AdvanceFocusTests.cs b/UnitTests/View/Navigation/AdvanceFocusTests.cs index 63e7843cc..efa89d721 100644 --- a/UnitTests/View/Navigation/AdvanceFocusTests.cs +++ b/UnitTests/View/Navigation/AdvanceFocusTests.cs @@ -1,4 +1,5 @@ -using Xunit.Abstractions; +using System.Runtime.Intrinsics; +using Xunit.Abstractions; namespace Terminal.Gui.ViewTests; @@ -72,6 +73,38 @@ public class AdvanceFocusTests () r.Dispose (); } + + [Fact] + public void AdvanceFocus_Subviews_TabStop () + { + TabBehavior behavior = TabBehavior.TabStop; + var top = new View { Id = "top", CanFocus = true }; + + 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 }; + + top.Add (v1, v2, v3); + + // Cycle through v1 & v2 + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (v1.HasFocus); + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (v2.HasFocus); + + // Should cycle back to v1 + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (v1.HasFocus); + + // Go backwards + top.AdvanceFocus (NavigationDirection.Backward, behavior); + Assert.True (v2.HasFocus); + top.AdvanceFocus (NavigationDirection.Backward, behavior); + Assert.True (v1.HasFocus); + + top.Dispose (); + } + [Fact] public void AdvanceFocus_Compound_Subview_TabStop () { @@ -145,6 +178,102 @@ public class AdvanceFocusTests () top.Dispose (); } + + [Fact] + public void AdvanceFocus_CompoundCompound_Subview_TabStop () + { + TabBehavior behavior = TabBehavior.TabStop; + var top = new View { Id = "top", CanFocus = true }; + var topv1 = new View { Id = "topv1", CanFocus = true, TabStop = behavior }; + var topv2 = new View { Id = "topv2", CanFocus = true, TabStop = behavior }; + var topv3 = new View { Id = "topv3", CanFocus = false, TabStop = behavior }; + top.Add (topv1, topv2, topv3); + + var compoundSubview = new View + { + CanFocus = true, + Id = "compoundSubview", + TabStop = behavior + }; + 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); + + + var compoundCompoundSubview = new View + { + CanFocus = true, + Id = "compoundCompoundSubview", + TabStop = behavior + }; + var v4 = new View { Id = "v4", CanFocus = true, TabStop = behavior }; + var v5 = new View { Id = "v5", CanFocus = true, TabStop = behavior }; + var v6 = new View { Id = "v6", CanFocus = false, TabStop = behavior }; + + compoundCompoundSubview.Add (v4, v5, v6); + + compoundSubview.Add (compoundCompoundSubview); + + top.Add (compoundSubview); + + top.SetFocus (); + Assert.True (topv1.HasFocus); + + // Cycle through topv2 + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (topv2.HasFocus); + + // Cycle v1, v2, v4, v5 + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (v1.HasFocus); + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (v2.HasFocus); + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (v4.HasFocus); + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (v5.HasFocus); + + // Should cycle back to topv1 + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (topv1.HasFocus); + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (topv2.HasFocus); + + // Add another top subview. Should cycle to it after v5 + View otherSubview = new () + { + CanFocus = true, + TabStop = behavior, + Id = "otherSubview" + }; + + top.Add (otherSubview); + + // Adding a focusable subview causes advancefocus + Assert.True (otherSubview.HasFocus); + + // Cycle through topv1, topv2, v1, v2, v4, v5 + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (topv1.HasFocus); + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (topv2.HasFocus); + top.AdvanceFocus (NavigationDirection.Forward, behavior); + + // the above should have cycled to v5 since it was the previously most focused subview of compoundSubView + Assert.True (v5.HasFocus); + + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (otherSubview.HasFocus); + + // Should cycle back to topv1 + top.AdvanceFocus (NavigationDirection.Forward, behavior); + Assert.True (topv1.HasFocus); + + top.Dispose (); + } + [Fact] public void AdvanceFocus_Compound_Subview_TabGroup () { diff --git a/UnitTests/Views/DatePickerTests.cs b/UnitTests/Views/DatePickerTests.cs index 7fa24aae3..956df1649 100644 --- a/UnitTests/Views/DatePickerTests.cs +++ b/UnitTests/Views/DatePickerTests.cs @@ -71,18 +71,25 @@ public class DatePickerTests top.Add (datePicker); Application.Begin (top); + Assert.Equal (datePicker.Subviews.First (v => v.Id == "_dateField"), datePicker.Focused); + // Set focus to next month button datePicker.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + Assert.Equal (datePicker.Subviews.First (v => v.Id == "_calendar"), datePicker.Focused); datePicker.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + Assert.Equal (datePicker.Subviews.First (v => v.Id == "_previousMonthButton"), datePicker.Focused); datePicker.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + Assert.Equal (datePicker.Subviews.First (v => v.Id == "_nextMonthButton"), datePicker.Focused); // Change month to December Assert.True (Application.OnKeyDown (Key.Enter)); Assert.Equal (12, datePicker.Date.Month); - // Date should change as next month button was disabled, causing focus to advance - Assert.True (Application.OnKeyDown (Key.Enter)); - Assert.Equal (11, datePicker.Date.Month); + // Next month button is disabled, so focus advanced to edit field + Assert.Equal (datePicker.Subviews.First (v => v.Id == "_dateField"), datePicker.Focused); + + // Pressing enter on datefield does nothing + Assert.False (Application.OnKeyDown (Key.Enter)); top.Dispose (); } @@ -98,17 +105,22 @@ public class DatePickerTests top.Add (datePicker); Application.Begin (top); - // set focus to the previous month button + Assert.Equal (datePicker.Subviews.First (v => v.Id == "_dateField"), datePicker.Focused); + datePicker.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + Assert.Equal (datePicker.Subviews.First (v => v.Id == "_calendar"), datePicker.Focused); datePicker.AdvanceFocus (NavigationDirection.Forward, TabBehavior.TabStop); + Assert.Equal (datePicker.Subviews.First (v => v.Id == "_previousMonthButton"), datePicker.Focused); // Change month to January Assert.True (datePicker.NewKeyDownEvent (Key.Enter)); Assert.Equal (1, datePicker.Date.Month); - // Previous month button is disabled, so focus advanced to edit field - Assert.True (datePicker.NewKeyDownEvent (Key.Enter)); - Assert.Equal (1, datePicker.Date.Month); + // Next prev button is disabled, so focus advanced to edit button + Assert.Equal (datePicker.Subviews.First (v => v.Id == "_dateField"), datePicker.Focused); + + // Pressing enter on datefield does nothing + Assert.False (Application.OnKeyDown (Key.Enter)); top.Dispose (); } }