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

Fixes #3276. Adornment.Bounds/FrameToScreen #3277

Merged
merged 9 commits into from
Feb 28, 2024
15 changes: 9 additions & 6 deletions Terminal.Gui/Application.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1165,7 +1165,7 @@
&& top != OverlappedTop
&& top != Current
&& Current?.Running == false
&& !top.Running)

Check warning on line 1168 in Terminal.Gui/Application.cs

View workflow job for this annotation

GitHub Actions / build_and_test

Dereference of a possibly null reference.
{
lock (_topLevels)
{
Expand Down Expand Up @@ -1362,7 +1362,7 @@
/// </para>
/// <para>The <see cref="MouseEvent.View"/> will contain the <see cref="View"/> that contains the mouse coordinates.</para>
/// </remarks>
public static event EventHandler<MouseEventEventArgs> MouseEvent;

Check warning on line 1365 in Terminal.Gui/Application.cs

View workflow job for this annotation

GitHub Actions / build_and_test

Non-nullable event 'MouseEvent' must contain a non-null value when exiting constructor. Consider declaring the event as nullable.

/// <summary>Called when a mouse event occurs. Raises the <see cref="MouseEvent"/> event.</summary>
/// <remarks>This method can be used to simulate a mouse event, e.g. in unit tests.</remarks>
Expand All @@ -1374,7 +1374,8 @@
return;
}

var view = View.FindDeepestView (Current, a.MouseEvent.X, a.MouseEvent.Y, out int screenX, out int screenY);
// TODO: In PR #3273, FindDeepestView will return adornments. Update logic below to fix adornment mouse handling
var view = View.FindDeepestView (Current, a.MouseEvent.X, a.MouseEvent.Y);

if (view is { WantContinuousButtonPressed: true })
{
Expand Down Expand Up @@ -1437,11 +1438,11 @@
&& a.MouseEvent.Flags != 0)
{
View? top = FindDeepestTop (Top, a.MouseEvent.X, a.MouseEvent.Y);
view = View.FindDeepestView (top, a.MouseEvent.X, a.MouseEvent.Y, out screenX, out screenY);
view = View.FindDeepestView (top, a.MouseEvent.X, a.MouseEvent.Y);

if (view is { } && view != OverlappedTop && top != Current)
{
MoveCurrent ((Toplevel)top);

Check warning on line 1445 in Terminal.Gui/Application.cs

View workflow job for this annotation

GitHub Actions / build_and_test

Converting null literal or possible null value to non-nullable type.
}
}

Expand All @@ -1450,6 +1451,8 @@
return;
}

var screen = view.FrameToScreen ();

// Work inside-out (Padding, Border, Margin)
// TODO: Debate whether inside-out or outside-in is the right strategy
if (AdornmentHandledMouseEvent (view.Padding, a))
Expand All @@ -1469,11 +1472,11 @@

var me = new MouseEvent
{
X = screenX,
Y = screenY,
X = a.MouseEvent.X - screen.X,
Y = a.MouseEvent.Y - screen.Y,
Flags = a.MouseEvent.Flags,
OfX = screenX,
OfY = screenY,
OfX = a.MouseEvent.X - screen.X,
OfY = a.MouseEvent.Y - screen.Y,
View = view
};

Expand Down Expand Up @@ -1512,7 +1515,7 @@
return;
}

Rectangle bounds = view.BoundsToScreen (view.Bounds);

Check warning on line 1518 in Terminal.Gui/Application.cs

View workflow job for this annotation

GitHub Actions / build_and_test

Dereference of a possibly null reference.

