diff --git a/Terminal.Gui/ViewBase/View.Layout.cs b/Terminal.Gui/ViewBase/View.Layout.cs index 175c35e34..618acb6e3 100644 --- a/Terminal.Gui/ViewBase/View.Layout.cs +++ b/Terminal.Gui/ViewBase/View.Layout.cs @@ -11,7 +11,7 @@ public partial class View // Layout APIs /// /// SuperView-relative coordinate /// if the specified SuperView-relative coordinates are within the View. - public virtual bool Contains (in Point location) { return Frame.Contains (location); } + public virtual bool Contains (in Point location) => Frame.Contains (location); private Rectangle? _frame; @@ -227,7 +227,7 @@ public partial class View // Layout APIs /// public Pos X { - get => VerifyIsInitialized (_x, nameof (X)); + get => _x; set { if (Equals (_x, value)) @@ -272,7 +272,7 @@ public partial class View // Layout APIs /// public Pos Y { - get => VerifyIsInitialized (_y, nameof (Y)); + get => _y; set { if (Equals (_y, value)) @@ -323,7 +323,7 @@ public partial class View // Layout APIs /// public Dim Height { - get => VerifyIsInitialized (_height, nameof (Height)); + get => _height; set { CWPPropertyHelper.ChangeProperty ( @@ -352,7 +352,7 @@ public partial class View // Layout APIs /// /// The event arguments containing the current and proposed new height. /// True to cancel the change, false to proceed. - protected virtual bool OnHeightChanging (ValueChangingEventArgs args) { return false; } + protected virtual bool OnHeightChanging (ValueChangingEventArgs args) => false; /// /// Called after the property changes, allowing subclasses to react to the change. @@ -411,7 +411,7 @@ public partial class View // Layout APIs /// public Dim Width { - get => VerifyIsInitialized (_width, nameof (Width)); + get => _width; set { CWPPropertyHelper.ChangeProperty ( @@ -437,8 +437,9 @@ public partial class View // Layout APIs private void NeedsClearScreenNextIteration () { - if (App is { TopRunnableView: { } } && App.TopRunnableView == this - && App.SessionStack!.Select (r => r.Runnable as View).Count() == 1) + if (App is { TopRunnableView: { } } + && App.TopRunnableView == this + && App.SessionStack!.Select (r => r.Runnable as View).Count () == 1) { // If this is the only Runnable, we need to redraw the screen App.ClearScreenNextIteration = true; @@ -450,7 +451,7 @@ public partial class View // Layout APIs /// /// The event arguments containing the current and proposed new width. /// True to cancel the change, false to proceed. - protected virtual bool OnWidthChanging (ValueChangingEventArgs args) { return false; } + protected virtual bool OnWidthChanging (ValueChangingEventArgs args) => false; /// /// Called after the property changes, allowing subclasses to react to the change. @@ -576,7 +577,6 @@ public partial class View // Layout APIs Debug.Assert (_x is { }); Debug.Assert (_y is { }); - CheckDimAuto (); // TODO: Should move to View.LayoutSubViews? @@ -968,6 +968,7 @@ public partial class View // Layout APIs if (dim!.Has (out DimCombine dc)) { + // TODO: Redo without recursion CollectDim (dc.Left, from, ref nNodes, ref nEdges); CollectDim (dc.Right, from, ref nNodes, ref nEdges); } @@ -998,6 +999,7 @@ public partial class View // Layout APIs return; case PosCombine pc: + // TODO: Redo without recursion CollectPos (pc.Left, from, ref nNodes, ref nEdges); CollectPos (pc.Right, from, ref nNodes, ref nEdges); @@ -1119,11 +1121,9 @@ public partial class View // Layout APIs : App?.Screen.Size ?? new (2048, 2048)); return superViewContentSize; - } // BUGBUG: This method interferes with Dialog/MessageBox default min/max size. - // TODO: Get rid of MenuBar coupling as part of https://github.com/gui-cs/Terminal.Gui/issues/2975 // TODO: Refactor / rewrite this - It's a mess /// /// Gets a new location of the that is within the Viewport of the 's @@ -1176,35 +1176,19 @@ public partial class View // Layout APIs { nx = Math.Max (targetX, 0); nx = nx + viewToMove.Frame.Width > maxDimension ? Math.Max (maxDimension - viewToMove.Frame.Width, 0) : nx; - - //if (nx > viewToMove.Frame.X + viewToMove.Frame.Width) - //{ - // nx = Math.Max (viewToMove.Frame.Right, 0); - //} } else { nx = 0; //targetX; } - //System.Diagnostics.Debug.WriteLine ($"nx:{nx}, rWidth:{rWidth}"); - //var menuVisible = false; - //var statusVisible = false; - maxDimension = 0; ny = Math.Max (targetY, maxDimension); - if (viewToMove?.SuperView is null || viewToMove == app?.TopRunnableView || viewToMove?.SuperView == app?.TopRunnableView) + if (viewToMove?.SuperView is null || viewToMove == app?.TopRunnableView || viewToMove.SuperView == app?.TopRunnableView) { - if (app is { }) - { - maxDimension = app.Screen.Height; - } - else - { - maxDimension = 0; - } + maxDimension = app is { } ? app.Screen.Height : 0; } else { @@ -1229,9 +1213,7 @@ public partial class View // Layout APIs ny = 0; } - //System.Diagnostics.Debug.WriteLine ($"ny:{ny}, rHeight:{rHeight}"); - - return superView!; + return superView; } /// @@ -1267,7 +1249,7 @@ public partial class View // Layout APIs // Traverse all visible runnables, topmost first (reverse stack order) if (App?.SessionStack!.Count > 0) { - foreach (View? runnable in App.SessionStack!.Select(r => r.Runnable as View)) + foreach (View? runnable in App.SessionStack!.Select (r => r.Runnable as View)) { if (runnable!.Visible && runnable.Contains (screenLocation)) { @@ -1398,10 +1380,35 @@ public partial class View // Layout APIs // Add the current view to the result result.Add (currentView); - // Add adornments for the current view - result.AddRange (Adornment.GetViewsAtLocation (currentView.Margin, location)); - result.AddRange (Adornment.GetViewsAtLocation (currentView.Border, location)); - result.AddRange (Adornment.GetViewsAtLocation (currentView.Padding, location)); + // Push adornments onto the stack BEFORE subviews so adornments' subviews are processed AFTER regular subviews + // This ensures that adornment subviews (e.g., ExpanderButton in Border) are considered "deeper" than + // regular subviews' adornments (e.g., childView.Border) when they overlap. + // Push in reverse order (Padding, Border, Margin) so they're processed in correct order (Margin, Border, Padding) + Point superViewRelativeLocation = currentView.SuperView?.ScreenToViewport (location) ?? location; + + if (currentView.Padding is { } padding && padding.Thickness != Thickness.Empty) + { + if (padding.Contains (superViewRelativeLocation) && padding.FrameToScreen ().Contains (location)) + { + viewsToProcess.Push (padding); + } + } + + if (currentView.Border is { } border && border.Thickness != Thickness.Empty) + { + if (border.Contains (superViewRelativeLocation) && border.FrameToScreen ().Contains (location)) + { + viewsToProcess.Push (border); + } + } + + if (currentView.Margin is { } margin && margin.Thickness != Thickness.Empty) + { + if (margin.Contains (superViewRelativeLocation) && margin.FrameToScreen ().Contains (location)) + { + viewsToProcess.Push (margin); + } + } // Add subviews to the stack in reverse order // This maintains the original depth-first traversal order @@ -1423,35 +1430,6 @@ public partial class View // Layout APIs #region Diagnostics and Verification - // Diagnostics to highlight when X or Y is read before the view has been initialized - private Pos VerifyIsInitialized (Pos pos, string member) - { - //#if DEBUG - // if (pos.ReferencesOtherViews () && !IsInitialized) - // { - // Debug.WriteLine ( - // $"WARNING: {member} = {pos} of {this} is dependent on other views and {member} " - // + $"is being accessed before the View has been initialized. This is likely a bug." - // ); - // } - //#endif // DEBUG - return pos; - } - - // Diagnostics to highlight when Width or Height is read before the view has been initialized - private Dim VerifyIsInitialized (Dim dim, string member) - { - //#if DEBUG - // if (dim.ReferencesOtherViews () && !IsInitialized) - // { - // Debug.WriteLine ( - // $"WARNING: {member} = {dim} of {this} is dependent on other views and {member} " - // + $"is being accessed before the View has been initialized. This is likely a bug." - // ); - // } - //#endif // DEBUG - return dim; - } /// Gets or sets whether validation of and occurs. /// diff --git a/Tests/UnitTestsParallelizable/ViewBase/Layout/GetViewsUnderLocationTests.cs b/Tests/UnitTestsParallelizable/ViewBase/Layout/GetViewsUnderLocationTests.cs index 6db388683..b6046a39a 100644 --- a/Tests/UnitTestsParallelizable/ViewBase/Layout/GetViewsUnderLocationTests.cs +++ b/Tests/UnitTestsParallelizable/ViewBase/Layout/GetViewsUnderLocationTests.cs @@ -1,6 +1,4 @@ -#nullable enable - -namespace ViewBaseTests.Mouse; +namespace ViewBaseTests.Mouse; [Trait ("Category", "Input")] public class GetViewsUnderLocationTests @@ -145,5 +143,87 @@ public class GetViewsUnderLocationTests Assert.Equal (expectedAdornmentType, containedType); } -} + [Fact] + public void GetViewsUnderLocation_Returns_Adornment_Subview_When_Parent_Has_Subview_At_Same_Location () + { + // Arrange - Reproduces the bug where: + // - Parent has ExpanderButton in its Border + // - A subview with X=-1 (extends outside parent's content) has a Border that overlaps ExpanderButton + // - Bug: GetViewsUnderLocation returns subview.Border instead of ExpanderButton + IApplication app = Application.Create (); + + Runnable runnable = new () + { + Width = 50, + Height = 50 + }; + app.Begin (runnable); + + // Create parent view + var parent = new View + { + X = 0, + Y = 0, + Width = 30, + Height = 10 + }; + parent.Border!.Thickness = new (1); + parent.Border.ViewportSettings = ViewportSettingsFlags.None; + + // Add ExpanderButton to parent's Border at (0, 0) + // Since parent.Border has thickness=1, the Border's viewport starts at (0,0) screen coords + // And the ExpanderButton at (0,0) relative to Border viewport is at screen (0,0) + var expanderButton = new Button + { + X = 0, + Y = 0, + Width = 1, + Height = 1, + Text = ">", + ShadowStyle = ShadowStyle.None + }; + parent.Border.Add (expanderButton); + + // Add a subview at X=-1, Y=-1 (extends outside parent's Viewport in both dimensions) + // The subview's Border will overlap with the ExpanderButton location + var childView = new View + { + X = -1, // This causes child's left edge to be at screen X=0 (parent content starts at X=1) + Y = -1, // This causes child's top edge to be at screen Y=0 (parent content starts at Y=1) + Width = 20, + Height = 5 + }; + childView.Border!.Thickness = new (1); + childView.Border!.ViewportSettings = ViewportSettingsFlags.None; + parent.Add (childView); + + runnable.Add (parent); + runnable.Layout (); + + // Get screen location of ExpanderButton + Rectangle buttonFrame = expanderButton.FrameToScreen (); + Point testLocation = buttonFrame.Location; + + // Verify that childView.Border also contains this location (this is the bug scenario) + Rectangle childBorderFrame = childView.Border.FrameToScreen (); + + Assert.True ( + childBorderFrame.Contains (testLocation), + $"Test setup failed: childView.Border ({childBorderFrame}) should contain testLocation ({testLocation})"); + + // Act + List viewsUnderLocation = runnable.GetViewsUnderLocation (testLocation, ViewportSettingsFlags.None); + + // Assert + View? deepestView = viewsUnderLocation.LastOrDefault (); + Assert.NotNull (deepestView); + + // The ExpanderButton is a subview of parent.Border, which is processed before childView + // But childView.Border is processed AFTER ExpanderButton, causing the bug + // The correct deepest view should be ExpanderButton, not childView.Border + Assert.Equal (expanderButton, deepestView); + + app.Dispose (); + } +}