From a85617df7ca23e73d3e94c63f680845e7fca9949 Mon Sep 17 00:00:00 2001 From: BDisp Date: Tue, 21 Apr 2026 16:55:09 +0100 Subject: [PATCH 1/6] Fixes #5040. SelectorBase.TabBehavior setter calls CreateSubViews() without UpdateChecked(), desyncing CheckBox visual state --- .../Views/Selectors/OptionSelector.cs | 19 +++++++++++++++++++ Terminal.Gui/Views/Selectors/SelectorBase.cs | 6 +++--- .../Views/SelectorBaseTests.cs | 19 +++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/Terminal.Gui/Views/Selectors/OptionSelector.cs b/Terminal.Gui/Views/Selectors/OptionSelector.cs index a90c8c45d0..d8aa58e893 100644 --- a/Terminal.Gui/Views/Selectors/OptionSelector.cs +++ b/Terminal.Gui/Views/Selectors/OptionSelector.cs @@ -137,6 +137,8 @@ private void Cycle () /// public override void UpdateChecked () { + Dictionary checkBoxValueMap = SubViews.OfType ().ToDictionary (cb => cb, GetCheckBoxValue); + foreach (CheckBox cb in SubViews.OfType ()) { int value = GetCheckBoxValue (cb); @@ -144,6 +146,23 @@ public override void UpdateChecked () cb.Value = value == Value ? CheckState.Checked : CheckState.UnChecked; } + // If Value doesn't exist in any checkbox, use the first checkbox's value + if (Value is not null && checkBoxValueMap.Count > 0 && Values!.All (v => v != Value) && checkBoxValueMap.Values.All (v => v != Value)) + { + Value = checkBoxValueMap.Values.First (); + + foreach (KeyValuePair kvp in checkBoxValueMap) + { + if (kvp.Value != Value) + { + continue; + } + kvp.Key.Value = CheckState.Checked; + + break; + } + } + // Verify at most one is checked Debug.Assert (SubViews.OfType ().Count (cb => cb.Value == CheckState.Checked) <= 1); } diff --git a/Terminal.Gui/Views/Selectors/SelectorBase.cs b/Terminal.Gui/Views/Selectors/SelectorBase.cs index 49ce2e74c6..1fc8423991 100644 --- a/Terminal.Gui/Views/Selectors/SelectorBase.cs +++ b/Terminal.Gui/Views/Selectors/SelectorBase.cs @@ -156,7 +156,6 @@ public SelectorStyles Styles field = value; CreateSubViews (); - UpdateChecked (); } } @@ -316,7 +315,6 @@ public virtual IReadOnlyList? Values } CreateSubViews (); - UpdateChecked (); } } @@ -331,7 +329,6 @@ public IReadOnlyList? Labels field = value; CreateSubViews (); - UpdateChecked (); } } @@ -436,6 +433,9 @@ public virtual void CreateSubViews () // Note: Hotkey assignment is now handled automatically by the base class // when SubViews are added via Add(). No need to call AssignUniqueHotKeys() here. SetLayout (); + + // Ensure the checked state of the checkboxes is correct after recreating subviews + UpdateChecked (); } /// diff --git a/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs b/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs index 6d9c6acba0..e2afc00474 100644 --- a/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs +++ b/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs @@ -598,6 +598,25 @@ public void CreateSubViews_SetsCheckBoxProperties () Assert.True (checkBox.CanFocus); } + [Fact] + public void CreateSubViews_ResetsValueIfCurrentValueNotInNewValues_And_Call_UpdateChecked_Method () + { + OptionSelector selector = new (); + + selector.Labels = ["Option1", "Option2"]; + selector.Values = [10, 20]; + + // Verify initial value is set to first value (10) + Assert.Equal (10, selector.Value); + + // Change to new labels/values where current value (10) is not present + selector.Labels = ["New1", "New2"]; + selector.Values = [30, 40]; + + // Value should reset to first value (30) + Assert.Equal (30, selector.Value); + } + #endregion #region HotKey Command Tests From a5dbfcb66093f6682073a78f8d25184295732429 Mon Sep 17 00:00:00 2001 From: BDisp Date: Wed, 22 Apr 2026 00:27:00 +0100 Subject: [PATCH 2/6] Add unit test that test the original issue post --- .../Views/SelectorBaseTests.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs b/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs index e2afc00474..5e5147add6 100644 --- a/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs +++ b/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs @@ -617,6 +617,23 @@ public void CreateSubViews_ResetsValueIfCurrentValueNotInNewValues_And_Call_Upda Assert.Equal (30, selector.Value); } + [Fact] + public void Setting_TabBehavior_AfterSubViewsAreCreated_DoesNotLoseCheckedState () + { + OptionSelector selector = new () + { + Labels = ["Option1", "Option2"], Values = [10, 20], Orientation = Orientation.Vertical, TabBehavior = TabBehavior.NoStop + }; + + List checkBoxes = [.. selector.SubViews.OfType ()]; + + Assert.Equal (2, checkBoxes.Count); + Assert.Equal (CheckState.Checked, checkBoxes [0].Value); + Assert.Equal (CheckState.UnChecked, checkBoxes [1].Value); + Assert.Equal (10, selector.Value); + Assert.Equal (TabBehavior.NoStop, selector.TabBehavior); + } + #endregion #region HotKey Command Tests From 277b4c466767f87f935fd0606ed1ab26bab71bfe Mon Sep 17 00:00:00 2001 From: BDisp Date: Wed, 22 Apr 2026 01:07:56 +0100 Subject: [PATCH 3/6] Add unit test that test the original issue post with enum --- .../Views/SelectorBaseTests.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs b/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs index 5e5147add6..730fdf3566 100644 --- a/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs +++ b/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs @@ -634,6 +634,25 @@ public void Setting_TabBehavior_AfterSubViewsAreCreated_DoesNotLoseCheckedState Assert.Equal (TabBehavior.NoStop, selector.TabBehavior); } + [Fact] + public void Setting_TabBehavior_AfterSubViewsAreCreated_DoesNotLoseCheckedState_With_Enum () + { + OptionSelector selector = new () + { + Orientation = Orientation.Vertical, TabBehavior = TabBehavior.NoStop + }; + + List checkBoxes = [.. selector.SubViews.OfType ()]; + + Assert.Equal (4, checkBoxes.Count); + Assert.Equal (CheckState.Checked, checkBoxes [0].Value); + Assert.Equal (CheckState.UnChecked, checkBoxes [1].Value); + Assert.Equal (CheckState.UnChecked, checkBoxes [2].Value); + Assert.Equal (CheckState.UnChecked, checkBoxes [3].Value); + Assert.Equal (Side.Left, selector.Value); + Assert.Equal (TabBehavior.NoStop, selector.TabBehavior); + } + #endregion #region HotKey Command Tests From d065cf438950e6b26eb53cc337b7357fcaa13c99 Mon Sep 17 00:00:00 2001 From: BDisp Date: Wed, 22 Apr 2026 13:23:29 +0100 Subject: [PATCH 4/6] Update XML doc to the fact that Value can be changed in the UpdateChecked method --- Terminal.Gui/Views/Selectors/OptionSelector.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Terminal.Gui/Views/Selectors/OptionSelector.cs b/Terminal.Gui/Views/Selectors/OptionSelector.cs index d8aa58e893..6dc7da0530 100644 --- a/Terminal.Gui/Views/Selectors/OptionSelector.cs +++ b/Terminal.Gui/Views/Selectors/OptionSelector.cs @@ -135,6 +135,10 @@ private void Cycle () /// Updates the checked state of all checkbox subviews so that only the checkbox corresponding /// to the current is checked. /// + /// + /// If doesn't exist in the list of checkbox values, then the first checkbox will be checked by default + /// and will raise the / events. + /// public override void UpdateChecked () { Dictionary checkBoxValueMap = SubViews.OfType ().ToDictionary (cb => cb, GetCheckBoxValue); From 8d54d85d88f0dec64dad9092b03a8856315262cb Mon Sep 17 00:00:00 2001 From: BDisp Date: Wed, 22 Apr 2026 14:32:59 +0100 Subject: [PATCH 5/6] Add assertions to enforce correct behavior --- Terminal.Gui/Views/Selectors/OptionSelector.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Terminal.Gui/Views/Selectors/OptionSelector.cs b/Terminal.Gui/Views/Selectors/OptionSelector.cs index 6dc7da0530..45edda196f 100644 --- a/Terminal.Gui/Views/Selectors/OptionSelector.cs +++ b/Terminal.Gui/Views/Selectors/OptionSelector.cs @@ -165,6 +165,10 @@ public override void UpdateChecked () break; } + + // Sanity checks to verify the assumptions above + Debug.Assert (checkBoxValueMap.Values.First () != (int)SubViews.OfType ().First ().Value); + Debug.Assert (checkBoxValueMap.Values.First () == Values! [0]); } // Verify at most one is checked From a7f0e0f666479f81b8b6f8a37003d8f9386cab38 Mon Sep 17 00:00:00 2001 From: BDisp Date: Wed, 22 Apr 2026 14:35:47 +0100 Subject: [PATCH 6/6] Rename unit test method name as suggested --- Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs b/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs index 730fdf3566..611032e1f1 100644 --- a/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs +++ b/Tests/UnitTestsParallelizable/Views/SelectorBaseTests.cs @@ -599,7 +599,7 @@ public void CreateSubViews_SetsCheckBoxProperties () } [Fact] - public void CreateSubViews_ResetsValueIfCurrentValueNotInNewValues_And_Call_UpdateChecked_Method () + public void CreateSubViews_ResetsValue_WhenCurrentValueNotInNewValues () { OptionSelector selector = new ();