Merge pull request #1244 from sillsdev/sp-2297

SP-2297: Prevent changing row in ContributorsListControl if not in a valid state to commit edit

Author: tombogle

Palaso.sln.DotSettings

@@ -64,6 +64,7 @@
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=parsable/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Pashto/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Qaaa/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/UserDictionary/Words/=reentrant/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=Saami/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=SLDR/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/UserDictionary/Words/=subitem/@EntryIndexedValue">True</s:Boolean>

SIL.Windows.Forms/ClearShare/WinFormsUI/ContributorsListControl.cs

@@ -6,6 +6,7 @@
 using System.Linq;
 using System.Media;
 using System.Windows.Forms;
+using JetBrains.Annotations;
 using L10NSharp.UI;
 using SIL.Code;
 using SIL.Windows.Forms.Widgets.BetterGrid;
@@ -54,8 +55,6 @@ private void Initialize()
 		{
 			_grid.Font = SystemFonts.MenuFont;
 
-			// TODO: Localize column headings
-
 			DataGridViewColumn col = BetterGrid.CreateTextBoxColumn("name", "Name");
 			col.Width = 150;
 			_grid.Columns.Add(col);
@@ -74,7 +73,7 @@ private void Initialize()
 
 			_grid.AddRemoveRowColumn(null, null,
 				null /* TODO: Enhance BetterGrid to be able to show tool tips in non-virtual mode */,
-				rowIndex => DeleteRow(rowIndex));
+				DeleteRow);
 
 			_grid.AllowUserToAddRows = true;
 			_grid.AllowUserToDeleteRows = true;
@@ -88,15 +87,13 @@ private void Initialize()
 			_grid.RowsRemoved += HandleGridRowsRemoved;
 			_grid.ColumnHeaderMouseClick += _grid_ColumnHeaderMouseClick;
 
-			if (_model.ContributorsGridSettings != null)
-				_model.ContributorsGridSettings.InitializeGrid(_grid);
+			_model.ContributorsGridSettings?.InitializeGrid(_grid);
 		}
 
 		// SP-874: Not able to open L10NSharp with Alt-Shift-click
-		void _grid_ColumnHeaderMouseClick(object sender, DataGridViewCellMouseEventArgs e)
+		private void _grid_ColumnHeaderMouseClick(object sender, DataGridViewCellMouseEventArgs e)
 		{
-			if (ColumnHeaderMouseClick != null)
-				ColumnHeaderMouseClick(sender, e);
+			ColumnHeaderMouseClick?.Invoke(sender, e);
 		}
 
 		/// ------------------------------------------------------------------------------------
@@ -107,28 +104,20 @@ void _grid_ColumnHeaderMouseClick(object sender, DataGridViewCellMouseEventArgs
 		/// problem.
 		/// </summary>
 		/// ------------------------------------------------------------------------------------
-		private new bool DesignMode
-		{
-			get
-			{
-				return (base.DesignMode || GetService(typeof(IDesignerHost)) != null) ||
-					(LicenseManager.UsageMode == LicenseUsageMode.Designtime);
-			}
-		}
+		private new bool DesignMode => base.DesignMode ||
+			GetService(typeof(IDesignerHost)) != null ||
+			LicenseManager.UsageMode == LicenseUsageMode.Designtime;
 
 		/// ------------------------------------------------------------------------------------
-		public bool InEditMode
-		{
-			get { return _grid.IsCurrentRowDirty; }
-		}
+		[PublicAPI]
+		public bool InEditMode => _grid.IsCurrentRowDirty;
 
 		/// ------------------------------------------------------------------------------------
-		public bool InNewContributionRow
-		{
-			get { return (_grid.CurrentCellAddress.Y == _grid.NewRowIndex); }
-		}
+		[PublicAPI]
+		public bool InNewContributionRow => _grid.CurrentCellAddress.Y == _grid.NewRowIndex;
 
 		/// ------------------------------------------------------------------------------------
+		[PublicAPI]
 		public Contribution GetCurrentContribution()
 		{
 			return GetContributionFromRow(_grid.CurrentCellAddress.Y);
@@ -174,14 +163,32 @@ void HandleGridMouseClick(object sender, MouseEventArgs e)
 				return;
 			}
 
+			void SelectFirstCellInClickedRow() => _grid.CurrentCell = _grid[0, hi.RowIndex];
+
 			// At this point we know the user clicked on a row heading. Now we
 			// need to make sure the row they're leaving is in a valid state.
 			if (ValidatingContributor != null && _grid.CurrentCellAddress.Y >= 0 &&
 				_grid.CurrentCellAddress.Y < _grid.RowCount - 1)
 			{
-				if (_grid.CurrentCellAddress.Y == _model.Contributions.Count())
+				if (_grid.CurrentCellAddress.Y == _model.Contributions.Count)
 					return;
 
+				if (_grid.IsDirty)
+				{
+					try
+					{
+						// This actually forces the commit/validation and will fail if the
+						// current edit has the contribution in a bogus state.
+						SelectFirstCellInClickedRow();
+						return;
+					}
+					catch
+					{
+						SystemSounds.Beep.Play();
+						return;
+					}
+				}
+
 				var contribution = _model.Contributions.ElementAt(_grid.CurrentCellAddress.Y);
 				if (!GetIsValidContribution(contribution))
 				{
@@ -190,8 +197,7 @@ void HandleGridMouseClick(object sender, MouseEventArgs e)
 				}
 			}
 
-			// Make the first cell current in the row the user clicked.
-			_grid.CurrentCell = _grid[0, hi.RowIndex];
+			SelectFirstCellInClickedRow();
 		}
 
 		/// ------------------------------------------------------------------------------------
