Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Pos.AnchorEnd () (no param) to automatically account for the view's dimension #481

Closed
tig opened this issue May 21, 2020 · 14 comments
Closed
Labels
design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...)

Comments

@tig
Copy link
Collaborator

tig commented May 21, 2020

Today:

  • Serious lack of good AnchorEnd unit tests
  • Way to hard to use AnchorEnd (x) for the most common use-cases
var btn = new Button () {
	Text = "Ok",
	X = Pos.AnchorEnd (margin: 1),
	Y = Pos.Bottom (margin: 1)
};
image

Note, if margin: was 0, or omitted, nothing would be shown. (Which just illustrates my point; the API is surpising)

var btn = new Button () {
	Text = "Ok",
	Y = Pos.Bottom (margin: 1)
};
btn.X = Pos.AnchorEnd ()  -  (Pos.Right(btn) - Pos.Left(btn));
image

With proposed change:

var btn = new Button () {
	Text = "Ok",
	X = Pos.AnchorEnd (),
	Y = Pos.Bottom ()
};
image

Additionally, margin: is the wrong term for both today's behavior and my proposed behavior. What AnchorEnd really does when a non-zero margin is specified is to shift the position left/up. So it's not really margin, but shift or offset.

Updated API docs:

	/// <summary>
	/// Creates a <see cref="Pos"/> object that hast it's end (right side or bottom) anchored to the end (right side or bottom)
	/// of the SuperView, useful to flush the layout from the right or bottom.
	/// </summary>
	/// <returns>The <see cref="Pos"/> object anchored to the end (the bottom or the right side).</returns>
	/// <example>
	/// This sample shows how align a <see cref="Button"/> to the bottom-right the SuperView.
	/// <code>
	/// // See Issue #502 
	/// anchorButton.X = Pos.AnchorEnd ();
	/// anchorButton.Y = Pos.AnchorEnd ();
	/// </code>
	/// </example>
	public static Pos AnchorEnd ()
	{

	}

	/// <summary>
	/// Creates a <see cref="Pos"/> object that is anchored to the end (right side or bottom) of the SuperView, 
	/// useful to flush the layout from the right or bottom.
	/// </summary>
	/// <returns>The <see cref="Pos"/> object anchored to the end (the bottom or the right side).</returns>
	/// <param name="offset">The view will be shifted left or up by the amount specified.</param>
	/// <example>
	/// This sample shows how align a <see cref="Button"/> such that to it's left side is offset 10 columns from the right
	/// the right edge of the SuperView.
	/// <code>
	/// // See Issue #502 
	/// anchorButton.X = Pos.AnchorEnd (10);
	/// anchorButton.Y = 1
	/// </code>
	/// </example>
	public static Pos AnchorEnd (int offset)
@tig tig added the question label May 21, 2020
@BDisp
Copy link
Collaborator

BDisp commented May 21, 2020

It's more or less equal to the Dim.Width(view) - n

@tig
Copy link
Collaborator Author

tig commented May 24, 2020

New docs make it more clear.

https://migueldeicaza.github.io/gui.cs/api/Terminal.Gui/Terminal.Gui.Pos.html#Terminal_Gui_Pos_AnchorEnd_System_Int32_

Creates a Pos object that is anchored to the end (right side or bottom) of the dimension, useful to flush the layout from the right or bottom.

Closing.

@tig tig closed this as completed May 24, 2020
@tig
Copy link
Collaborator Author

tig commented Dec 22, 2023

Re-opening.

AnchorEnd functions well, but if you study the uses of it, in almost all cases one of two things happen

  1. The caller specifies AnchorEnd(dim of view). Eg AnchorEnd(1) to anchor a button to the bottom
  2. convoluted logic calculates the dim of the view. E.g view.X = Pos.AnchorEnd() -(Pos.Right(view) - Pos.Left(view))

I propose in v2 we change this such that there are two constructors:

Pos AnchorEnd ()

And

Pos AnchorEnd (int margin)

If the 1st is used, we apply (Pos.Right(view) - Pos.Left(view)).