if (bounds.Contains (a.MouseEvent.X, a.MouseEvent.Y))
{
Expand Down
9 changes: 9 additions & 0 deletions Terminal.Gui/Input/Responder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ public void Dispose ()
public event EventHandler Disposing;

/// <summary>Method invoked when a mouse event is generated</summary>
/// <remarks>
/// The coordinates are relative to <see cref="View.Bounds"/>.
/// </remarks>
/// <returns><c>true</c>, if the event was handled, <c>false</c> otherwise.</returns>
/// <param name="mouseEvent">Contains the details about the mouse event.</param>
public virtual bool MouseEvent (MouseEvent mouseEvent) { return false; }
Expand All @@ -81,6 +84,9 @@ public virtual void OnEnabledChanged () { }
/// Called when the mouse first enters the view; the view will now receives mouse events until the mouse leaves
/// the view. At which time, <see cref="OnMouseLeave(Gui.MouseEvent)"/> will be called.
/// </summary>
/// <remarks>
/// The coordinates are relative to <see cref="View.Bounds"/>.
/// </remarks>
/// <param name="mouseEvent"></param>
/// <returns><c>true</c>, if the event was handled, <c>false</c> otherwise.</returns>
public virtual bool OnMouseEnter (MouseEvent mouseEvent) { return false; }
Expand All @@ -89,6 +95,9 @@ public virtual void OnEnabledChanged () { }
/// Called when the mouse has moved outside of the view; the view will no longer receive mouse events (until the
/// mouse moves within the view again and <see cref="OnMouseEnter(Gui.MouseEvent)"/> is called).
/// </summary>
/// <remarks>
/// The coordinates are relative to <see cref="View.Bounds"/>.
/// </remarks>
/// <param name="mouseEvent"></param>
/// <returns><c>true</c>, if the event was handled, <c>false</c> otherwise.</returns>
public virtual bool OnMouseLeave (MouseEvent mouseEvent) { return false; }
Expand Down
14 changes: 8 additions & 6 deletions Terminal.Gui/View/Adornment/Adornment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public Adornment ()
/// <summary>Gets the rectangle that describes the inner area of the Adornment. The Location is always (0,0).</summary>
public override Rectangle Bounds
{
get => Thickness?.GetInside (new (Point.Empty, Frame.Size)) ?? new Rectangle (Point.Empty, Frame.Size);
get => new Rectangle (Point.Empty, Thickness?.GetInside (new (Point.Empty, Frame.Size)).Size ?? Frame.Size);
// QUESTION: So why even have a setter then?
set => throw new InvalidOperationException ("It makes no sense to set Bounds of a Thickness.");
}
Expand Down Expand Up @@ -100,17 +100,19 @@ public override void BoundsToScreen (int col, int row, out int rcol, out int rro
/// <inheritdoc/>
public override Rectangle FrameToScreen ()
{
if (Parent is null)
{
return Frame;
}

// Adornments are *Children* of a View, not SubViews. Thus View.FrameToScreen will not work.
// To get the screen-relative coordinates of a Adornment, we need to know who
// the Parent is
Rectangle ret = Parent?.Frame ?? Frame;
ret.Size = Frame.Size;

ret.Location = Parent?.FrameToScreen ().Location ?? ret.Location;
Rectangle parent = Parent.FrameToScreen ();

// We now have coordinates relative to our View. If our View's SuperView has
// a SuperView, keep going...
return ret;
return new (new (parent.X + Frame.X, parent.Y + Frame.Y), Frame.Size);
}

/// <summary>Does nothing for Adornment</summary>
Expand Down
42 changes: 29 additions & 13 deletions Terminal.Gui/View/Layout/ViewLayout.cs
Original file line number Diff line number Diff line change
Expand Up @@ -545,28 +545,49 @@ public virtual void BoundsToScreen (int x, int y, out int rx, out int ry, bool c
}
}

#nullable enable
#nullable enable
/// <summary>Finds which view that belong to the <paramref name="start"/> superview at the provided location.</summary>
/// <param name="start">The superview where to look for.</param>
/// <param name="x">The column location in the superview.</param>
/// <param name="y">The row location in the superview.</param>
/// <param name="resultX">The found view screen relative column location.</param>
/// <param name="resultY">The found view screen relative row location.</param>
/// <param name="findAdornments">TODO: Remove this in PR #3273</param>
/// <returns>
/// The view that was found at the <paramref name="x"/> and <paramref name="y"/> coordinates.
/// <see langword="null"/> if no view was found.
/// </returns>

// CONCURRENCY: This method is not thread-safe.
// Undefined behavior and likely program crashes are exposed by unsynchronized access to InternalSubviews.
public static View? FindDeepestView (View? start, int x, int y, out int resultX, out int resultY)
public static View? FindDeepestView (View? start, int x, int y, bool findAdornments = false)
{
resultY = resultX = 0;
if (start is null || !start.Visible)
{
return null;
}

if (start is null || !start.Frame.Contains (x, y))
if (!start.Frame.Contains (x, y))
{
return null;
}

if (findAdornments)
{
// TODO: This is a temporary hack for PR #3273; it is not actually used anywhere but unit tests at this point.
if (start.Margin.Thickness.Contains (start.Margin.Frame, x, y))
{
return start.Margin;
}
if (start.Border.Thickness.Contains (start.Border.Frame, x, y))
{
return start.Border;
}
if (start.Padding.Thickness.Contains (start.Padding.Frame, x, y))
{
return start.Padding;
}

}

if (start.InternalSubviews is { Count: > 0 })
{
Point boundsOffset = start.GetBoundsOffset ();
Expand All @@ -579,19 +600,14 @@ public virtual void BoundsToScreen (int x, int y, out int rx, out int ry, bool c

if (v.Visible && v.Frame.Contains (rx, ry))
{
View? deep = FindDeepestView (v, rx, ry, out resultX, out resultY);

View? deep = FindDeepestView (v, rx, ry, findAdornments);
return deep ?? v;
}
}
}

resultX = x - start.Frame.X;
resultY = y - start.Frame.Y;

return start;
}
#nullable restore
#nullable restore

/// <summary>Gets the <see cref="Frame"/> with a screen-relative location.</summary>
/// <returns>The location and size of the view in screen-relative coordinates.</returns>
Expand Down
16 changes: 8 additions & 8 deletions Terminal.Gui/Views/Menu/Menu.cs
Original file line number Diff line number Diff line change
Expand Up @@ -727,13 +727,7 @@ private void Application_RootMouseEvent (object sender, MouseEventEventArgs a)
locationOffset.Y += SuperView.Border.Thickness.Top;
}