@@ -232,8 +238,7 @@ private void HandleGridRowValidating(object sender, DataGridViewCellCancelEventA
 
 			if (!string.IsNullOrEmpty(kvp.Key))
 			{
-				if (_msgWindow == null)
-					_msgWindow = new FadingMessageWindow();
+				_msgWindow ??= new FadingMessageWindow();
 
 				var dataGridViewColumn = _grid.Columns[kvp.Key];
 				if (dataGridViewColumn != null)
@@ -324,42 +329,40 @@ protected void HandleEditingControlShowing(object sender, DataGridViewEditingCon
 		/// ------------------------------------------------------------------------------------
 		void HandleRoleValueChanged(object sender, EventArgs e)
 		{
-			if (_msgWindow != null)
-				_msgWindow.Close();
+			_msgWindow?.Close();
 		}
 
 		/// ------------------------------------------------------------------------------------
 		void HandleGridCellEndEdit(object sender, DataGridViewCellEventArgs e)
 		{
 			var ctrl = _grid.Tag as Control;
 
+			var txtBox = ctrl as TextBox;
 			// SP-793: Text should match case of autocomplete list
-			if (e.ColumnIndex == 0)
+			if (e.ColumnIndex == 0 && txtBox != null)
 			{
-				var txtBox = ctrl as TextBox;
-				if (txtBox != null)
-				{
-					// is the current text an exact match for the autocomplete list?
-					var list = txtBox.AutoCompleteCustomSource.Cast<object>().ToList();
-					var found = list.FirstOrDefault(item => String.Equals(item.ToString(), txtBox.Text, StringComparison.CurrentCulture));
+				// is the current text an exact match for the autocomplete list?
+				var list = txtBox.AutoCompleteCustomSource.Cast<object>().ToList();
+				var found = list.FirstOrDefault(item =>
+					String.Equals(item.ToString(), txtBox.Text, StringComparison.CurrentCulture));
 
-					if (found == null)
+				if (found == null)
+				{
+					// is the current text a match except for case for the autocomplete list?
+					found = list.FirstOrDefault(item => String.Equals(item.ToString(),
+						txtBox.Text, StringComparison.CurrentCultureIgnoreCase));
+					if (found != null)
 					{
-						// is the current text a match except for case for the autocomplete list?
-						found = list.FirstOrDefault(item => String.Equals(item.ToString(), txtBox.Text, StringComparison.CurrentCultureIgnoreCase));
-						if (found != null)
-						{
-							txtBox.Text = found.ToString();
-							_grid.CurrentCell.Value = txtBox.Text;
-						}
+						txtBox.Text = found.ToString();
+						_grid.CurrentCell.Value = txtBox.Text;
 					}
 				}
 			}
 
-			if (ctrl is TextBox)
-				ctrl.KeyPress -= HandleCellEditBoxKeyPress;
-			else if (ctrl is ComboBox)
-				((ComboBox)ctrl).SelectedIndexChanged -= HandleRoleValueChanged;
+			if (txtBox != null)
+				txtBox.KeyPress -= HandleCellEditBoxKeyPress;
+			else if (ctrl is ComboBox box)
+				box.SelectedIndexChanged -= HandleRoleValueChanged;
 
 			_grid.CellEndEdit -= HandleGridCellEndEdit;
 			_grid.Tag = null;
@@ -399,15 +402,13 @@ public void SetColumnHeaderText(int columnIndex, string headerText)
 
 		/// <remarks>SP-874: Localize column headers</remarks>
 		[CLSCompliant (false)]
+		[PublicAPI]
 		public void SetLocalizationExtender(L10NSharpExtender extender)
 		{
 			extender.SetLocalizingId(_grid, "ContributorsEditorGrid");
 		}
 
 		/// <remarks>We need to be able to adjust the visual properties to match the hosting program</remarks>
-		public BetterGrid Grid
-		{
-			get { return _grid; }
-		}
+		public BetterGrid Grid => _grid;
 	}
 }

TestApps/SIL.Windows.Forms.TestApp/ContributorsForm.cs

@@ -9,7 +9,7 @@
 
 namespace SIL.Windows.Forms.TestApp
 {
-	public partial class ContributorsForm : Form
+	public class ContributorsForm : Form
 	{
 		private TableLayoutPanel _tableLayout;
 		private ContributorsListControl _contributorsControl;
@@ -36,32 +36,19 @@ public ContributorsForm()
 		{
 			var autoCompleter = new AutoCompleter();
 			_model = new ContributorsListControlViewModel(autoCompleter, () => { });
-			var dataGridView = new DataGridView();
-
-			_contributorsControl = new ContributorsListControl(_model);
-			_contributorsControl.Dock = DockStyle.Fill;
-			_contributorsControl.Location = new System.Drawing.Point(0, 0);
-			_contributorsControl.Name = "_contributorsControl";
-
-			_contributorsControl.ValidatingContributor += HandleValidatingContributor;
-
-			// set the column header text
-			string[] headerText =
-			{
-				"Name",
-				"Role",
-				"Date",
-				"Comments"
-			};
-
-			for (var i = 0; i < headerText.Length; i++)
-				_contributorsControl.SetColumnHeaderText(i, headerText[i]);
 
 			InitializeComponent();
 			autoCompleter.Source = _contributorNames;
 
 			var contribs = new ContributionCollection(new [] { new Contribution("Fred", new Role("a", "Author", "guy who writes stuff")) });
 			_model.SetContributionList(contribs);
+			
+			_contributorsControl.Grid.Columns["name"].AutoSizeMode =
+				DataGridViewAutoSizeColumnMode.AllCells;
+			_contributorsControl.Grid.Columns["role"].AutoSizeMode =
+				DataGridViewAutoSizeColumnMode.AllCells;
+			_contributorsControl.Grid.Columns["comments"].AutoSizeMode =
+				DataGridViewAutoSizeColumnMode.Fill;
 		}
 
 		private void InitializeComponent()
@@ -111,6 +98,26 @@ private void InitializeComponent()
 			_contributorNames.Rows[1].Cells[0].Value = "Fred";
 			_contributorNames.Rows[2].Cells[0].Value = "Tom";
 
+			_contributorsControl = new ContributorsListControl(_model)
+			{
+				Dock = DockStyle.Fill,
+				Location = new System.Drawing.Point(0, 0),
+				Name = "_contributorsControl"
+			};
+			_contributorsControl.ValidatingContributor += HandleValidatingContributor;
+
+			// set the column header text
+			string[] headerText =
+			{
+				"Name",
+				"Role",
+				"Date",
+				"Comments"
+			};
+
+			for (var i = 0; i < headerText.Length; i++)
+				_contributorsControl.SetColumnHeaderText(i, headerText[i]);
+
 			_tableLayout.Controls.Add(_contributorsControl, 0, 0);
 			_tableLayout.SetColumnSpan(_contributorsControl, 2);
 			_tableLayout.Controls.Add(_contributorNames, 0, 1);