If the second is used, margin is applied as per the current implementation.

What do yall think?

@tig tig reopened this Dec 22, 2023
@BDisp
Copy link
Collaborator

BDisp commented Dec 22, 2023

The more options the better. Can you explain the differences between them in an image, please? In principle, it seems to make managing Pos.Absolute even more difficult because instead of assigning a final positioning value, we will still have to consider the margin. But it may have other advantages that I am not reaching for now.

@tig tig changed the title How is Pos.AnchorEnd supposed to work? Make AnchorEnd automatically account for views dimension Dec 22, 2023
@tig
Copy link
Collaborator Author

tig commented Dec 22, 2023

Today:

var btn = new Button () {
	Text = "Ok",
	X = Pos.AnchorEnd (margin: 1),
	Y = Pos.Bottom (margin: 1)
};
image

Note, if margin: was 0, or omitted, nothing would be shown. (Which just illustrates my point; the API is surpising)

var btn = new Button () {
	Text = "Ok",
	Y = Pos.Bottom (margin: 1)
};
btn.X = Pos.AnchorEnd ()  -  (Pos.Right(btn) - Pos.Left(btn));
image

With proposed change:

var btn = new Button () {
	Text = "Ok",
	X = Pos.AnchorEnd (),
	Y = Pos.Bottom ()
};
image

Note, additionally, margin: is the wrong term for both today's behavior and my proposed behavior. What AnchorEnd really does when a non-zero margin is specified is to shift the position left/up. So it's not really margin, but shift or offset. So I propose to change that too.

Proposed API docs:

	/// <summary>
	/// Creates a <see cref="Pos"/> object that hast it's end (right side or bottom) anchored to the end (right side or bottom)
	/// of the SuperView, useful to flush the layout from the right or bottom.
	/// </summary>
	/// <returns>The <see cref="Pos"/> object anchored to the end (the bottom or the right side).</returns>
	/// <example>
	/// This sample shows how align a <see cref="Button"/> to the bottom-right the SuperView.
	/// <code>
	/// // See Issue #502 
	/// anchorButton.X = Pos.AnchorEnd ();
	/// anchorButton.Y = Pos.AnchorEnd ();
	/// </code>
	/// </example>
	public static Pos AnchorEnd ()
	{

	}

	/// <summary>
	/// Creates a <see cref="Pos"/> object that is anchored to the end (right side or bottom) of the SuperView, 
	/// useful to flush the layout from the right or bottom.
	/// </summary>
	/// <returns>The <see cref="Pos"/> object anchored to the end (the bottom or the right side).</returns>
	/// <param name="offset">The view will be shifted left or up by the amount specified.</param>
	/// <example>
	/// This sample shows how align a <see cref="Button"/> such that to it's left side is offset 10 columns from the right
	/// the right edge of the SuperView.
	/// <code>
	/// // See Issue #502 
	/// anchorButton.X = Pos.AnchorEnd (10);
	/// anchorButton.Y = 1
	/// </code>
	/// </example>
	public static Pos AnchorEnd (int offset)

After drafting this, I have convinced myself that being able to specify a offset is not really useful. I can't come up with a use-case where someone would want to do that. And even if someone does, they could use btn.Margin.Thicknesss = new Thickness(0, 0, 10, 0) (which V1 didn't enable).

@tig
Copy link
Collaborator Author

tig commented Dec 22, 2023

The more options the better. Can you explain the differences between them in an image, please? In principle, it seems to make managing Pos.Absolute even more difficult because instead of assigning a final positioning value, we will still have to consider the margin. But it may have other advantages that I am not reaching for now.

I had a typo originally where I wrote Aboslute; that confused you. Sorry.

@tig tig added design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) and removed question labels Dec 22, 2023
@BDisp
Copy link
Collaborator

BDisp commented Dec 22, 2023

Ok now I understood. Go ahead. Goo work.

@Nutzzz
Copy link

Nutzzz commented Dec 22, 2023