View view = FindDeepestView (
this,
a.MouseEvent.X + locationOffset.X,
a.MouseEvent.Y + locationOffset.Y,
out int rx,
out int ry
);
View view = FindDeepestView (this, a.MouseEvent.X + locationOffset.X, a.MouseEvent.Y + locationOffset.Y);

if (view == this)
{
Expand All @@ -742,7 +736,13 @@ out int ry
throw new InvalidOperationException ("This shouldn't running on a invisible menu!");
}

var nme = new MouseEvent { X = rx, Y = ry, Flags = a.MouseEvent.Flags, View = view };
var screen = view.FrameToScreen ();
var nme = new MouseEvent {
X = a.MouseEvent.X - screen.X,
Y = a.MouseEvent.Y - screen.Y,
Flags = a.MouseEvent.Flags,
View = view
};

if (MouseEvent (nme) || a.MouseEvent.Flags == MouseFlags.Button1Pressed || a.MouseEvent.Flags == MouseFlags.Button1Released)
{
Expand Down
168 changes: 168 additions & 0 deletions UnitTests/View/Adornment/AdornmentTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,46 @@ public class AdornmentTests
private readonly ITestOutputHelper _output;
public AdornmentTests (ITestOutputHelper output) { _output = output; }

[Fact]
public void Bounds_Location_Always_Empty_Size_Correct ()
{
var view = new View
{
X = 1,
Y = 2,
Width = 20,
Height = 31
};

var marginThickness = 1;
view.Margin.Thickness = new Thickness (marginThickness);

var borderThickness = 2;
view.Border.Thickness = new Thickness (borderThickness);

var paddingThickness = 3;
view.Padding.Thickness = new Thickness (paddingThickness);

view.BeginInit ();
view.EndInit ();

Assert.Equal (new Rectangle (1, 2, 20, 31), view.Frame);
Assert.Equal (new Rectangle (0, 0, 8, 19), view.Bounds);

Assert.Equal (new Rectangle (0, 0, view.Margin.Frame.Width - marginThickness * 2, view.Margin.Frame.Height - marginThickness * 2), view.Margin.Bounds);

Assert.Equal (new Rectangle (0, 0, view.Border.Frame.Width - borderThickness * 2, view.Border.Frame.Height - borderThickness * 2), view.Border.Bounds);

Assert.Equal (
new Rectangle (
0,
0,
view.Padding.Frame.Width - (marginThickness + borderThickness) * 2,
view.Padding.Frame.Height - (marginThickness + borderThickness) * 2),
view.Padding.Bounds);
}

// Test that Adornment.Bounds_get override uses Parent not SuperView
[Fact]
public void BoundsToScreen_Uses_Parent_Not_SuperView ()
{
Expand All @@ -25,6 +65,134 @@ public void BoundsToScreen_Uses_Parent_Not_SuperView ()
Assert.Equal (new Rectangle (2, 4, 5, 5), boundsAsScreen);
}

[Fact]
public void Frames_are_Parent_SuperView_Relative ()
{
var view = new View
{
X = 1,
Y = 2,
Width = 20,
Height = 31
};

var marginThickness = 1;
view.Margin.Thickness = new Thickness (marginThickness);

var borderThickness = 2;
view.Border.Thickness = new Thickness (borderThickness);

var paddingThickness = 3;
view.Padding.Thickness = new Thickness (paddingThickness);

view.BeginInit ();
view.EndInit ();

Assert.Equal (new Rectangle (1, 2, 20, 31), view.Frame);
Assert.Equal (new Rectangle (0, 0, 8, 19), view.Bounds);

// Margin.Frame is always the same as the view frame
Assert.Equal (new Rectangle (0, 0, 20, 31), view.Margin.Frame);

// Border.Frame is View.Frame minus the Margin thickness
Assert.Equal (
new Rectangle (marginThickness, marginThickness, view.Frame.Width - marginThickness * 2, view.Frame.Height - marginThickness * 2),
view.Border.Frame);

// Padding.Frame is View.Frame minus the Border thickness plus Margin thickness
Assert.Equal (
new Rectangle (
marginThickness + borderThickness,
marginThickness + borderThickness,
view.Frame.Width - (marginThickness + borderThickness) * 2,
view.Frame.Height - (marginThickness + borderThickness) * 2),
view.Padding.Frame);
}

// Test that Adornment.FrameToScreen override retains Frame.Size
[Theory]
[InlineData (0, 0, 0)]
[InlineData (0, 1, 1)]
[InlineData (0, 10, 10)]
[InlineData (1, 0, 0)]
[InlineData (1, 1, 1)]
[InlineData (1, 10, 10)]
public void FrameToScreen_Retains_Frame_Size (int marginThickness, int w, int h)
{
var parent = new View { X = 1, Y = 2, Width = w, Height = h };
parent.Margin.Thickness = new Thickness (marginThickness);

parent.BeginInit ();
parent.EndInit ();

Assert.Equal (new Rectangle (1, 2, w, h), parent.Frame);
Assert.Equal (new Rectangle (0, 0, w, h), parent.Margin.Frame);

Assert.Equal (parent.Frame, parent.Margin.FrameToScreen ());
}

// Test that Adornment.FrameToScreen override returns Frame if Parent is null
[Fact]
public void FrameToScreen_Returns_Frame_If_Parent_Is_Null ()
{
var a = new Adornment
{
X = 1,
Y = 2,
Width = 3,
Height = 4
};

Assert.Null (a.Parent);
Assert.Equal (a.Frame, a.FrameToScreen ());
}

// Test that Adornment.FrameToScreen override returns correct location
[Theory]
[InlineData (0, 0, 0, 0)]
[InlineData (0, 0, 1, 1)]
[InlineData (0, 0, 10, 10)]
[InlineData (1, 0, 0, 0)]
[InlineData (1, 0, 1, 1)]
[InlineData (1, 0, 10, 10)]
[InlineData (0, 1, 0, 0)]
[InlineData (0, 1, 1, 1)]
[InlineData (0, 1, 10, 10)]
[InlineData (1, 1, 0, 0)]
[InlineData (1, 1, 1, 1)]
[InlineData (1, 1, 10, 10)]
public void FrameToScreen_Returns_Screen_Location (int marginThickness, int borderThickness, int x, int y)
{
var superView = new View
{
X = 1,
Y = 1,
Width = 20,
Height = 20
};
superView.Margin.Thickness = new Thickness (marginThickness);
superView.Border.Thickness = new Thickness (borderThickness);

var view = new View { X = x, Y = y, Width = 1, Height = 1 };
superView.Add (view);
superView.BeginInit ();
superView.EndInit ();

Assert.Equal (new Rectangle (x, y, 1, 1), view.Frame);
Assert.Equal (new Rectangle (0, 0, 20, 20), superView.Margin.Frame);

Assert.Equal (
new Rectangle (marginThickness, marginThickness, 20 - marginThickness * 2, 20 - marginThickness * 2),
superView.Border.Frame
);

Assert.Equal (
new Rectangle (superView.Frame.X + marginThickness, superView.Frame.Y + marginThickness, 20 - marginThickness * 2, 20 - marginThickness * 2),
superView.Border.FrameToScreen ()
);
}

// Test that Adornment.FrameToScreen override uses Parent not SuperView
[Fact]
public void FrameToScreen_Uses_Parent_Not_SuperView ()
{
Expand Down
Loading
Loading