TreeView: why highlight entire row?

[tig]

@tznind, TreeView currently highlights the entire row fo the selected item:

focus:

notfocus:

I think it would look better if just the text of the node was highlighted.

here's a potential implementation:

		/// <summary>
		/// Renders the current <see cref="Model"/> on the specified line <paramref name="y"/>
		/// </summary>
		/// <param name="driver"></param>
		/// <param name="colorScheme"></param>
		/// <param name="y"></param>
		/// <param name="availableWidth"></param>
		public virtual void Draw (ConsoleDriver driver, ColorScheme colorScheme, int y, int availableWidth)
		{
			// true if the current line of the tree is the selected one and control has focus
			bool isSelected = tree.IsSelected (Model);// && tree.HasFocus;
			Attribute textColor = isSelected ? (tree.HasFocus ? colorScheme.HotFocus : colorScheme.HotNormal) : colorScheme.Normal ;
			Attribute symbolColor = colorScheme.Normal;

			// Everything on line before the expansion run and branch text
			Rune [] prefix = GetLinePrefix (driver).ToArray ();
			Rune expansion = GetExpandableSymbol (driver);
			string lineBody = tree.AspectGetter (Model) ?? "";

			tree.Move (0, y);

			// if we have scrolled to the right then bits of the prefix will have dispeared off the screen
			int toSkip = tree.ScrollOffsetHorizontal;

			driver.SetAttribute (symbolColor);
			// Draw the line prefix (all parallel lanes or whitespace and an expand/collapse/leaf symbol)
			foreach (Rune r in prefix) {

				if (toSkip > 0) {
					toSkip--;
				} else {
					driver.AddRune (r);
					availableWidth -= Rune.ColumnWidth (r);
				}
			}

			// pick color for expanded symbol
			if (tree.Style.ColorExpandSymbol || tree.Style.InvertExpandSymbolColors) {
				Attribute color = symbolColor;

				if (tree.Style.ColorExpandSymbol) {
					color = isSelected ? tree.ColorScheme.HotFocus : tree.ColorScheme.HotNormal;
				} else {
					color = symbolColor;
				}

				if (tree.Style.InvertExpandSymbolColors) {
					color = new Attribute (color.Background, color.Foreground);
				}

				driver.SetAttribute (color);
			}

			if (toSkip > 0) {
				toSkip--;
			} else {
				driver.AddRune (expansion);
				availableWidth -= Rune.ColumnWidth (expansion);
			}

			// horizontal scrolling has already skipped the prefix but now must also skip some of the line body
			if (toSkip > 0) {
				if (toSkip > lineBody.Length) {
					lineBody = "";
				} else {
					lineBody = lineBody.Substring (toSkip);
				}
			}

			// If body of line is too long
			if (lineBody.Sum (l => Rune.ColumnWidth (l)) > availableWidth) {
				// remaining space is zero and truncate the line
				lineBody = new string (lineBody.TakeWhile (c => (availableWidth -= Rune.ColumnWidth (c)) >= 0).ToArray ());
				availableWidth = 0;
			} else {

				// line is short so remaining width will be whatever comes after the line body
				availableWidth -= lineBody.Length;
			}


			// default behaviour is for model to use the color scheme
			// of the tree view
			var modelColor = textColor;

			// if custom color delegate invoke it
			if(tree.ColorGetter != null)
			{
				var modelScheme = tree.ColorGetter(Model);

				// if custom color scheme is defined for this Model
				if(modelScheme != null)
				{
					// use it
					modelColor = isSelected ? modelScheme.Focus : modelScheme.Normal;
				}
			}

			driver.SetAttribute (modelColor);
			driver.AddStr (lineBody);
			driver.SetAttribute (colorScheme.Normal);

			if (availableWidth > 0) {
				driver.AddStr (new string (' ', availableWidth));
			}
		}

No UnitTests broke with this...

What do you think?

[tig]

See #2155

[tznind]

Nice, I like it. But I think it should be a setting that can be toggled.

Both for backwards compatibility and for preference. I could see some people preferring the full row select especially if the control is used alongside ListView.

If you agree then it should be added to TreeStyle class for consistency and discoverability.

[tig]

Not sure I agree (visually, it's clearly my preference) and I'm biased against knobs and dials.

But it is your feature and I'll defer to you. Happy to tweak the PR to add a flag...

[tznind]

Visual Studio Code has a tree view with a full row select. I think we should support both. I agree that flags can get out of hand but it is at least in a dedicated Style subclass so won't clutter the root TreeView API.

[tig]

You're right about VS Code (and VS). Ok, I'm sold on the idea of a style.