Finish Adornments (master Issue) #2994

tig · 2023-11-15T15:21:13Z

Todo

Related Todos

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):

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:

Simply force implementers to wrap such a view within a View
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)

The text was updated successfully, but these errors were encountered:

tig · 2023-11-15T15:49:01Z

@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):

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

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

BDisp · 2023-11-15T16:11:26Z

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 · 2023-11-16T13:13:04Z

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 · 2023-11-16T13:15:21Z

deleted

BDisp · 2023-11-16T13:48:06Z

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 · 2023-11-16T14:10:02Z

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 · 2023-11-16T14:15:35Z

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 · 2024-01-10T21:07:29Z

@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 · 2024-01-10T21:33:35Z

@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 · 2024-01-10T21:47:19Z

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 · 2024-01-10T21:55:40Z

In, otherwords:

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.
Long term (once other Adornments issues like focus handling are sorted), delete Border_DrawContent and replace what it does with the right subviews.

BDisp · 2024-01-10T23:03:51Z

@tig how you type ?

tig · 2024-01-10T23:39:40Z

@tig how you type ?

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

BDisp · 2024-01-11T13:40:04Z

@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 · 2024-01-11T19:50:48Z

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 · 2024-01-11T20:06:12Z

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.

tig added the v2 For discussions, issues, etc... relavant for v2 label Nov 15, 2023

This was referenced Nov 15, 2023

Fixes #2882. TabView: 'Frame.DrawFrame(Rect, bool)' is obsolete: 'This method is obsolete in v2. Use use LineCanvas or Frame #2980

Merged

Auto-join of Border is not always working #2995

Open

tig changed the title ~~Enable Adornments (Margin, Border, Padding) to be overridden~~ Finish Adornments (master Issue) Nov 15, 2023

This was referenced Nov 25, 2023

Fixes #2578 - Updates mouse events to be relative to View.Bounds #2920

Merged

Add mouse/keyboard resizing of views #2537

Closed

View need a alternative DrawFrame for v2. #2983

Closed

tig mentioned this issue Jan 11, 2024

Renames Frame->Adornment; changes Frame to have a Border subclass #3158

Merged

9 tasks

This was referenced Mar 4, 2024

Finish implementing Line and remove LineView #3014

Open

Enable Close/Quit/Collapse/Expand buttons on Border #2814

Closed

Fixes #3273 (Again). Updates FindDeepestView etc... to finish Adornment #3295

Merged

tig mentioned this issue Apr 12, 2024

Fixes #3169. Bounds -> Viewport: Content Scrolling in View #3323

Merged

10 tasks

tig added this to the V2 Beta milestone May 25, 2024

tig self-assigned this May 25, 2024

This was referenced Aug 13, 2024

TGD: Master Issue tracking TG issues that must be fixed in v2 #3650

Open

Refactor Border to use subviews for title, lines, and close button #3407

Open

tig mentioned this issue Aug 30, 2024

Adornments = too many Views #3706

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Finish Adornments (master Issue) #2994

Finish Adornments (master Issue) #2994

tig commented Nov 15, 2023 •

edited

Loading

tig commented Nov 15, 2023

BDisp commented Nov 15, 2023

tig commented Nov 16, 2023

tig commented Nov 16, 2023 •

edited

Loading

BDisp commented Nov 16, 2023

tig commented Nov 16, 2023

BDisp commented Nov 16, 2023

tig commented Jan 10, 2024 •

edited

Loading

BDisp commented Jan 10, 2024 •

edited

Loading

tig commented Jan 10, 2024

tig commented Jan 10, 2024

BDisp commented Jan 10, 2024

tig commented Jan 10, 2024

BDisp commented Jan 11, 2024

tig commented Jan 11, 2024

BDisp commented Jan 11, 2024

Finish Adornments (master Issue) #2994

Finish Adornments (master Issue) #2994

Comments

tig commented Nov 15, 2023 • edited Loading

Todo

Related Todos

Background

tig commented Nov 15, 2023

BDisp commented Nov 15, 2023

tig commented Nov 16, 2023

tig commented Nov 16, 2023 • edited Loading

BDisp commented Nov 16, 2023

tig commented Nov 16, 2023

BDisp commented Nov 16, 2023

tig commented Jan 10, 2024 • edited Loading

BDisp commented Jan 10, 2024 • edited Loading

tig commented Jan 10, 2024

tig commented Jan 10, 2024

BDisp commented Jan 10, 2024

tig commented Jan 10, 2024

BDisp commented Jan 11, 2024

tig commented Jan 11, 2024

BDisp commented Jan 11, 2024

tig commented Nov 15, 2023 •

edited

Loading

tig commented Nov 16, 2023 •

edited

Loading

tig commented Jan 10, 2024 •

edited

Loading

BDisp commented Jan 10, 2024 •

edited

Loading