Refactored SetRelativeLayout etc; looks good but tests fail

Terminal.Gui/Core/View.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.ComponentModel;
+using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
 using NStack;
@@ -2174,162 +2175,104 @@ namespace Terminal.Gui {
 		}
 
 		/// <summary>
-		/// Sets the View's <see cref="Frame"/> to the relative coordinates if its container, given the <see cref="Frame"/> for its container.
+		/// Sets the View's <see cref="Frame"/> to the frame-relative coordinates if its container. The
+		/// container size and location are specified by <paramref name="superviewFrame"/> and are relative to the
+		/// View's superview.
 		/// </summary>
-		/// <param name="hostFrame">The screen-relative frame for the host.</param>
-		/// <remarks>
-		/// Reminder: <see cref="Frame"/> is superview-relative; <see cref="Bounds"/> is view-relative.
-		/// </remarks>
-		internal void SetRelativeLayout (Rect hostFrame)
+		/// <param name="superviewFrame">The supserview-relative rectangle describing View's container (nominally the 
+		/// same as <c>this.SuperView.Frame</c>).</param>
+		internal void SetRelativeLayout (Rect superviewFrame)
 		{
-			int actW, actH, actX, actY;
-			var s = Size.Empty;
+			int newX, newW, newY, newH;
+			var autosize = Size.Empty;
 
 			if (AutoSize) {
-				s = GetAutoSize ();
+				// Note this is global to this function and used as such within the local functions defined
+				// below. In v2 AutoSize will be re-factored to not need to be dealt with in this function.
+				autosize = GetAutoSize ();
 			}
 
-			if (x is Pos.PosCenter) {
-				if (width == null) {
-					actW = AutoSize ? s.Width : hostFrame.Width;
-				} else {
-					actW = width.Anchor (hostFrame.Width);
-					actW = AutoSize && s.Width > actW ? s.Width : actW;
-				}
-				actX = x.Anchor (hostFrame.Width - actW);
-			} else {
-				actX = x?.Anchor (hostFrame.Width) ?? 0;
+			// Returns the new dimension (width or height) and location (x or y) for the View given
+			//   the superview's Frame.X or Frame.Y
+			//   the superview's width or height
+			//   the current Pos (View.X or View.Y)
+			//   the current Dim (View.Width or View.Height)
+			(int newLocation, int newDimension) GetNewLocationAndDimension (int superviewLocation, int superviewDimension, Pos pos, Dim dim, int autosizeDimension)
+			{				
+				int newDimension, newLocation;
 
-				actW = Math.Max (CalculateActualWidth (width, hostFrame, actX, s), 0);
-			}
+				switch (pos) {
+				case Pos.PosCenter:
+					if (dim == null) {
+						newDimension = AutoSize ? autosizeDimension : superviewDimension;
+					} else {
+						newDimension = width.Anchor (superviewDimension);
+						newDimension = AutoSize && autosizeDimension > newDimension ? autosizeDimension : newDimension;
+					}
+					newLocation = pos.Anchor (superviewDimension - newDimension);
+					break;
 
-			if (y is Pos.PosCenter) {
-				if (height == null) {
-					actH = AutoSize ? s.Height : hostFrame.Height;
-				} else {
-					actH = height.Anchor (hostFrame.Height);
-					actH = AutoSize && s.Height > actH ? s.Height : actH;
+				case Pos.PosAbsolute:
+				case Pos.PosAnchorEnd:
+				case Pos.PosCombine:
+				case Pos.PosFactor:
+				case Pos.PosFunc:
+				case Pos.PosView:
+				default:
+					newLocation = pos?.Anchor (superviewDimension) ?? 0;
+					newDimension = Math.Max (CalculateNewDimension (dim, newLocation, superviewDimension, autosizeDimension), 0);
+					break;
 				}
-				actY = y.Anchor (hostFrame.Height - actH);
-			} else {
-				actY = y?.Anchor (hostFrame.Height) ?? 0;
-
-				actH = Math.Max (CalculateActualHight (height, hostFrame, actY, s), 0);
+				return (newLocation, newDimension);
 			}
 
-			var r = new Rect (actX, actY, actW, actH);
-			if (Frame != r) {
-				Frame = r;
-				if (!SetMinWidthHeight ())
-					TextFormatter.Size = GetBoundsTextFormatterSize ();
-			}
-		}
-
-		private int CalculateActualWidth (Dim width, Rect hostFrame, int actX, Size s)
-		{
-			int actW;
-			switch (width) {
-			case null:
-				actW = AutoSize ? s.Width : hostFrame.Width;
-				break;
-			case Dim.DimCombine combine:
-				int leftActW = CalculateActualWidth (combine.left, hostFrame, actX, s);
-				int rightActW = CalculateActualWidth (combine.right, hostFrame, actX, s);
-				if (combine.add) {
-					actW = leftActW + rightActW;
-				} else {
-					actW = leftActW - rightActW;
+			// Recursively calculates the new dimension (width or height) of the given Dim given:
+			//   the current location (x or y)
+			//   the current dimennsion (width or height)
+			int CalculateNewDimension (Dim d, int location, int dimension, int autosize)
+			{
+				int newDimension;
+				switch (d) {
+				case null:
+					newDimension = AutoSize ? autosize : dimension;
+					break;
+				case Dim.DimCombine combine:
+					int leftActW = CalculateNewDimension (combine.left, location, dimension, autosize);
+					int rightActW = CalculateNewDimension (combine.right, dimension, location, autosize);
+					if (combine.add) {
+						newDimension = leftActW + rightActW;
+					} else {
+						newDimension = leftActW - rightActW;
+					}
+					newDimension = AutoSize && autosize > newDimension ? autosize : newDimension;
+					break;
+				case Dim.DimFactor factor when !factor.IsFromRemaining ():
+					newDimension = d.Anchor (dimension);
+					newDimension = AutoSize && autosize > newDimension ? autosize : newDimension;
+					break;
+				default:
+					newDimension = Math.Max (d.Anchor (dimension - location), 0);
+					newDimension = AutoSize && autosize > newDimension ? autosize : newDimension;
+					break;
 				}
-				actW = AutoSize && s.Width > actW ? s.Width : actW;
-				break;
-			case Dim.DimFactor factor when !factor.IsFromRemaining ():
-				actW = width.Anchor (hostFrame.Width);
-				actW = AutoSize && s.Width > actW ? s.Width : actW;
-				break;
-			default:
-				actW = Math.Max (width.Anchor (hostFrame.Width - actX), 0);
-				actW = AutoSize && s.Width > actW ? s.Width : actW;
-				break;
-			}
-
-			return actW;
-		}
 
-		private int CalculateActualHight (Dim height, Rect hostFrame, int actY, Size s)
-		{
-			int actH;
-			switch (height) {
-			case null:
-				actH = AutoSize ? s.Height : hostFrame.Height;
-				break;
-			case Dim.DimCombine combine:
-				int leftActH = CalculateActualHight (combine.left, hostFrame, actY, s);
-				int rightActH = CalculateActualHight (combine.right, hostFrame, actY, s);
-				if (combine.add) {
-					actH = leftActH + rightActH;
-				} else {
-					actH = leftActH - rightActH;
-				}
-				actH = AutoSize && s.Height > actH ? s.Height : actH;
-				break;
-			case Dim.DimFactor factor when !factor.IsFromRemaining ():
-				actH = height.Anchor (hostFrame.Height);
-				actH = AutoSize && s.Height > actH ? s.Height : actH;
-				break;
-			default:
-				actH = Math.Max (height.Anchor (hostFrame.Height - actY), 0);
-				actH = AutoSize && s.Height > actH ? s.Height : actH;
-				break;
+				return newDimension;
 			}
 
-			return actH;
-		}
-
-		// https://en.wikipedia.org/wiki/Topological_sorting
-		List<View> TopologicalSort (IEnumerable<View> nodes, ICollection<(View From, View To)> edges)
-		{
-			var result = new List<View> ();
-
-			// Set of all nodes with no incoming edges
-			var noEdgeNodes = new HashSet<View> (nodes.Where (n => edges.All (e => !e.To.Equals (n))));
-
-			while (noEdgeNodes.Any ()) {
-				//  remove a node n from S
-				var n = noEdgeNodes.First ();
-				noEdgeNodes.Remove (n);
-
-				// add n to tail of L
-				if (n != this?.SuperView)
-					result.Add (n);
-
-				// for each node m with an edge e from n to m do
-				foreach (var e in edges.Where (e => e.From.Equals (n)).ToArray ()) {
-					var m = e.To;
 
-					// remove edge e from the graph
-					edges.Remove (e);
+			// horiztonal
+			(newX, newW) = GetNewLocationAndDimension (superviewFrame.X, superviewFrame.Width, x, Width, autosize.Width);
 
-					// if m has no other incoming edges then
-					if (edges.All (me => !me.To.Equals (m)) && m != this?.SuperView) {
-						// insert m into S
-						noEdgeNodes.Add (m);
-					}
-				}
-			}
+			// vertical
+			(newY, newH) = GetNewLocationAndDimension (superviewFrame.Y, superviewFrame.Height, y, Height, autosize.Height);
 
-			if (edges.Any ()) {
-				(var from, var to) = edges.First ();
-				if (from != Application.Top) {
-					if (!ReferenceEquals (from, to)) {
-						throw new InvalidOperationException ($"TopologicalSort (for Pos/Dim) cannot find {from} linked with {to}. Did you forget to add it to {this}?");
-					} else {
-						throw new InvalidOperationException ("TopologicalSort encountered a recursive cycle in the relative Pos/Dim in the views of " + this);
-					}
+			var r = new Rect (newX, newY, newW, newH);
+			if (Frame != r) {
+				Frame = r;
+				if (!SetMinWidthHeight ()) {
+					TextFormatter.Size = GetBoundsTextFormatterSize ();
 				}
 			}
-
-			// return L (a topologically sorted order)
-			return result;
 		}
 
 		/// <summary>
@@ -2399,7 +2342,6 @@ namespace Terminal.Gui {
 
 			TextFormatter.Size = GetBoundsTextFormatterSize ();
 
-
 			// Sort out the dependencies of the X, Y, Width, Height properties
 			var nodes = new HashSet<View> ();
 			var edges = new HashSet<(View, View)> ();
@@ -2460,6 +2402,52 @@ namespace Terminal.Gui {
 
 			CollectAll (this, ref nodes, ref edges);
 
+			// https://en.wikipedia.org/wiki/Topological_sorting
+			List<View> TopologicalSort (IEnumerable<View> nodes, ICollection<(View From, View To)> edges)
+			{
+				var result = new List<View> ();
+
+				// Set of all nodes with no incoming edges
+				var noEdgeNodes = new HashSet<View> (nodes.Where (n => edges.All (e => !e.To.Equals (n))));
+
+				while (noEdgeNodes.Any ()) {
+					//  remove a node n from S
+					var n = noEdgeNodes.First ();
+					noEdgeNodes.Remove (n);
+
+					// add n to tail of L
+					if (n != this?.SuperView)
+						result.Add (n);
+
+					// for each node m with an edge e from n to m do
+					foreach (var e in edges.Where (e => e.From.Equals (n)).ToArray ()) {
+						var m = e.To;
+
+						// remove edge e from the graph
+						edges.Remove (e);
+
+						// if m has no other incoming edges then
+						if (edges.All (me => !me.To.Equals (m)) && m != this?.SuperView) {
+							// insert m into S
+							noEdgeNodes.Add (m);
+						}
+					}
+				}
+
+				if (edges.Any ()) {
+					(var from, var to) = edges.First ();
+					if (from != Application.Top) {
+						if (!ReferenceEquals (from, to)) {
+							throw new InvalidOperationException ($"TopologicalSort (for Pos/Dim) cannot find {from} linked with {to}. Did you forget to add it to {this}?");
+						} else {
+							throw new InvalidOperationException ("TopologicalSort encountered a recursive cycle in the relative Pos/Dim in the views of " + this);
+						}
+					}
+				}
+				// return L (a topologically sorted order)
+				return result;
+			} // TopologicalSort
+			
 			var ordered = TopologicalSort (nodes, edges);
 
 			foreach (var v in ordered) {
@@ -2471,9 +2459,12 @@ namespace Terminal.Gui {
 				v.LayoutNeeded = false;
 			}
 
+			// If our SuperView is Application.Top and the layoutstyle is Computed it's a special-cass.
+			// Use SetRelativeaLayout with the Frame of the Application.Top
 			if (SuperView != null && SuperView == Application.Top && LayoutNeeded
 			    && ordered.Count == 0 && LayoutStyle == LayoutStyle.Computed) {
-				SetRelativeLayout (SuperView.Frame);
+				Debug.Assert (Application.Top.Frame.Location == Point.Empty);
+				SetRelativeLayout (Application.Top.Frame);
 			}
 
 			LayoutNeeded = false;
@@ -2756,9 +2747,11 @@ namespace Terminal.Gui {
 		}
 
 		/// <summary>
-		/// Gets the size to fit all text if <see cref="AutoSize"/> is true.
+		/// Gets the dimensions required to fit <see cref="Text"/> using the text <see cref="Direction"/> specified by the
+		/// <see cref="TextFormatter"/> property and accounting for any <see cref="HotKeySpecifier"/> characters.
+		/// .
 		/// </summary>
-		/// <returns>The <see cref="Size"/></returns>
+		/// <returns>The <see cref="Size"/> required to fit the text.</returns>
 		public Size GetAutoSize ()
 		{
 			var rect = TextFormatter.CalcRect (Bounds.X, Bounds.Y, TextFormatter.Text, TextFormatter.Direction);
@@ -2793,10 +2786,11 @@ namespace Terminal.Gui {
 		}
 
 		/// <summary>
-		/// Get the width or height of the <see cref="Terminal.Gui.TextFormatter.HotKeySpecifier"/> length.
+		/// Gets the width or height of the <see cref="Terminal.Gui.TextFormatter.HotKeySpecifier"/> characters in the <see cref="Text"/> property.
 		/// </summary>
-		/// <param name="isWidth"><see langword="true"/> if is the width (default) <see langword="false"/> if is the height.</param>
-		/// <returns>The length of the <see cref="Terminal.Gui.TextFormatter.HotKeySpecifier"/>.</returns>
+		/// <param name="isWidth">If <see langword="true"/> (the default) the width required for the hotkey specifier is returned. Otherwise the height is returned.</param>
+		/// <returns>The number of characters required for the <see cref="Terminal.Gui.TextFormatter.HotKeySpecifier"/>. If the text direction specified
+		/// by <see cref="TextDirection"/> does not match the <paramref name="isWidth"/> parameter, <c>0</c> is returned.</returns>
 		public int GetHotKeySpecifierLength (bool isWidth = true)
 		{
 			if (isWidth) {
@@ -2811,7 +2805,7 @@ namespace Terminal.Gui {
 		}
 
 		/// <summary>
-		/// Gets the bounds size from a <see cref="Terminal.Gui.TextFormatter.Size"/>.
+		/// Gets the <see cref="TextFormatter.Size"/> minus the size required for the <see cref="Terminal.Gui.TextFormatter.HotKeySpecifier"/>.
 		/// </summary>
 		/// <returns>The bounds size minus the <see cref="Terminal.Gui.TextFormatter.HotKeySpecifier"/> length.</returns>
 		public Size GetTextFormatterBoundsSize ()

Terminal.Gui/Terminal.Gui.csproj

@@ -56,6 +56,7 @@
   </ItemGroup>
   <PropertyGroup>
     <TargetFrameworks>net472;netstandard2.0;net6.0</TargetFrameworks>
+    <LangVersion>9</LangVersion>
     <RootNamespace>Terminal.Gui</RootNamespace>
     <AssemblyName>Terminal.Gui</AssemblyName>
     <DocumentationFile>bin\Release\Terminal.Gui.xml</DocumentationFile>

UICatalog/Scenarios/ComputedLayout.cs

@@ -19,34 +19,23 @@ namespace UICatalog.Scenarios {
 
 		public override void Setup ()
 		{
-			var menu = new MenuBar (new MenuBarItem [] {
-				new MenuBarItem ("_Settings", new MenuItem [] {
-					null,
-					new MenuItem ("_Quit", "", () => Quit()),
-				}),
-			});
-			Application.Top.Add (menu);
-
 			var statusBar = new StatusBar (new StatusItem [] {
 				new StatusItem(Key.CtrlMask | Key.Q, "~^Q~ Quit", () => Quit()),
 			});
 			Application.Top.Add (statusBar);
 
-			//Top.LayoutStyle = LayoutStyle.Computed;
 			// Demonstrate using Dim to create a horizontal ruler that always measures the parent window's width
-			// BUGBUG: Dim.Fill returns too big a value sometimes.
 			const string rule = "|123456789";
 			var horizontalRuler = new Label ("") {
 				X = 0,
 				Y = 0,
-				Width = Dim.Fill (),  // FIXED: I don't think this should be needed; DimFill() should respect container's frame. X does.
+				Width = Dim.Fill (),
 				ColorScheme = Colors.Error
 			};
 
 			Win.Add (horizontalRuler);
 
 			// Demonstrate using Dim to create a vertical ruler that always measures the parent window's height
-			// TODO: Either build a custom control for this or implement linewrap in Label #352
 			const string vrule = "|\n1\n2\n3\n4\n5\n6\n7\n8\n9\n";
 
 			var verticalRuler = new Label ("") {
@@ -64,16 +53,23 @@ namespace UICatalog.Scenarios {
 
 			Win.Add (verticalRuler);
 
-			// Demonstrate At - Absolute Layout using Pos
-			var absoluteButton = new Button ("Absolute At(2,1)") {
+			// Demonstrate At - Using Pos.At to locate a view in an absolute location
+			var atButton = new Button ("At(2,1)") {
 				X = Pos.At (2),
 				Y = Pos.At (1)
 			};
+			Win.Add (atButton);
+
+			// Throw in a literal absolute - Should funciton identically to above
+			var absoluteButton = new Button ("X = 30, Y = 1") {
+				X = 30,
+				Y = 1
+			};
 			Win.Add (absoluteButton);
 
 			// Demonstrate using Dim to create a window that fills the parent with a margin
 			int margin = 10;
-			var subWin = new Window ($"Centered Sub Window with {margin} character margin") {
+			var subWin = new Window ($"Centered Window with {margin} character margin") {
 				X = Pos.Center (),
 				Y = 2,
 				Width = Dim.Fill (margin),
@@ -113,17 +109,77 @@ namespace UICatalog.Scenarios {
 				X = Pos.Center (),
 				Y = Pos.Percent (50),
 				Width = Dim.Percent (80),
-				Height = Dim.Percent (30),
+				Height = Dim.Percent (10),
 				ColorScheme = Colors.TopLevel,
 			};
-			textView.Text = "This text view should be half-way down the terminal,\n20% of its height, and 80% of its width.";
+			textView.Text = $"This TextView should horizontally & vertically centered and \n10% of the screeen height, and 80% of its width.";
 			Win.Add (textView);
 
+			var oddballButton = new Button ("The Buttons below should be centered") {
+				X = Pos.Center (),
+				Y = Pos.Bottom (textView) + 1
+			};
+			Win.Add (oddballButton);
+
+			#region Issue2358
+			// Demonstrate odd-ball Combine scenarios
+			// Until https://github.com/gui-cs/Terminal.Gui/issues/2358 is fixed these won't work right
+
+			oddballButton = new Button ("Center + 0") {
+				X = Pos.Center () + 0,
+				Y = Pos.Bottom (oddballButton)
+			};
+			Win.Add (oddballButton);
+
+			oddballButton = new Button ("0 + Center") {
+				X = 0 + Pos.Center (),
+				Y = Pos.Bottom (oddballButton)
+			};
+			Win.Add (oddballButton);
+
+			// This demonstrates nonsense: it the same as using Pos.AnchorEnd (100/2=50 + 100/2=50 = 100 - 50)
+			// The `- Pos.Percent(5)` is there so at least something is visible
+			oddballButton = new Button ("Center + Center - Percent(50)") {
+				X = Pos.Center () + Pos.Center () - Pos.Percent(50),
+				Y = Pos.Bottom (oddballButton)
+			};
+			Win.Add (oddballButton);
+
+			// This demonstrates nonsense: it the same as using Pos.AnchorEnd (100/2=50 + 100/2=50 = 100 - 50)
+			// The `- Pos.Percent(5)` is there so at least something is visible
+			oddballButton = new Button ("Percent(50) + Center - Percent(50)") {
+				X = Pos.Percent (50) + Pos.Center () - Pos.Percent (50),
+				Y = Pos.Bottom (oddballButton)
+			};
+			Win.Add (oddballButton);
+
+			// This demonstrates nonsense: it the same as using Pos.AnchorEnd (100/2=50 + 100/2=50 = 100 - 50)
+			// The `- Pos.Percent(5)` is there so at least something is visible
+			oddballButton = new Button ("Center + Percent(50) - Percent(50)") {
+				X = Pos.Center () + Pos.Percent (50) - Pos.Percent (50),
+				Y = Pos.Bottom (oddballButton)
+			};
+			Win.Add (oddballButton);
+
+			#endregion
+			// This demonstrates nonsense: Same as At(0)
+			oddballButton = new Button ("Center - Center - Percent(50)") {
+				X = Pos.Center () + Pos.Center () - Pos.Percent (50),
+				Y = Pos.Bottom (oddballButton)
+			};
+			Win.Add (oddballButton);
+
+			// This demonstrates combining Percents)
+			oddballButton = new Button ("Percent(40) + Percent(10)") {
+				X = Pos.Percent (40) + Pos.Percent(10),
+				Y = Pos.Bottom (oddballButton)
+			};
+			Win.Add (oddballButton);
+
 			// Demonstrate AnchorEnd - Button is anchored to bottom/right
-			var anchorButton = new Button ("Anchor End") {
+			var anchorButton = new Button ("Button using AnchorEnd") {
 				Y = Pos.AnchorEnd () - 1,
 			};
-			// TODO: Use Pos.Width instead of (Right-Left) when implemented (#502)
 			anchorButton.X = Pos.AnchorEnd () - (Pos.Right (anchorButton) - Pos.Left (anchorButton));
 			anchorButton.Clicked += () => {
 				// Ths demonstrates how to have a dynamically sized button
@@ -135,22 +191,31 @@ namespace UICatalog.Scenarios {
 			};
 			Win.Add (anchorButton);
 
+			// Demonstrate AnchorEnd(n) 
+			// This is intentionally convoluted to illustrate potential bugs.
+			var anchorEndLabel1 = new Label ("This Button should be the 2nd to last line (AnchorEnd (2)).") {
+				TextAlignment = Terminal.Gui.TextAlignment.Centered,
+				ColorScheme = Colors.Menu,
+				Width = Dim.Fill (1),
+				X = 1,
+				Y = Pos.AnchorEnd (2)
+			};
+			Win.Add (anchorEndLabel1);
 
-			// Centering multiple controls horizontally. 
+			// Demonstrate DimCombine (via AnchorEnd(n) - 1)
 			// This is intentionally convoluted to illustrate potential bugs.
-			var bottomLabel = new Label ("This should be the 2nd to last line (Bug #xxx).") {
+			var anchorEndLabel2 = new Label ("This Button should be the 3rd to last line (AnchorEnd (2) - 1).") {
 				TextAlignment = Terminal.Gui.TextAlignment.Centered,
 				ColorScheme = Colors.Menu,
-				Width = Dim.Fill (),
-				X = Pos.Center (),
-				Y = Pos.AnchorEnd () - 2 // FIXED: -2 should be two lines above border; but it has to be -4
+				Width = Dim.Fill (1),
+				X = 1,
+				Y = Pos.AnchorEnd (2) - 1 // Pos.Combine
 			};
-			Win.Add (bottomLabel);
+			Win.Add (anchorEndLabel2);
 
-			// Show positioning vertically using Pos.Bottom 
-			// BUGBUG: -1 should be just above border; but it has to be -3
+			// Show positioning vertically using Pos.AnchorEnd via Pos.Combine
 			var leftButton = new Button ("Left") {
-				Y = Pos.AnchorEnd () - 1
+				Y = Pos.AnchorEnd () - 1 // Pos.Combine
 			};
 			leftButton.Clicked += () => {
 				// Ths demonstrates how to have a dynamically sized button
@@ -165,7 +230,7 @@ namespace UICatalog.Scenarios {
 			// show positioning vertically using Pos.AnchorEnd
 			var centerButton = new Button ("Center") {
 				X = Pos.Center (),
-				Y = Pos.AnchorEnd () - 1
+				Y = Pos.AnchorEnd (1)  // Pos.AnchorEnd(1)
 			};
 			centerButton.Clicked += () => {
 				// Ths demonstrates how to have a dynamically sized button
@@ -190,7 +255,6 @@ namespace UICatalog.Scenarios {
 			};
 
 			// Center three buttons with 5 spaces between them
-			// TODO: Use Pos.Width instead of (Right-Left) when implemented (#502)
 			leftButton.X = Pos.Left (centerButton) - (Pos.Right (leftButton) - Pos.Left (leftButton)) - 5;
 			rightButton.X = Pos.Right (centerButton) + 5;
 

docfx/v2specs/View.md

@@ -0,0 +1,176 @@
+# V2 Spec for View refactor
+
+IMPORTANT: I am critical of the existing codebase below. Do not take any of this personally. It is about the code, not the amazing people who wrote the code.
+
+ALSO IMPORTANT: I've written this to encourage and drive DEBATE. My style is to "Have strong opinions, weakly held." If you read something here you don't understand or don't agree with, SAY SO. Tell me why. Take a stand. 
+
+This covers my thinking on how we will refactor `View` and the classes in the `View` heirarchy(inclidng `Responder`). It does not cover 
+  * Text formatting which will be covered in another spec. 
+  * TrueColor support which will be covered separately.
+  * ConsoleDriver refactor.
+
+## What's wrong with the View and the View-class Heirarchy in v1?
+
+
+
+## Terminal.Gui v2 View-related Lexicon & Taxonomy
+
+  * *Responder* - A class that can handle user input. Implemented in the `Responder` base-class. 
+    * We will move all mouse/keyboard base-logic out of `View` and `Window` and into `Responder`.
+
+  * *Container* - A class that can hold other Responders. Implemented in the `Container` base-class. 
+    * We will move all logic for adding/removing views out of `View` and `Window` and into `Container`.
+      * OR, we will move all of this logic to another .cs file as either a helper class or a `partial` of `View.
+    * NOT DONE YET and somewhat confused in my current WIP.
+
+  * *View* - The key base-class for implementing higher level visual/interactive Terminal.Gui elements. Implemented in the `View` base-class, which derives from `Responder` and `Container`. 
+    * We will move all logic for rendering out of `FrameView` & `Window` into `View`.
+    * We will further refactor View to attempt to radically simplify it (and reducee the size of the monsterous View.cs). 
+      * Content (including SubView) Rendering (and the `Redraw` method) will be moved to a new `ViewRenderer` class.
+      * Things outside of the content area (see Adornments below) will be moved to a new `ViewAdornment` class.
+      * `Redraw` will be a simple method calling:
+        * `OnDrawFrames` - cancelable event that allows the View to draw its border, title, and adornments.
+        * `OnDrawSubviews` - cancelable event that allows the View to draw its SubViews.
+        * `OnDrawContent` - cancelable event that allows the View to draw its content.
+        * `OnRerawComplete` - event that allows sublcasses to override behavior after other drawing is complete.
+    * `View.Frames` will be a list of `Frame`-subclasss (see below) will comprise the Margin, Border, Adornments, and Padding. These Frames are not children (not subviews), lie outside of the View's `ContentArea`/`Bounds` and are not clipped by the View's `ClipArea`.
+
+  * *SubView* - A View that is contained in antoher view and will be rendered as part of the containing view's *ContentArea*. SubViews are added to another view via the `View.Add` method. A View may only be a SubView of a single View. 
+  * *SuperView* - The View that a *SubView* was added to. 
+  * *Child View* - A view that is held by another view in a parent/child relationshiop, but is NOT a SubView. Examples of this are sub-menus of `MenuBar`. 
+  * *Parent View* - A view that holds a reference to another view in a parent/child relationship, but is NOT a SuperView of the child. 
+  * *Thickness* - Describes rectangle where each of the four sides can have a width. Valid width values are >= 0. The inner area of a Thickness is the sum of the widths of the four sides minus the size of the rectangle.
+  * *Margin* - Means the Thickness that separtes a View from other SubViews of the same SuperView. The Margin is not part of the View's content and is not clipped by the View's `ClipArea`. 
+    * QUESTION: Will it be possilble to have a negative Margin? If so, will that allow us to have "magic borderframe connections" as I've demonsrated in my TileViewExperiment?
+  * *Title* - Means text that is displayed for the View that describes the View to users. Typically the Title is displayed at the top-left, overlaying the Border. The Title is not part of the View's content and is not clipped by the View's `ClipArea`. 
+  * *Border* - Means the Thickness where a visual border (drawn using line-drawing glyphs) and the Title are drawn. The Border expands inward; in other words if `Border.Thickness.Top == 2` the border & title will take up the first row and the second row will be filled with spaces. The Border is not part of the View's content and is not clipped by the View's `ClipArea`.
+  * *Adornments* (NOT IMPLEMENTED YET)- The Thickness between the Margin and Padding. The Adornments property of `View` is a `View`-subclass that hosts SubViews that are not part of the View's content and are rendered within the Adornment Thickness. Adornments are not part of the View's content and are not clipped by the View's `ClipArea`. Examples of Adornments:
+    * A `TitleBar` renders the View's `Title` and a horizontal line defining the top of the View. Adds thickness to the top of Adornments. 
+    * One or more `LineView`s that render the View's border (NOTE: The magic of `LineCanvas` lets us automatically have the right joins for these and `TitleBar`!).
+    * A `Vertical Scrollbar` adds thickness to `Adornments.Right` (or `.Left` when right-to-left language support is added). 
+    * A `Horizontal Scrollbar` adds thickness to `Adornments.Bottom` when enabled.
+    * A `MenuBar` adds thickness to `Adornments.Top` (NOTE: This is a change from v1 where `subview.Y = 1` is required).
+    * A `StatusBar` adds thickness ot `Adornments.Bottom` and is rendered at the bottom of Padding.
+    * NOTE: The use of `View.Add` in v1 to add adornments to Views is the cause of much code complexity. Changing the API such that `View.Add` is ONLY for subviews and adding a `View.Adornments.Add` API for menu, statusbar, scroll bar... will enable us to signficantly simplify the codebase.
+  * *Padding* - Means the Thickness inside of an element that offsets the `Content` from the Border. (NOTE: in v1 `Padding` is OUTSIDE of the `Border`). Padding is `{0, 0, 0, 0}` by default. Padding is not part of the View's content and is not clipped by the View's `ClipArea`.
+  * *Frame* - Means the `Rect` that defines the location and size of the `View` including all of the margin, border, adornments, padding, and content area. The coordinates are relative to the SuperView of the View (or, in the case of `Application.Top`, `ConsoleDriver.Row == 0; ConsoleDriver.Col == 0`). The Frame's location and size are controlled by either `Absolute` or `Computed` positioning via the `.X`, `.Y`, `.Height`, and `.Width` properties of the View. 
+  * *VisibleArea* - (NOT IMPLEMENTED YET) Means the area inside of the Margin + Border (Title) + Padding. `VisibleArea.Location` is always `{0, 0}`. `VisibleArea.Size` is the `View.Frame.Size` shrunk by Margin + Border + Padding. 
+  * *ContentArea* - (NOT IMPLEMENTED YET; currently `Bounds`) The `Rect` that describes the location and size of the View's content, relative to `VisibleArea`. If `ContentArea.Location` is negative, anything drawn there will be clipped and any subview positioned in the negative area will cause (optional) scrollbars to appear (making the Thickness of Padding thicker on the appropriate sides). If `ContentArea.Size` is changed such that the dimensions fall outside of `Frame.Size shrunk by Margin + Border + Padding`, drawning will be clipped and (optional) scrollbars will appear.
+    * QUESTION: Can we just have one `ContentArea` property that is the `Rect` that describes the location and size of the View's content, relative to `Frame`? If so, we can remove `VisibleArea` and `Bounds` and just have `ContentArea` and `Frame`? The key to answering this is all wrapped up in scrolling and clipping.
+  * *Bounds* - Synomous with *VisibleArea*. (Debate: Do we rename `Bounds` to `VisbleArea` in v2?)
+  * *ClipArea* - Means the currently vislble portion of the *Content*. This is defined as a`Rect` in coordinates relative to *ContentArea* (NOT *VisibleArea*) (e.g. `ClipArea {X = 0, Y = 0} == ContentArea {X = 0, Y = 0}`). This `Rect` is passed to `View.Redraw` (and should be named "clipArea" not "bounds"). It defines the clip-region the caller desires the `Redraw` implementation to clip itself to (see notes on clipping below).
+  * *Modal* - The term used when describing a View that was created using the `Application.Run(view)` or `Application.Run<T>` APIs. When a View is running as a modal, user input is restricted to just that View until `Application.Run` exits. A `Modal` View has its own `RunState`. 
+  * *TopLevel* - The v1 term used to describe a view that is both Modal and can have a MenuBar and/or StatusBar. I propose in v2 we deprecate the term `TopLevel` and instead use `Modal` to describe the same thing. I do not think `Modal` should be a class, but a property of `View` that can be set to `true` or `false`.
+  * *Window* - A View that, by default, has a `Border` and a `Title`. 
+    * QUESTION: Why can't this just be a property on `View` (e.g. `View.Border = true`)? Why do we need a `Window` class at all in v2?
+
+### View classes to be nuked
+* PanelView (done)
+* FrameView (almost done)
+* Window?
+
+
+  ### Questions
+
+
+### Problems with Current Architecture & Implementation
+
+* `Frame`, `Bounds`, and `ClipRect` are confusing and not consistently applied...
+  * `Bounds` is `Rect` but is used to describe a `Size` (e.g. `Bounds.Size` is the size of the `View`'s content area). It literaly is implemented as a property that returns `new Rect(0, 0, Width, Height)`. Throughtout the codebase `bounds` is used for things that have non-zero `Size` (and actually descibe either the cliprect or the Frame).
+  * The restrictive nature of how `Bounds` is defined led to the hacky `FrameView` and `Window` classes with an embedded `ContentView` in order to draw a border around the content. 
+    * The only reason FrameView exists is because the original architecture didn't support offsetting `View.Bounds`  such that a border could be drawn and the interior content would clip correctly. Thus Miguel (or someone) built
+  FrameView with nested `ContentView` that was at `new Rect(+1, +1, -2, -2)`. 
+    * `Border` was added later, but couldn't be retrofitted into `View` such that if `View.Border ~= null` just worked like `FrameView`.
+    * Thus devs are forced to use the clunky `FrameView` instead of just setting `View.Border`.
+  * `Border` has a bunch of confusing concepts that don't match other systems (esp the Web/HTML)
+    * `Margin` on the web means the space between elements - `Border` doesn't have a margin property, but does has the confusing `DrawMarginFrame` property.
+    * `Border` on the web means the space where a border is drawn. The current implementaiton confuses the term `Frame` and `Border`. `BorderThickness` is provided. 
+    * `Padding` on the web means the padding inside of an element between the `Border` and `Content`. In the current implementation `Padding` is actually OUTSIDE of the `Border`. This means it's not possible for a view to offset internally by simply changing `Bounds`. 
+    * `Content` on the web means the area inside of the Margin + Border + Padding. `View` does not currently have a concept of this (but `FrameView` and `Window` do via the embeded `ContentView`s.
+    * `Border` has a `Title` property. So does `Window` and `FrameView`. This is unneeded duplicate code.
+    * It is not possilble for a class drived from View to orverride the drawing of the "Border" (frame, title, padding, etc...). Multiple devs have asked to be able to have the border frame to be drawn with a different color than `View.ColorScheme`. The API should explicitly enable devs to override the drawing of `Border` independently of the `View.Draw` method. See how `WM_NCDRAW` works in wWindows (Draw non-client). It should be easy to do this from within a `View` sub-class (e.g. override `OnDrawBorder`) and externally (e.g. `DrawBorder += () => ...`. 
+
+* `AutoSize` mostly works, but only because of heroic special-casing logic all over the place by @bdisp. This should be massively simplified.
+* `FrameView` is superlufous and should be removed from the heirarchy (instead devs should just be able to manipulate `View.Border` (or similar) to achieve what `FrameView` provides). The internal `FrameView.ContentView` is a bug-farm and un-needed if `View.Border` worked correctly. 
+* `TopLevel` is currently built around several concepts that are muddled:
+  * Views that host a Menu and StatusBar. It is not clear why this is and if it's needed as a concept. 
+  * Views that can be run via `Application.Run<TopLevel>` (need a separate `RunState`). It is not clear why ANY VIEW can't be run this way, but it seems to be a limitation of the current implementation.
+  * Views that can be used as a pop-up (modal) (e.g. `Dialog`). As proven by `Wizard`, it is possible to build a View that works well both ways. But it's way too hard to do this today.
+  * Views that can be moved by the user must inherit from `Window` today. It should be possilbe to enable moving of any View (e.g. `View.CanMove = true`).
+* The `MdiContainer` stuff is complex, perhaps overly so, and is not actually used by anyone outside of the project. It's also mis-named because Terminal.Gui doesn't actually support "documents" nor does it have a full "MDI" system like Windows (did). It seems to represent features useful in overlapping Views, but it is super confusing on how this works, and the naming doesn't help. This all can be refactored to support specific scenarios and thus be simplified.
+* There is no facility for users' resizing of Views. @tznind's awesome work on `LineCanvas` and `TileView` combined with @tig's experiments show it could be done in a great way for both modal (overlapping) and tiled Views. 
+* `DrawFrame` and `DrawTitle` are implemented in `ConsoleDriver` and can be replaced by a combination of `LineCanvas` and `Border`.
+* Colors - 
+  * As noted above each of Margin, Border, Padding, and Content should support independent colors.
+  * Many View sub-classes bastardize the exiting ColorSchemes to get look/feel that works (e.g. `TextView` and `Wizard`). Separately we should revamp ColorSchemes to enable more scenarios. 
+  * TrueColor support is needed and should be the default.
+* `Responder` is supposed to be where all common, non-visual-related, code goes. We should ensure this is the case.
+* `View` should have default support for scroll bars. e.g. assume in the new world `View.ContentBounds` is the clip area (defined by `VIew.Frame` minus `Margin` + `Border` + `Padding`) then if any view is added with `View.Add` that has Frame coordinates outside of `ContentBounds` the appropriate scroll bars show up automatgically (optioally of course). Without any code, scrolling just works. 
+* We have many requests to support non-full-screen apps. We need to ensure the `View` class heirachy suppports this in a simple, understandable way. In a world with non-full-screen (where screen is defined as the visible terminal view) apps, the idea that `Frame` is "screen relative" is broken. Although we COULD just define "screen" as "the area that bounds the Terminal.GUI app.".  
+
+## Thoughts on Built-in Views
+* `LineView` can be replaced by `LineCanvas`?
+* `Button` and `Label` can be merged. 
+* `StatusBar` and `Menu` could be combined. If not, then at least made more consistent (e.g. in how hotkeys are specified).
+
+## Design
+
+* `Responder`("Responder base class implemented by objects that want to participate on keyboard and mouse input.") remains mostly unchanged, with minor changes:
+   * Methods that take `View` parametsrs (e.g. `OnEnter`) change to take `Responder` (bad OO design).
+   * Nuke `IsOverriden` (bad OO design)
+   * Move `View.Data` to `Responder` (primitive)
+   * Move `Command` and `KeyBinding` stuff from `View`.
+   * Move generic mouse and keyboard stuff from `View` (e.g. `WantMousePositionReports`)
+
+
+## Example of creating Adornments
+```cs
+// ends up looking just like the v1 default Window with a menu & status bar
+// and a vertical scrollbar. In v2 the Window class would do all of this automatically.
+var top = new TitleBar() {
+    X = 0, Y = 0,
+    Width = Dim.Fill(),
+    Height = 1
+    LineStyle = LineStyle.Single
+};
+var left = new LineView() {
+    X = 0, Y = 0,
+    Width = 1,
+    Height = Dim.Fill(),
+    LineStyle = LineStyle.Single
+};
+var right = new LineView() {
+    X = Pos.AnchorEnd(), Y = 0,
+    Width = 1,
+    Height = Dim.Fill(),
+    LineStyle = LineStyle.Single
+};
+var bottom = new LineView() {
+    X = 0, Y = Pos.AnchorEnd(),
+    Width = Dim.Fill(),
+    Height = 1,
+    LineStyle = LineStyle.Single
+};
+
+var menu = new MenuBar() { 
+    X = Pos.Right(left), Y = Pos.Bottom(top)
+};
+var status = new StatusBar () {
+    X = Pos.Right(left), Y = Pos.Top(bottom)
+};
+var vscroll = new ScrollBarView () {
+    X = Pos.Left(right),
+    Y = Dim.Fill(2) // for menu & status bar
+};
+
+Adornments.Add(titleBar);
+Adornments.Add(left);
+Adornments.Add(right);
+Adornments.Add(bottom);
+Adornments.Add(vscroll);
+
+var treeView = new TreeView () {
+    X = 0, Y = 0, Width = Dim.Fill(), Height = Dim.Fill()
+};
+Add (treeView);
+```