From ae5e41fd6bd954e13668ef4c7d2c08d03fa2d4c7 Mon Sep 17 00:00:00 2001 From: BDisp Date: Wed, 27 Mar 2024 00:17:31 +0000 Subject: [PATCH 1/4] Fixes #3351. TabIndex with the same setter value but with wrong index return without set the correct value. --- Terminal.Gui/View/ViewKeyboard.cs | 2 +- UnitTests/View/NavigationTests.cs | 47 +++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/Terminal.Gui/View/ViewKeyboard.cs b/Terminal.Gui/View/ViewKeyboard.cs index 409f7de64..52aa07da9 100644 --- a/Terminal.Gui/View/ViewKeyboard.cs +++ b/Terminal.Gui/View/ViewKeyboard.cs @@ -267,7 +267,7 @@ public partial class View return; } - if (_tabIndex == value) + if (_tabIndex == value && TabIndexes.IndexOf (this) == value) { return; } diff --git a/UnitTests/View/NavigationTests.cs b/UnitTests/View/NavigationTests.cs index 3d1e6611c..653c447ef 100644 --- a/UnitTests/View/NavigationTests.cs +++ b/UnitTests/View/NavigationTests.cs @@ -1290,6 +1290,53 @@ public class NavigationTests r.Dispose (); } + [Fact] + public void TabIndex_Invert_Order () + { + var r = new View (); + var v1 = new View () { Id = "1", CanFocus = true }; + var v2 = new View () { Id = "2", CanFocus = true }; + var v3 = new View () { Id = "3", CanFocus = true }; + + r.Add (v1, v2, v3); + + v1.TabIndex = 2; + v2.TabIndex = 1; + v3.TabIndex = 0; + Assert.True (r.TabIndexes.IndexOf (v1) == 2); + Assert.True (r.TabIndexes.IndexOf (v2) == 1); + Assert.True (r.TabIndexes.IndexOf (v3) == 0); + + Assert.True (r.Subviews.IndexOf (v1) == 0); + Assert.True (r.Subviews.IndexOf (v2) == 1); + Assert.True (r.Subviews.IndexOf (v3) == 2); + } + + [Fact] + public void TabIndex_Invert_Order_Mixed () + { + var r = new View (); + var vl1 = new View () { Id = "vl1" }; + var v1 = new View () { Id = "v1", CanFocus = true }; + var vl2 = new View () { Id = "vl2" }; + var v2 = new View () { Id = "v2", CanFocus = true }; + var vl3 = new View () { Id = "vl3" }; + var v3 = new View () { Id = "v3", CanFocus = true }; + + r.Add (vl1, v1, vl2, v2, vl3, v3); + + v1.TabIndex = 2; + v2.TabIndex = 1; + v3.TabIndex = 0; + Assert.True (r.TabIndexes.IndexOf (v1) == 4); + Assert.True (r.TabIndexes.IndexOf (v2) == 2); + Assert.True (r.TabIndexes.IndexOf (v3) == 0); + + Assert.True (r.Subviews.IndexOf (v1) == 1); + Assert.True (r.Subviews.IndexOf (v2) == 3); + Assert.True (r.Subviews.IndexOf (v3) == 5); + } + [Fact] public void TabStop_All_False_And_All_True_And_Changing_TabStop_Later () { From b9c0bcb613f0ffed3061341c157472dde92fd775 Mon Sep 17 00:00:00 2001 From: BDisp Date: Wed, 27 Mar 2024 15:15:59 +0000 Subject: [PATCH 2/4] Add one more unit test and replace True/False to Equal/NotEqual. --- UnitTests/View/NavigationTests.cs | 50 +++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/UnitTests/View/NavigationTests.cs b/UnitTests/View/NavigationTests.cs index 653c447ef..399d3a6c5 100644 --- a/UnitTests/View/NavigationTests.cs +++ b/UnitTests/View/NavigationTests.cs @@ -1303,13 +1303,39 @@ public class NavigationTests v1.TabIndex = 2; v2.TabIndex = 1; v3.TabIndex = 0; - Assert.True (r.TabIndexes.IndexOf (v1) == 2); - Assert.True (r.TabIndexes.IndexOf (v2) == 1); - Assert.True (r.TabIndexes.IndexOf (v3) == 0); + Assert.Equal (2, r.TabIndexes.IndexOf (v1)); + Assert.Equal (1, r.TabIndexes.IndexOf (v2)); + Assert.Equal (0, r.TabIndexes.IndexOf (v3)); - Assert.True (r.Subviews.IndexOf (v1) == 0); - Assert.True (r.Subviews.IndexOf (v2) == 1); - Assert.True (r.Subviews.IndexOf (v3) == 2); + Assert.Equal (0, r.Subviews.IndexOf (v1)); + Assert.Equal (1, r.Subviews.IndexOf (v2)); + Assert.Equal (2, r.Subviews.IndexOf (v3)); + } + + [Fact] + public void TabIndex_Invert_Order_Added_One_By_One_Does_Not_Do_What_Is_Expected () + { + var r = new View (); + var v1 = new View () { Id = "1", CanFocus = true }; + r.Add (v1); + v1.TabIndex = 2; + var v2 = new View () { Id = "2", CanFocus = true }; + r.Add (v2); + v2.TabIndex = 1; + var v3 = new View () { Id = "3", CanFocus = true }; + r.Add (v3); + v3.TabIndex = 0; + + Assert.NotEqual (2, r.TabIndexes.IndexOf (v1)); + Assert.Equal (1, r.TabIndexes.IndexOf (v1)); + Assert.NotEqual (1, r.TabIndexes.IndexOf (v2)); + Assert.Equal (2, r.TabIndexes.IndexOf (v2)); + // Only the last is in the expected index + Assert.Equal (0, r.TabIndexes.IndexOf (v3)); + + Assert.Equal (0, r.Subviews.IndexOf (v1)); + Assert.Equal (1, r.Subviews.IndexOf (v2)); + Assert.Equal (2, r.Subviews.IndexOf (v3)); } [Fact] @@ -1328,13 +1354,13 @@ public class NavigationTests v1.TabIndex = 2; v2.TabIndex = 1; v3.TabIndex = 0; - Assert.True (r.TabIndexes.IndexOf (v1) == 4); - Assert.True (r.TabIndexes.IndexOf (v2) == 2); - Assert.True (r.TabIndexes.IndexOf (v3) == 0); + Assert.Equal (4, r.TabIndexes.IndexOf (v1)); + Assert.Equal (2, r.TabIndexes.IndexOf (v2)); + Assert.Equal (0, r.TabIndexes.IndexOf (v3)); - Assert.True (r.Subviews.IndexOf (v1) == 1); - Assert.True (r.Subviews.IndexOf (v2) == 3); - Assert.True (r.Subviews.IndexOf (v3) == 5); + Assert.Equal (1, r.Subviews.IndexOf (v1)); + Assert.Equal (3, r.Subviews.IndexOf (v2)); + Assert.Equal (5, r.Subviews.IndexOf (v3)); } [Fact] From f272f8941f51aac49652f6c588fd224cfe265b73 Mon Sep 17 00:00:00 2001 From: BDisp Date: Wed, 27 Mar 2024 15:41:35 +0000 Subject: [PATCH 3/4] Revert "Add one more unit test and replace True/False to Equal/NotEqual." This reverts commit b9c0bcb613f0ffed3061341c157472dde92fd775. --- UnitTests/View/NavigationTests.cs | 50 ++++++++----------------------- 1 file changed, 12 insertions(+), 38 deletions(-) diff --git a/UnitTests/View/NavigationTests.cs b/UnitTests/View/NavigationTests.cs index 399d3a6c5..653c447ef 100644 --- a/UnitTests/View/NavigationTests.cs +++ b/UnitTests/View/NavigationTests.cs @@ -1303,39 +1303,13 @@ public class NavigationTests v1.TabIndex = 2; v2.TabIndex = 1; v3.TabIndex = 0; - Assert.Equal (2, r.TabIndexes.IndexOf (v1)); - Assert.Equal (1, r.TabIndexes.IndexOf (v2)); - Assert.Equal (0, r.TabIndexes.IndexOf (v3)); + Assert.True (r.TabIndexes.IndexOf (v1) == 2); + Assert.True (r.TabIndexes.IndexOf (v2) == 1); + Assert.True (r.TabIndexes.IndexOf (v3) == 0); - Assert.Equal (0, r.Subviews.IndexOf (v1)); - Assert.Equal (1, r.Subviews.IndexOf (v2)); - Assert.Equal (2, r.Subviews.IndexOf (v3)); - } - - [Fact] - public void TabIndex_Invert_Order_Added_One_By_One_Does_Not_Do_What_Is_Expected () - { - var r = new View (); - var v1 = new View () { Id = "1", CanFocus = true }; - r.Add (v1); - v1.TabIndex = 2; - var v2 = new View () { Id = "2", CanFocus = true }; - r.Add (v2); - v2.TabIndex = 1; - var v3 = new View () { Id = "3", CanFocus = true }; - r.Add (v3); - v3.TabIndex = 0; - - Assert.NotEqual (2, r.TabIndexes.IndexOf (v1)); - Assert.Equal (1, r.TabIndexes.IndexOf (v1)); - Assert.NotEqual (1, r.TabIndexes.IndexOf (v2)); - Assert.Equal (2, r.TabIndexes.IndexOf (v2)); - // Only the last is in the expected index - Assert.Equal (0, r.TabIndexes.IndexOf (v3)); - - Assert.Equal (0, r.Subviews.IndexOf (v1)); - Assert.Equal (1, r.Subviews.IndexOf (v2)); - Assert.Equal (2, r.Subviews.IndexOf (v3)); + Assert.True (r.Subviews.IndexOf (v1) == 0); + Assert.True (r.Subviews.IndexOf (v2) == 1); + Assert.True (r.Subviews.IndexOf (v3) == 2); } [Fact] @@ -1354,13 +1328,13 @@ public class NavigationTests v1.TabIndex = 2; v2.TabIndex = 1; v3.TabIndex = 0; - Assert.Equal (4, r.TabIndexes.IndexOf (v1)); - Assert.Equal (2, r.TabIndexes.IndexOf (v2)); - Assert.Equal (0, r.TabIndexes.IndexOf (v3)); + Assert.True (r.TabIndexes.IndexOf (v1) == 4); + Assert.True (r.TabIndexes.IndexOf (v2) == 2); + Assert.True (r.TabIndexes.IndexOf (v3) == 0); - Assert.Equal (1, r.Subviews.IndexOf (v1)); - Assert.Equal (3, r.Subviews.IndexOf (v2)); - Assert.Equal (5, r.Subviews.IndexOf (v3)); + Assert.True (r.Subviews.IndexOf (v1) == 1); + Assert.True (r.Subviews.IndexOf (v2) == 3); + Assert.True (r.Subviews.IndexOf (v3) == 5); } [Fact] From d7fec3c3f13b6d378fe3bd0d6dc571eda9e4fbca Mon Sep 17 00:00:00 2001 From: BDisp Date: Wed, 27 Mar 2024 15:43:55 +0000 Subject: [PATCH 4/4] Add unit test that proof setting TabIndex before all views are added, will have unexpected result. --- UnitTests/View/NavigationTests.cs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/UnitTests/View/NavigationTests.cs b/UnitTests/View/NavigationTests.cs index 653c447ef..773ab0f77 100644 --- a/UnitTests/View/NavigationTests.cs +++ b/UnitTests/View/NavigationTests.cs @@ -1312,6 +1312,32 @@ public class NavigationTests Assert.True (r.Subviews.IndexOf (v3) == 2); } + [Fact] + public void TabIndex_Invert_Order_Added_One_By_One_Does_Not_Do_What_Is_Expected () + { + var r = new View (); + var v1 = new View () { Id = "1", CanFocus = true }; + r.Add (v1); + v1.TabIndex = 2; + var v2 = new View () { Id = "2", CanFocus = true }; + r.Add (v2); + v2.TabIndex = 1; + var v3 = new View () { Id = "3", CanFocus = true }; + r.Add (v3); + v3.TabIndex = 0; + + Assert.False (r.TabIndexes.IndexOf (v1) == 2); + Assert.True (r.TabIndexes.IndexOf (v1) == 1); + Assert.False (r.TabIndexes.IndexOf (v2) == 1); + Assert.True (r.TabIndexes.IndexOf (v2) == 2); + // Only the last is in the expected index + Assert.True (r.TabIndexes.IndexOf (v3) == 0); + + Assert.True (r.Subviews.IndexOf (v1) == 0); + Assert.True (r.Subviews.IndexOf (v2) == 1); + Assert.True (r.Subviews.IndexOf (v3) == 2); + } + [Fact] public void TabIndex_Invert_Order_Mixed () {