Fixed TableView always showing selected cell(s) even when not focused (#1776)

* Fixed TableView always showing selected cell(s) even when not focused

* Changed CsvEditor label to a text field to show focus changing and allow manually entering a cell

* Fixed TableView_ColorsTest_ColorGetter and improved Exception when wrong colors are used

* Added full focused/not test suite for TableView

Terminal.Gui/Views/TableView.cs

@@ -486,6 +486,8 @@ namespace Terminal.Gui {
 		}
 		private void RenderRow (int row, int rowToRender, ColumnToRender [] columnsToRender)
 		{
+			var focused = HasFocus;
+
 			var rowScheme = (Style.RowColorGetter?.Invoke (
 				new RowColorGetterArgs(Table,rowToRender))) ?? ColorScheme;
 
@@ -495,8 +497,18 @@ namespace Terminal.Gui {
 
 			//start by clearing the entire line
 			Move (0, row);
-			Driver.SetAttribute (FullRowSelect && IsSelected (0, rowToRender) ? rowScheme.HotFocus
-				: Enabled ? rowScheme.Normal : rowScheme.Disabled);
+
+			Attribute color;
+
+			if(FullRowSelect && IsSelected (0, rowToRender)) {
+				color = focused ? rowScheme.HotFocus : rowScheme.HotNormal;
+			}
+			else 
+			{
+				color = Enabled ? rowScheme.Normal : rowScheme.Disabled;
+			}
+
+			Driver.SetAttribute (color);
 			Driver.AddStr (new string (' ', Bounds.Width));
 
 			// Render cells for each visible header for the current row
@@ -536,7 +548,12 @@ namespace Terminal.Gui {
 					scheme = rowScheme;
 				}
 
-				var cellColor = isSelectedCell ? scheme.HotFocus : Enabled ? scheme.Normal : scheme.Disabled;
+				Attribute cellColor;
+				if (isSelectedCell) {
+					cellColor = focused ? scheme.HotFocus : scheme.HotNormal;
+				} else {
+					cellColor = Enabled ? scheme.Normal : scheme.Disabled;
+				}
 
 				var render = TruncateOrPad (val, representation, current.Width, colStyle);
 
@@ -547,8 +564,14 @@ namespace Terminal.Gui {
 								
 				// Reset color scheme to normal for drawing separators if we drew text with custom scheme
 				if (scheme != rowScheme) {
-					Driver.SetAttribute (isSelectedCell ? rowScheme.HotFocus
-						: Enabled ? rowScheme.Normal : rowScheme.Disabled);
+
+					if(isSelectedCell) {
+						color = focused ? rowScheme.HotFocus : rowScheme.HotNormal;
+					}
+					else {
+						color = Enabled ? rowScheme.Normal : rowScheme.Disabled;
+					}
+					Driver.SetAttribute (color);
 				}
 
 				// If not in full row select mode always, reset color scheme to normal and render the vertical line (or space) at the end of the cell

UICatalog/Scenarios/CsvEditor.cs

@@ -8,6 +8,7 @@ using System.Globalization;
 using System.IO;
 using System.Text;
 using NStack;
+using System.Text.RegularExpressions;
 
 namespace UICatalog.Scenarios {
 
@@ -26,7 +27,7 @@ namespace UICatalog.Scenarios {
 		private MenuItem miLeft;
 		private MenuItem miRight;
 		private MenuItem miCentered;
-		private Label selectedCellLabel;
+		private TextField selectedCellLabel;
 
 		public override void Setup ()
 		{
@@ -78,14 +79,14 @@ namespace UICatalog.Scenarios {
 
 			Win.Add (tableView);
 
-			selectedCellLabel = new Label(){
+			selectedCellLabel = new TextField(){
 				X = 0,
 				Y = Pos.Bottom(tableView),
 				Text = "0,0",
 				Width = Dim.Fill(),
-				TextAlignment = TextAlignment.Right
-				
+				TextAlignment = TextAlignment.Right				
 			};
+			selectedCellLabel.TextChanged += SelectedCellLabel_TextChanged;
 
 			Win.Add(selectedCellLabel);
 
@@ -96,10 +97,26 @@ namespace UICatalog.Scenarios {
 			SetupScrollBar();
 		}
 
+		private void SelectedCellLabel_TextChanged (ustring last)
+		{
+			// if user is in the text control and editing the selected cell
+			if (!selectedCellLabel.HasFocus)
+				return;
+			
+			// change selected cell to the one the user has typed into the box
+			var match = Regex.Match (selectedCellLabel.Text.ToString(), "^(\\d+),(\\d+)$");
+			if(match.Success) {
+
+				tableView.SelectedColumn = int.Parse (match.Groups [1].Value);
+				tableView.SelectedRow = int.Parse (match.Groups [2].Value);
+			}
+		}
 
 		private void OnSelectedCellChanged (TableView.SelectedCellChangedEventArgs e)
 		{
-			selectedCellLabel.Text = $"{tableView.SelectedRow},{tableView.SelectedColumn}";
+			// only update the text box if the user is not manually editing it
+			if (!selectedCellLabel.HasFocus)
+				selectedCellLabel.Text = $"{tableView.SelectedRow},{tableView.SelectedColumn}";
 			
 			if(tableView.Table == null || tableView.SelectedColumn == -1)
 				return;

UnitTests/GraphViewTests.cs

@@ -240,7 +240,7 @@ namespace Terminal.Gui.Views {
 
 					var match = expectedColors.Where (e => e.Value == val).ToList ();
 					if (match.Count == 0) {
-						throw new Exception ($"Unexpected color {val} was used at row {r} and col {c}.  Color value was {val} (expected colors were {string.Join (",", expectedColors.Select (c => c.Value))})");
+						throw new Exception ($"Unexpected color {DescribeColor (val)} was used at row {r} and col {c} (indexes start at 0).  Color value was {val} (expected colors were {string.Join (",", expectedColors.Select (c => c.Value))})");
 					} else if (match.Count > 1) {
 						throw new ArgumentException ($"Bad value for expectedColors, {match.Count} Attributes had the same Value");
 					}
@@ -249,7 +249,7 @@ namespace Terminal.Gui.Views {
 					var userExpected = line [c];
 
 					if (colorUsed != userExpected) {
-						throw new Exception ($"Colors used did not match expected at row {r} and col {c}.  Color index used was {colorUsed} but test expected {userExpected} (these are indexes into the expectedColors array)");
+						throw new Exception ($"Colors used did not match expected at row {r} and col {c} (indexes start at 0).  Color index used was {DescribeColor(colorUsed)} but test expected {DescribeColor(userExpected)} (these are indexes into the expectedColors array)");
 					}
 				}
 
@@ -257,6 +257,12 @@ namespace Terminal.Gui.Views {
 			}
 		}
 
+		private static object DescribeColor (int userExpected)
+		{
+			var a = new Attribute (userExpected);
+			return $"{a.Foreground},{a.Background}";
+		}
+
 		#region Screen to Graph Tests
 
 		[Fact]

UnitTests/TableViewTests.cs

@@ -7,6 +7,7 @@ using Terminal.Gui;
 using Xunit;
 using System.Globalization;
 using Xunit.Abstractions;
+using System.Reflection;
 
 namespace Terminal.Gui.Views {
 
@@ -549,24 +550,216 @@ namespace Terminal.Gui.Views {
 			Assert.Equal ("R0C0", activatedValue);
 		}
 
-		[Fact]
-		public void TableView_ColorsTest_ColorGetter ()
+		[Theory]
+		[InlineData (false)]
+		[InlineData (true)]
+		public void TableView_ColorTests_FocusedOrNot (bool focused)
 		{
 			var tv = SetUpMiniTable ();
 
-			tv.Style.ExpandLastColumn = false;
+			// width exactly matches the max col widths
+			tv.Bounds = new Rect (0, 0, 5, 4);
+
+			// private method for forcing the view to be focused/not focused
+			var setFocusMethod = typeof (View).GetMethod ("SetHasFocus", BindingFlags.Instance | BindingFlags.NonPublic);
+
+			// when the view is/isn't focused 
+			setFocusMethod.Invoke (tv, new object [] { focused, tv, true });
+
+			tv.Redraw (tv.Bounds);
+
+			string expected = @"
+┌─┬─┐
+│A│B│
+├─┼─┤
+│1│2│
+";
+			GraphViewTests.AssertDriverContentsAre (expected, output);
+
+
+			string expectedColors = @"
+00000
+00000
+00000
+01000
+";
+			
+			GraphViewTests.AssertDriverColorsAre (expectedColors, new Attribute [] {
+				// 0
+				tv.ColorScheme.Normal,				
+				// 1
+				focused ? tv.ColorScheme.HotFocus : tv.ColorScheme.HotNormal});
+
+			Application.Shutdown();
+		}
+
+		[Theory]
+		[InlineData (false)]
+		[InlineData (true)]
+		public void TableView_ColorTests_InvertSelectedCellFirstCharacter (bool focused)
+		{
+			var tv = SetUpMiniTable ();
 			tv.Style.InvertSelectedCellFirstCharacter = true;
 
 			// width exactly matches the max col widths
 			tv.Bounds = new Rect (0, 0, 5, 4);
+
+			// private method for forcing the view to be focused/not focused
+			var setFocusMethod = typeof (View).GetMethod ("SetHasFocus", BindingFlags.Instance | BindingFlags.NonPublic);
+
+			// when the view is/isn't focused 
+			setFocusMethod.Invoke (tv, new object [] { focused, tv, true });
+
+			tv.Redraw (tv.Bounds);
+
+			string expected = @"
+┌─┬─┐
+│A│B│
+├─┼─┤
+│1│2│
+";
+			GraphViewTests.AssertDriverContentsAre (expected, output);
+
+
+			string expectedColors = @"
+00000
+00000
+00000
+01000
+";
+			
+			var invertHotFocus = new Attribute(tv.ColorScheme.HotFocus.Background,tv.ColorScheme.HotFocus.Foreground);
+			var invertHotNormal = new Attribute(tv.ColorScheme.HotNormal.Background,tv.ColorScheme.HotNormal.Foreground);
+
+			GraphViewTests.AssertDriverColorsAre (expectedColors, new Attribute [] {
+				// 0
+				tv.ColorScheme.Normal,				
+				// 1
+				focused ?  invertHotFocus : invertHotNormal});
+			
+			Application.Shutdown();
+		}
+
+
+		[Theory]
+		[InlineData (false)]
+		[InlineData (true)]
+		public void TableView_ColorsTest_RowColorGetter (bool focused)
+		{
+			var tv = SetUpMiniTable ();
+
+			// width exactly matches the max col widths
+			tv.Bounds = new Rect (0, 0, 5, 4);
+
+			var rowHighlight = new ColorScheme () {
+				Normal = Attribute.Make (Color.BrightCyan, Color.DarkGray),
+				HotNormal = Attribute.Make (Color.Green, Color.Blue),
+				HotFocus = Attribute.Make (Color.BrightYellow, Color.White),
+				Focus = Attribute.Make (Color.Cyan, Color.Magenta),
+			};
+
+			// when B is 2 use the custom highlight colour for the row
+			tv.Style.RowColorGetter += (e)=>Convert.ToInt32(e.Table.Rows[e.RowIndex][1]) == 2 ? rowHighlight : null;
+
+			// private method for forcing the view to be focused/not focused
+			var setFocusMethod = typeof (View).GetMethod ("SetHasFocus", BindingFlags.Instance | BindingFlags.NonPublic);
+
+			// when the view is/isn't focused 
+			setFocusMethod.Invoke (tv, new object [] { focused, tv, true });
+
+			tv.Redraw (tv.Bounds);
+
+			string expected = @"
+┌─┬─┐
+│A│B│
+├─┼─┤
+│1│2│
+";
+			GraphViewTests.AssertDriverContentsAre (expected, output);
+
+
+			string expectedColors = @"
+00000
+00000
+00000
+21222
+";
 			
+			GraphViewTests.AssertDriverColorsAre (expectedColors, new Attribute [] {
+				// 0
+				tv.ColorScheme.Normal,				
+				// 1
+				focused ? rowHighlight.HotFocus : rowHighlight.HotNormal,
+				// 2
+				rowHighlight.Normal});
+
+
+			// change the value in the table so that
+			// it no longer matches the RowColorGetter
+			// delegate conditional ( which checks for
+			// the value 2)
+			tv.Table.Rows[0][1] = 5;
+
+			tv.Redraw (tv.Bounds);
+			expected = @"
+┌─┬─┐
+│A│B│
+├─┼─┤
+│1│5│
+";
+			GraphViewTests.AssertDriverContentsAre (expected, output);
+
+
+			expectedColors = @"
+00000
+00000
+00000
+01000
+";
+
+			// now we only see 2 colors used (the selected cell color and Normal
+			// rowHighlight should no longer be used because the delegate returned null
+			// (now that the cell value is 5 - which does not match the conditional)
+			GraphViewTests.AssertDriverColorsAre (expectedColors, new Attribute [] {
+				// 0
+				tv.ColorScheme.Normal,
+				// 1
+				focused ? tv.ColorScheme.HotFocus : tv.ColorScheme.HotNormal });
+
+
+			// Shutdown must be called to safely clean up Application if Init has been called
+			Application.Shutdown ();
+		}
+
+		[Theory]
+		[InlineData (false)]
+		[InlineData (true)]
+		public void TableView_ColorsTest_ColorGetter (bool focused)
+		{
+			var tv = SetUpMiniTable ();
+
+			// width exactly matches the max col widths
+			tv.Bounds = new Rect (0, 0, 5, 4);
+
 			// Create a style for column B
 			var bStyle = tv.Style.GetOrCreateColumnStyle (tv.Table.Columns ["B"]);
 
 			// when B is 2 use the custom highlight colour
-			ColorScheme cellHighlight = new ColorScheme () { Normal = Attribute.Make (Color.BrightCyan, Color.DarkGray) };
+			var cellHighlight = new ColorScheme () {
+				Normal = Attribute.Make (Color.BrightCyan, Color.DarkGray),
+				HotNormal = Attribute.Make (Color.Green, Color.Blue),
+				HotFocus = Attribute.Make (Color.BrightYellow, Color.White),
+				Focus = Attribute.Make (Color.Cyan, Color.Magenta),
+			};
+
 			bStyle.ColorGetter = (a) => Convert.ToInt32(a.CellValue) == 2 ? cellHighlight : null;
 
+			// private method for forcing the view to be focused/not focused
+			var setFocusMethod = typeof (View).GetMethod ("SetHasFocus", BindingFlags.Instance | BindingFlags.NonPublic);
+
+			// when the view is/isn't focused 
+			setFocusMethod.Invoke (tv, new object [] { focused, tv, true });
+
 			tv.Redraw (tv.Bounds);
 
 			string expected = @"
@@ -577,22 +770,56 @@ namespace Terminal.Gui.Views {
 ";
 			GraphViewTests.AssertDriverContentsAre (expected, output);
 
+
 			string expectedColors = @"
 00000
 00000
 00000
 01020
 ";
-			var invertedNormalColor = Application.Driver.MakeAttribute (tv.ColorScheme.Normal.Background, tv.ColorScheme.Normal.Foreground);
-
+			
 			GraphViewTests.AssertDriverColorsAre (expectedColors, new Attribute [] {
 				// 0
 				tv.ColorScheme.Normal,				
 				// 1
-				invertedNormalColor,				
+				focused ? tv.ColorScheme.HotFocus : tv.ColorScheme.HotNormal,
 				// 2
 				cellHighlight.Normal});
 
+
+			// change the value in the table so that
+			// it no longer matches the ColorGetter
+			// delegate conditional ( which checks for
+			// the value 2)
+			tv.Table.Rows[0][1] = 5;
+
+			tv.Redraw (tv.Bounds);
+			expected = @"
+┌─┬─┐
+│A│B│
+├─┼─┤
+│1│5│
+";
+			GraphViewTests.AssertDriverContentsAre (expected, output);
+
+
+			expectedColors = @"
+00000
+00000
+00000
+01000
+";
+
+			// now we only see 2 colors used (the selected cell color and Normal
+			// cellHighlight should no longer be used because the delegate returned null
+			// (now that the cell value is 5 - which does not match the conditional)
+			GraphViewTests.AssertDriverColorsAre (expectedColors, new Attribute [] {
+				// 0
+				tv.ColorScheme.Normal,				
+				// 1
+				focused ? tv.ColorScheme.HotFocus : tv.ColorScheme.HotNormal });
+
+
 			// Shutdown must be called to safely clean up Application if Init has been called
 			Application.Shutdown ();
 		}
@@ -615,10 +842,7 @@ namespace Terminal.Gui.Views {
 			tv.Style.GetOrCreateColumnStyle (colB).MaxWidth = 1;
 
 			GraphViewTests.InitFakeDriver ();
-			tv.ColorScheme = new ColorScheme () {
-				Normal = Application.Driver.MakeAttribute (Color.White, Color.Black),
-				HotFocus = Application.Driver.MakeAttribute (Color.White, Color.Black)
-			};
+			tv.ColorScheme = Colors.Base;
 			return tv;
 		}