Finish Adornments (master Issue)

[tig]

Todo

• Fix focus and keyboard nav with Adornments - In progress: Partially Adresses #2491. Refactors how Focus works #3627
• Auto-join of Border is not always working #2995
• Create new ScrollBar based on a new Scroll and remove ScrollBarView/ScrollView #2489
• Refactor Border to use subviews for title, lines, and close button #3407
• Get rid of Toplevel - Introduce Runnable and Overlapped instead #2491
• Finish implementing Line and remove LineView #3014
• Add mouse/keyboard resizing of views #2537
  • See Arrangement enum
  • Drag to move is implemented (for mouse)
  • Resize is undone (for either mouse or keyboard)
• Simplify existing Views by taking advantage of Adornment
  • Border - Use subviews instead of bespoke OnDrawContent for lines and title.
  • Dialog (and MessageBox) - Put buttons in bottom Padding
  • Wizard - Put nav buttons in bottom Padding. Put Help in right Padding
  • Toplevel - Put MenuBarin topPaddingandStatusBarin bottomPadding` - See Refactor MenuBar and StatusBar to be adornments #2488
  • ScrollBarView - Update any view that uses it (orScrollView to use new ScrollBars in Padding and built-in content scrolling.

Related Todos

• Adornments: Enable SubViews in Margin, Border, and Padding #2563
• Create new ScrollBar based on a new Scroll and remove ScrollBarView/ScrollView #2489
• EnsureVisibleBounds is complex & confusing #2407

Background

When I was designing Frames I had the idea that View's Margin, Border, and Padding could be replaced by a View. I didn't complete the implementation of this, but did make View.CreateFrames virtual:

/// <summary>
/// Creates the view's <see cref="Frame"/> objects. This internal method is overridden by Frame to do nothing
/// to prevent recursion during View construction.
/// </summary>
internal virtual void CreateFrames ()
{
	void ThicknessChangedHandler (object sender, EventArgs e)
	{
		LayoutFrames ();
		SetNeedsLayout ();
		SetNeedsDisplay ();
	}

	if (Margin != null) {
		Margin.ThicknessChanged -= ThicknessChangedHandler;
		Margin.Dispose ();
	}
	Margin = new Frame () { Id = "Margin", Thickness = new Thickness (0) };
	Margin.ThicknessChanged += ThicknessChangedHandler;
	Margin.Parent = this;

	if (Border != null) {
		Border.ThicknessChanged -= ThicknessChangedHandler;
		Border.Dispose ();
	}
	Border = new Frame () { Id = "Border", Thickness = new Thickness (0) };
	Border.ThicknessChanged += ThicknessChangedHandler;
	Border.Parent = this;

	// TODO: Create View.AddAdornment

	if (Padding != null) {
		Padding.ThicknessChanged -= ThicknessChangedHandler;
		Padding.Dispose ();
	}
	Padding = new Frame () { Id = "Padding", Thickness = new Thickness (0) };
	Padding.ThicknessChanged += ThicknessChangedHandler;
	Padding.Parent = this;
}

In addition, I left unfinished how the Frames are drawn. I used a hack of setting Frame.Id to "Border" etc...:

// TODO: v2 - this will eventually be two controls: "BorderView" and "Label" (for the title)

if (Id == "Border" && canDrawBorder && Thickness.Top > 0 && maxTitleWidth > 0 && !string.IsNullOrEmpty (Parent?.Title)) {
	var prevAttr = Driver.GetAttribute ();
	if (ColorScheme != null) {
		Driver.SetAttribute (HasFocus ? GetHotNormalColor () : GetNormalColor ());
	} else {
		Driver.SetAttribute (Parent.HasFocus ? Parent.GetHotNormalColor () : Parent.GetNormalColor ());
	}
	DrawTitle (new Rect (borderBounds.X, titleY, maxTitleWidth, 1), Parent?.Title);
	Driver.SetAttribute (prevAttr);
}

If we were to complete this idea ("Adornments"), we would have 3 built-in derivations of Frame:

• MarginView
• BorderView
• PaddingView

Then, a view like TabView could replace any of these. In TabView's case TabRowView would derive from Frame (and be named TabFrame).

I have an open Issue for this already: #2563

Note that in it, I call out that the problem of how Mouse/Keyboard events are handled correctly. In the current v2_develop, Frame can never get focus. But for all this to work, we have to change that.

Note that by doing this it we also can remove TabView.ContentView which is a hack that was required in v1... and one of the motivations I had for implementing Frames.

This same solution, BTW, will also simplify Dialog and Wizard (the button/nav bar just becomes a set of controls in a custom Border.

@BDisp, what would you think of attacking solving all of this such that TabView is a great example of how to replace View.Border with an advanced implementation that provides all of the Tab navigation etc...?

Related (you'd be taking on enabling all of these Issues to be addressed):

• Adornments: Enable SubViews in Margin, Border, and Padding #2563
• Refactor MenuBar and StatusBar to be adornments #2488
• Create new ScrollBar based on a new Scroll and remove ScrollBarView/ScrollView #2489
• EnsureVisibleBounds is complex & confusing #2407
• Enable Close/Quit/Collapse/Expand buttons on Border #2814
• Get rid of Toplevel - Introduce Runnable and Overlapped instead #2491
• Finish implementing Line and remove LineView #3014
• Add mouse/keyboard resizing of views #2537

Extra Credit:

Figure out the best way to satisfy this requirement:

• A View where Border (or Margin or Padding) has been overridden, can still have a "standard" border with a Title, frame, and (eventually standard things like close buttons (Adornments: Enable SubViews in Margin, Border, and Padding #2563).

There are two potential solutions I can think of:

1. Simply force implementers to wrap such a view within a View
2. Allow implementers to add ADDITIONAL adornments (Frames)

I'm not sure of the best approach, but I suspect 1) is simpler and more obvious.

What do you think???

Originally posted by @tig in #2980 (comment)

[tig]

@BDisp which part of this do you feel like tackling first?

If it were me, I'd do it in this order (each as a separate PR):

1. Implement class Margin : Frame, class Border : Frame, class Padding: Frame, getting rid of the Id == "Border" hacks.
2. Auto-join of Border is not always working #2995 - Figure out why auto-join is not always working is essential.
3. Enable Close/Quit/Collapse/Expand buttons on Border #2814 - Use this to flush out how Mouse/Keyboard/Focus work for the current Border implementation
4. Re-factor TabView to use all of this.
5. Create new ScrollBar based on a new Scroll and remove ScrollBarView/ScrollView #2489
6. ...

But I'm happy to let you approach it how you see fit.

[BDisp]

My doubt is if you add views or adornments on the Margin, Border and Padding is if the LineCanvas should render all of them. There will be situations where you don't need/want to do that. For now I'm only think on them to use colors and on the Border frame to draw border. My head is empty for now :-)

[tig]

My doubt is if you add views or adornments on the Margin, Border and Padding is if the LineCanvas should render all of them. There will be situations where you don't need/want to do that. For now I'm only think on them to use colors and on the Border frame to draw border. My head is empty for now :-)

I think your thinking is spot on. We have to figure out how to enable auto-join of lines and ALSO let arbitrary views in Frames (adornments) render appropriately. This will be tricky and will require deep thought and experimentation. I was going down this path with the View Experiments scenario...

Personally, i'm excited about the challenge of working on this with you ;-)

[tig]

deleted

[BDisp]

A thought: When we're done with all of this, instead of having 3 adornments as individual members of View:

I think that the 3 adornments should be primitive on View because they are common to other API's.

We would have a List member:

If a user want more adornments he can create new Frame then a List<Frame> would be nice and thus we use the both code in the same OnDrawFrames

		// TODO: Make this cancelable
		/// <summary>
		/// Prepares <see cref="View.LineCanvas"/>. If <see cref="SuperViewRendersLineCanvas"/> is true, only the <see cref="LineCanvas"/> of 
		/// this view's subviews will be rendered. If <see cref="SuperViewRendersLineCanvas"/> is false (the default), this 
		/// method will cause the <see cref="LineCanvas"/> be prepared to be rendered.
		/// </summary>
		/// <returns></returns>
		public virtual bool OnDrawFrames ()
		{
			if (!IsInitialized) {
				return false;
			}

			// Each of these renders lines to either this View's LineCanvas 
			// Those lines will be finally rendered in OnRenderLineCanvas
			Margin?.OnDrawContent (Margin.Bounds);
			Border?.OnDrawContent (Border.Bounds);
			Padding?.OnDrawContent (Padding.Bounds);

			// Each of these renders lines to either this View's LineCanvas 
			// Those lines will be finally rendered in OnRenderLineCanvas
                       foreach (var frame in Frames) {
                            frame.OnDrawContent (frame.Bounds);
                        }

			return true;
		}

[tig]

A thought: When we're done with all of this, instead of having 3 adornments as individual members of View:

I think that the 3 adornments should be primitive on View because they are common to other API's.

We would have a List member:

If a user want more adornments he can create new Frame then a List<Frame> would be nice and thus we use the both code in the same OnDrawFrames

		// TODO: Make this cancelable
		/// <summary>
		/// Prepares <see cref="View.LineCanvas"/>. If <see cref="SuperViewRendersLineCanvas"/> is true, only the <see cref="LineCanvas"/> of 
		/// this view's subviews will be rendered. If <see cref="SuperViewRendersLineCanvas"/> is false (the default), this 
		/// method will cause the <see cref="LineCanvas"/> be prepared to be rendered.
		/// </summary>
		/// <returns></returns>
		public virtual bool OnDrawFrames ()
		{
			if (!IsInitialized) {
				return false;
			}

			// Each of these renders lines to either this View's LineCanvas 
			// Those lines will be finally rendered in OnRenderLineCanvas
			Margin?.OnDrawContent (Margin.Bounds);
			Border?.OnDrawContent (Border.Bounds);
			Padding?.OnDrawContent (Padding.Bounds);

			// Each of these renders lines to either this View's LineCanvas 
			// Those lines will be finally rendered in OnRenderLineCanvas
                       foreach (var frame in Frames) {
                            frame.OnDrawContent (frame.Bounds);
                        }

			return true;
		}

Thinking about this more, I may be overcomplicating things. If we support replaceability of the 3 built-ins, the need for adding additional is mitigated. In addition, I'm now worried about ordering. There is a clear ordering for "Margin->Border->Padding" that can be seen in the current (hacky) code in Frame.OnDrawContent: Padding gets drawn first, then Border, then Margin. Likewise, Viwe.LayoutFrames:

/// <summary>
/// Overriden by <see cref="Frame"/> to do nothing, as the <see cref="Frame"/> does not have frames.
/// </summary>
internal virtual void LayoutFrames ()
{
	if (Margin == null) return; // CreateFrames() has not been called yet

	if (Margin.Frame.Size != Frame.Size) {
		Margin._frame = new Rect (Point.Empty, Frame.Size);
		Margin.X = 0;
		Margin.Y = 0;
		Margin.Width = Frame.Size.Width;
		Margin.Height = Frame.Size.Height;
		Margin.SetNeedsLayout ();
		Margin.LayoutSubviews ();
		Margin.SetNeedsDisplay ();
	}

	var border = Margin.Thickness.GetInside (Margin.Frame);
	if (border != Border.Frame) {
		Border._frame = new Rect (new Point (border.Location.X, border.Location.Y), border.Size);
		Border.X = border.Location.X;
		Border.Y = border.Location.Y;
		Border.Width = border.Size.Width;
		Border.Height = border.Size.Height;
		Border.SetNeedsLayout ();
		Border.LayoutSubviews ();
		Border.SetNeedsDisplay ();
	}

	var padding = Border.Thickness.GetInside (Border.Frame);
	if (padding != Padding.Frame) {
		Padding._frame = new Rect (new Point (padding.Location.X, padding.Location.Y), padding.Size);
		Padding.X = padding.Location.X;
		Padding.Y = padding.Location.Y;
		Padding.Width = padding.Size.Width;
		Padding.Height = padding.Size.Height;
		Padding.SetNeedsLayout ();
		Padding.LayoutSubviews ();
		Padding.SetNeedsDisplay ();
	}
}

Therefore, I retract my previous assertion that we should have a List<Frames>.

[BDisp]

Therefore, I retract my previous assertion that we should have a List.

Unless you can remove the virtual keyword and thus you make the current sort order first. But I think this will complicate much more.

[tig]

@BDisp I'm wondering if you've made progress on:

I'm working on some stuff that needs that done. I'd be happy to jump on it if you're on other things..

[BDisp]

@BDisp I'm wondering if you've made progress on:

I'm working on some stuff that needs that done. I'd be happy to jump on it if you're on other things..

Do you already have any idea about this? I'm thinking that instead of using the Id maybe we can create new classes Margin, Border and Padding derived from Frame and use that new classes as properties in the View class. all the common code can reside in the Frame or in the View. All the code that must be specific to each one can be handle with virtual methods on the Frame that can be override by Margin, Border and Padding. Or even by a IFrame interface with all the additional properties, methods needed. What do you think?

In this case we refer to them as if variable is Margin, variable is Border and variable is Padding.

[tig]

Do you already have any idea about this? I'm thinking that instead of using the Id maybe we can create new classes Margin, Border and Padding derived from Frame and use that new classes as properties in the View class. all the common code can reside in the Frame or in the View. All the code that must be specific to each one can be handle with virtual methods on the Frame that can be override by Margin, Border and Padding. Or even by a IFrame interface with all the additional properties, methods needed. What do you think?

In this case we refer to them as if variable is Margin, variable is Border and variable is Padding.

See #2563.

I considered (and wrote about) 3 sub-classes above. However, I now think just having public Frame Margin { get; private set; } is fine, but CreateFrames should do something like this:

To get there, the first step is to move the OnDrawContent code out of Frame and do this:

Where Border_DrawContent would basically be the code currently in Frame.OnDrawContent.

[tig]

In, otherwords:

1. Short term: Move the hacky manual draw code from Frame.OnDrawContent to Border_DrawContent and delete Frame.OnDrawContent, just letting the base do it's thing.

2. Long term (once other Adornments issues like focus handling are sorted), delete Border_DrawContent and replace what it does with the right subviews.

[BDisp]

@tig how you type ?

[tig]

@tig how you type ?

Use a font that supports ligatures and it just does it automatically. e.g

[BDisp]

@tig how you type ?

Use a font that supports ligatures and it just does it automatically. e.g

Thanks. Cascadia Code also supports it.

[tig]

After sleeping on it, I'm back on the "have Border be a subclass of Frame" bandwagon.

Just Border and not Margin or Padding. Why?

We want helpers for common things like BorderStyle = LineStyle.Single. Margin and Padding will not have a border.

@BDisp if you're ok with this I'm going to start a PR that just refactors Border. Ok?

[BDisp]

After sleeping on it, I'm back on the "have Border be a subclass of Frame" bandwagon.

Just Border and not Margin or Padding. Why?

We want helpers for common things like BorderStyle = LineStyle.Single. Margin and Padding will not have a border.

@BDisp if you're ok with this I'm going to start a PR that just refactors Border. Ok?

Because of the difference between them I proposed the sub-class. I'm glad you accept my opinion.