After drafting this, I have convinced myself that being able to specify a offset is not really useful. I can't come up with a use-case where someone would want to do that. And even if someone does, they could use btn.Margin.Thicknesss = new Thickness(0, 0, 10, 0) (which V1 didn't enable).

Devil's advocate: I can imagine a case where you want, say, a text field to fill out the horizontal width of a dynamically-sized view, but also want a button to be anchored to the bottom right of the view, but offset it with more space than the view's margin. Admittedly you could do this by adding a subview for the buttons and give that margins, but especially if there's just one button that seems like overkill.

@BDisp
Copy link
Collaborator

BDisp commented Dec 22, 2023

After drafting this, I have convinced myself that being able to specify a offset is not really useful. I can't come up with a use-case where someone would want to do that. And even if someone does, they could use btn.Margin.Thicknesss = new Thickness(0, 0, 10, 0) (which V1 didn't enable).

The user may not want to touch the margin thickness addressing this and the offset will be the better choice. I know what I'm talk about after dealing with the new TabView look.

@BDisp
Copy link
Collaborator

BDisp commented Feb 19, 2024

@tig something is wrong with the AnchorEnd. I think it's also related with the public bool Contains (int x, int y) { return x >= Left && x < Right && y >= Top && y < Bottom; } of the Rect method.
See that the ScrollView Width should be 50 and it's showing 49 and the Height should be 20 and it's showing 19.

image

Changing the Rect method to public bool Contains (int x, int y) { return x >= Left && x <= Right && y >= Top && y <= Bottom; } the size is showing right. Only the scroll bars is wrong because they use the AnchorEnd(1).

image

Changing it to AnchorEnd() the scroll bars are in the right position but the Dim.Fill(1) isn't right.

image

The change I made to the Contains method of the Rect also affect the button click on a Dialog, but not on the Press me button in the scroll view. I've to click bellow the button to close the Dialog. I think I found some bugs here because them are affecting on my work in the built-in scroll bars. I'll investigate this even more but any help is welcome. Thanks.

@BDisp
Copy link
Collaborator

BDisp commented Feb 19, 2024

Ops sorry @tig I was wrong I forgot to take on account of the 0 number which already count 1 unit, My heads hurts and will explode ;-).

@BDisp
Copy link
Collaborator

BDisp commented Feb 19, 2024

But anyway something is wrong with the AnchorEnd. On the ScrollView I've to use AnchorEnd (1) for the scroll bars to fit and in the Padding I only need to use AnchorEnd (), without dimension. I think it should be the same on both.

@tig
Copy link
Collaborator Author

tig commented Feb 20, 2024

Well, the AnchorEnd unit tests are pretty lame, so it's not surprising to me that there's a nuanced bug or two in there (I'm suspect of any unit test that relies on AutoInitShutdown and/or AssertDriverContents....

I don't fully understand the scenario you're describing. My suggestion is for you to boil it down to the most simple and focused set of unit tests you can come up with. Use only View with minimal subviews and see if you can reproduce the bug.

@tig
Copy link
Collaborator Author

tig commented Apr 15, 2024

Another great example of why we should fix this:
> May you can doing this change in this PR in the Color Picker scenario.

        // Background ColorPicker.
        backgroundColorPicker = new ColorPicker
        {
            Title = "Background Color",
            X = Pos.AnchorEnd ((8 * 4) + 2), // 8 box * 4 width + 2 for border
            BoxHeight = 1,
            BoxWidth = 4,
            BorderStyle = LineStyle.Single
        };

image

Haha. Everytime I run that I'm like "I should just fix this now". Then I'm like "Oh, Rabbit!" and I go elsewhere.

Originally posted by @tig in #3323 (comment)

@tig tig changed the title Make AnchorEnd automatically account for views dimension Add Pos.AnchorEnd () (no param) to automatically account for the view's dimension Apr 15, 2024
tig added a commit that referenced this issue Apr 17, 2024
Fixes #481. Adds `Pos.AnchorEnd ()` (no param) to automatically account for the view's dimension.
@tig tig closed this as completed Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...)
Projects
No open projects
Status: ✅ Done
Development

No branches or pull requests

3 participants