diff --git a/Source/Terminals/Forms/EditFavorite/GeneralPropertiesUserControl.Designer.cs b/Source/Terminals/Forms/EditFavorite/GeneralPropertiesUserControl.Designer.cs index bc95f2da..904a684b 100644 --- a/Source/Terminals/Forms/EditFavorite/GeneralPropertiesUserControl.Designer.cs +++ b/Source/Terminals/Forms/EditFavorite/GeneralPropertiesUserControl.Designer.cs @@ -54,7 +54,7 @@ private void InitializeComponent() this.chkAddtoToolbar.Location = new System.Drawing.Point(136, 272); this.chkAddtoToolbar.Name = "chkAddtoToolbar"; this.chkAddtoToolbar.Size = new System.Drawing.Size(96, 17); - this.chkAddtoToolbar.TabIndex = 39; + this.chkAddtoToolbar.TabIndex = 13; this.chkAddtoToolbar.Text = "Add to &Toolbar"; this.chkAddtoToolbar.UseVisualStyleBackColor = true; // @@ -65,7 +65,7 @@ private void InitializeComponent() this.NewWindowCheckbox.Location = new System.Drawing.Point(136, 252); this.NewWindowCheckbox.Name = "NewWindowCheckbox"; this.NewWindowCheckbox.Size = new System.Drawing.Size(130, 17); - this.NewWindowCheckbox.TabIndex = 40; + this.NewWindowCheckbox.TabIndex = 12; this.NewWindowCheckbox.Text = "&Open in New Window"; this.NewWindowCheckbox.UseVisualStyleBackColor = true; // @@ -74,7 +74,7 @@ private void InitializeComponent() this.httpUrlTextBox.Location = new System.Drawing.Point(133, 45); this.httpUrlTextBox.Name = "httpUrlTextBox"; this.httpUrlTextBox.Size = new System.Drawing.Size(259, 20); - this.httpUrlTextBox.TabIndex = 38; + this.httpUrlTextBox.TabIndex = 4; this.httpUrlTextBox.Text = "https://github.com/Terminals-Origin/Terminals/issues"; this.httpUrlTextBox.Visible = false; this.httpUrlTextBox.TextChanged += new System.EventHandler(this.HttpUrlTextBox_TextChanged); @@ -84,14 +84,14 @@ private void InitializeComponent() this.txtPort.Location = new System.Drawing.Point(443, 43); this.txtPort.Name = "txtPort"; this.txtPort.Size = new System.Drawing.Size(46, 20); - this.txtPort.TabIndex = 26; + this.txtPort.TabIndex = 6; // // txtName // this.txtName.Location = new System.Drawing.Point(133, 75); this.txtName.Name = "txtName"; this.txtName.Size = new System.Drawing.Size(325, 20); - this.txtName.TabIndex = 28; + this.txtName.TabIndex = 8; // // chkSavePassword // @@ -100,7 +100,7 @@ private void InitializeComponent() this.chkSavePassword.Location = new System.Drawing.Point(136, 232); this.chkSavePassword.Name = "chkSavePassword"; this.chkSavePassword.Size = new System.Drawing.Size(99, 17); - this.chkSavePassword.TabIndex = 6; + this.chkSavePassword.TabIndex = 11; this.chkSavePassword.Text = "S&ave password"; this.chkSavePassword.UseVisualStyleBackColor = true; // @@ -112,7 +112,7 @@ private void InitializeComponent() this.pictureBox2.Name = "pictureBox2"; this.pictureBox2.Size = new System.Drawing.Size(16, 16); this.pictureBox2.SizeMode = System.Windows.Forms.PictureBoxSizeMode.StretchImage; - this.pictureBox2.TabIndex = 36; + this.pictureBox2.TabIndex = 9; this.pictureBox2.TabStop = false; this.pictureBox2.Click += new System.EventHandler(this.PictureBox2_Click); // @@ -122,7 +122,7 @@ private void InitializeComponent() this.lblPort.Location = new System.Drawing.Point(398, 46); this.lblPort.Name = "lblPort"; this.lblPort.Size = new System.Drawing.Size(29, 13); - this.lblPort.TabIndex = 25; + this.lblPort.TabIndex = 5; this.lblPort.Text = "Port:"; // // ProtocolComboBox @@ -133,7 +133,7 @@ private void InitializeComponent() this.ProtocolComboBox.MaxDropDownItems = 10; this.ProtocolComboBox.Name = "ProtocolComboBox"; this.ProtocolComboBox.Size = new System.Drawing.Size(356, 21); - this.ProtocolComboBox.TabIndex = 35; + this.ProtocolComboBox.TabIndex = 1; this.ProtocolComboBox.SelectedIndexChanged += new System.EventHandler(this.ProtocolComboBox_SelectedIndexChanged); // // ProtocolLabel @@ -142,7 +142,7 @@ private void InitializeComponent() this.ProtocolLabel.Location = new System.Drawing.Point(6, 17); this.ProtocolLabel.Name = "ProtocolLabel"; this.ProtocolLabel.Size = new System.Drawing.Size(49, 13); - this.ProtocolLabel.TabIndex = 34; + this.ProtocolLabel.TabIndex = 0; this.ProtocolLabel.Text = "&Protocol:"; // // label5 @@ -151,7 +151,7 @@ private void InitializeComponent() this.label5.Location = new System.Drawing.Point(6, 78); this.label5.Name = "label5"; this.label5.Size = new System.Drawing.Size(93, 13); - this.label5.TabIndex = 27; + this.label5.TabIndex = 7; this.label5.Text = "Connection na&me:"; // // cmbServers @@ -161,7 +161,7 @@ private void InitializeComponent() this.cmbServers.Location = new System.Drawing.Point(133, 45); this.cmbServers.Name = "cmbServers"; this.cmbServers.Size = new System.Drawing.Size(259, 21); - this.cmbServers.TabIndex = 24; + this.cmbServers.TabIndex = 3; this.cmbServers.SelectedIndexChanged += new System.EventHandler(this.CmbServers_SelectedIndexChanged); this.cmbServers.TextChanged += new System.EventHandler(this.CmbServers_TextChanged); this.cmbServers.Leave += new System.EventHandler(this.CmbServers_Leave); @@ -172,7 +172,7 @@ private void InitializeComponent() this.lblServerName.Location = new System.Drawing.Point(6, 46); this.lblServerName.Name = "lblServerName"; this.lblServerName.Size = new System.Drawing.Size(55, 13); - this.lblServerName.TabIndex = 23; + this.lblServerName.TabIndex = 2; this.lblServerName.Text = "&Computer:"; // // securityPanel1 @@ -180,7 +180,7 @@ private void InitializeComponent() this.securityPanel1.Location = new System.Drawing.Point(6, 101); this.securityPanel1.Name = "securityPanel1"; this.securityPanel1.Size = new System.Drawing.Size(483, 128); - this.securityPanel1.TabIndex = 41; + this.securityPanel1.TabIndex = 10; // // GeneralPropertiesUserControl // diff --git a/Source/Terminals/Forms/EditFavorite/GroupsControl.Designer.cs b/Source/Terminals/Forms/EditFavorite/GroupsControl.Designer.cs index eb0fa5e9..df360f3b 100644 --- a/Source/Terminals/Forms/EditFavorite/GroupsControl.Designer.cs +++ b/Source/Terminals/Forms/EditFavorite/GroupsControl.Designer.cs @@ -73,7 +73,7 @@ private void InitializeComponent() this.btnRemoveTag.Location = new System.Drawing.Point(521, 74); this.btnRemoveTag.Name = "btnRemoveTag"; this.btnRemoveTag.Size = new System.Drawing.Size(21, 21); - this.btnRemoveTag.TabIndex = 1; + this.btnRemoveTag.TabIndex = 5; this.btnRemoveTag.UseVisualStyleBackColor = true; this.btnRemoveTag.Click += new System.EventHandler(this.BtnRemoveTag_Click); // @@ -83,7 +83,7 @@ private void InitializeComponent() this.lvConnectionTags.Location = new System.Drawing.Point(12, 74); this.lvConnectionTags.Name = "lvConnectionTags"; this.lvConnectionTags.Size = new System.Drawing.Size(503, 66); - this.lvConnectionTags.TabIndex = 0; + this.lvConnectionTags.TabIndex = 4; this.lvConnectionTags.UseCompatibleStateImageBehavior = false; this.lvConnectionTags.View = System.Windows.Forms.View.List; this.lvConnectionTags.DoubleClick += new System.EventHandler(this.LvConnectionTags_DoubleClick); @@ -94,7 +94,7 @@ private void InitializeComponent() this.AllTagsAddButton.Location = new System.Drawing.Point(521, 176); this.AllTagsAddButton.Name = "AllTagsAddButton"; this.AllTagsAddButton.Size = new System.Drawing.Size(21, 21); - this.AllTagsAddButton.TabIndex = 1; + this.AllTagsAddButton.TabIndex = 8; this.AllTagsAddButton.UseVisualStyleBackColor = true; this.AllTagsAddButton.Click += new System.EventHandler(this.AllTagsAddButton_Click); // @@ -104,7 +104,7 @@ private void InitializeComponent() this.AllTagsListView.Location = new System.Drawing.Point(9, 176); this.AllTagsListView.Name = "AllTagsListView"; this.AllTagsListView.Size = new System.Drawing.Size(506, 150); - this.AllTagsListView.TabIndex = 0; + this.AllTagsListView.TabIndex = 7; this.AllTagsListView.UseCompatibleStateImageBehavior = false; this.AllTagsListView.View = System.Windows.Forms.View.List; this.AllTagsListView.DoubleClick += new System.EventHandler(this.AllTagsListView_DoubleClick); @@ -124,7 +124,7 @@ private void InitializeComponent() this.allLabel.Location = new System.Drawing.Point(9, 160); this.allLabel.Name = "allLabel"; this.allLabel.Size = new System.Drawing.Size(101, 13); - this.allLabel.TabIndex = 4; + this.allLabel.TabIndex = 6; this.allLabel.Text = "All available groups:"; // // GroupsControl diff --git a/Source/Tests/Tests.csproj b/Source/Tests/Tests.csproj index 4c76e091..910c3589 100644 --- a/Source/Tests/Tests.csproj +++ b/Source/Tests/Tests.csproj @@ -190,6 +190,7 @@ + diff --git a/Source/Tests/UserInterface/NewTerminalTabOrderTests.cs b/Source/Tests/UserInterface/NewTerminalTabOrderTests.cs new file mode 100644 index 00000000..b5bf9a6f --- /dev/null +++ b/Source/Tests/UserInterface/NewTerminalTabOrderTests.cs @@ -0,0 +1,107 @@ +using System.Collections.Generic; +using System.Linq; +using System.Windows.Forms; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Terminals.Forms.EditFavorite; + +namespace Tests.UserInterface +{ + /// + /// Regression tests for issue #132: the General and Groups sections of the + /// New Connection dialog had random/colliding tab order indices, which made + /// keyboard navigation jump around the form. These tests assert the tab order + /// of each section follows the visible reading order (top to bottom, then left + /// to right) and that focusable controls do not share a tab index. + /// + [TestClass] + public class NewTerminalTabOrderTests + { + /// + /// Controls on the same visual row may differ slightly in Top (a label and + /// its field are not pixel aligned). Treat anything within this band as one row. + /// + private const int RowTolerance = 12; + + [TestMethod] + public void GeneralProperties_TabOrder_FollowsReadingOrder() + { + using (var control = new GeneralPropertiesUserControl()) + AssertReadingOrder(control); + } + + [TestMethod] + public void Groups_TabOrder_FollowsReadingOrder() + { + using (var control = new GroupsControl()) + AssertReadingOrder(control); + } + + [TestMethod] + public void GeneralProperties_FocusableControls_HaveUniqueTabIndex() + { + using (var control = new GeneralPropertiesUserControl()) + AssertUniqueTabIndex(control); + } + + [TestMethod] + public void Groups_FocusableControls_HaveUniqueTabIndex() + { + using (var control = new GroupsControl()) + AssertUniqueTabIndex(control); + } + + /// + /// Walks the focusable controls in tab order and verifies each next control is + /// either below the previous one, or on the same row but further right - never + /// jumping back up the form. + /// + private static void AssertReadingOrder(Control container) + { + List ordered = FocusableControls(container) + .OrderBy(c => c.TabIndex) + .ToList(); + + for (int i = 1; i < ordered.Count; i++) + { + Control previous = ordered[i - 1]; + Control current = ordered[i]; + + bool sameRow = System.Math.Abs(current.Top - previous.Top) <= RowTolerance; + bool isBelow = current.Top - previous.Top > RowTolerance; + bool rightOnSameRow = sameRow && current.Left >= previous.Left; + + string message = string.Format( + "Tab order jumps backwards: '{0}' (TabIndex {1}, {2}) should not come before '{3}' (TabIndex {4}, {5}).", + previous.Name, previous.TabIndex, FormatLocation(previous), + current.Name, current.TabIndex, FormatLocation(current)); + + Assert.IsTrue(isBelow || rightOnSameRow, message); + } + } + + private static void AssertUniqueTabIndex(Control container) + { + List focusable = FocusableControls(container).ToList(); + var duplicates = focusable + .GroupBy(c => c.TabIndex) + .Where(g => g.Count() > 1) + .Select(g => string.Format("TabIndex {0}: {1}", g.Key, string.Join(", ", g.Select(c => c.Name)))) + .ToList(); + + Assert.AreEqual(0, duplicates.Count, + "Focusable controls must not share a tab index. Collisions: " + string.Join(" | ", duplicates)); + } + + private static IEnumerable FocusableControls(Control container) + { + // Labels (TabStop == false) and hidden alternates are skipped by real tab + // navigation, so they are excluded here too. + return container.Controls.Cast().Where(c => c.TabStop && c.Visible); + } + + private static string FormatLocation(Control control) + { + return string.Format("Top={0},Left={1}", control.Top, control.Left); + } + } +}