Skip to content

Commit 6f8c627

Browse files
nickprotopnickprotop
committed
fix(menu): address 10 bugs, 5 enhancements, and portal arrange bug
Bugs fixed: - Scroll indicators placed on border rows, use narrow arrows (Rule E) - Submenu indicator uses ControlDefaults constant (Rule 2) - Menu items render with MarkupParser.Parse for Unicode correctness (Rule C) - Dead textWidth variable used as truncation guard - FindItemByPath/JumpToItemStartingWith strip markup before comparison - HitTest filters to visible range for scrolled dropdowns - RemoveItem closes orphaned dropdown portals - Double-click toggle fixed for open menu items - Menu aim delay moved to ProcessDropdownMouseEvent where submenu closing actually happens (CloseSiblingSubmenus) - DesktopPortal root node arranged at portal bounds, not desired size Enhancements: - DownArrow on leaf top-level item executes action - IsSticky prevents Escape from unfocusing menu - Focus() initializes _focusedItem to first enabled item - MenuItem.GetDepth() cached with InvalidateDepthCache - Null-coalescing chains extracted to ColorResolver (Rule 7) Tests: - 110 comprehensive MenuControl tests - DesktopPortal lifecycle, rendering, hit-testing, and input tests
1 parent c39831e commit 6f8c627

22 files changed

Lines changed: 3183 additions & 490 deletions

‎SharpConsoleUI.Tests/Controls/MenuControlTests.cs‎

Lines changed: 1658 additions & 0 deletions
Large diffs are not rendered by default.

‎SharpConsoleUI.Tests/InputHandling/ShortcutsTests.cs‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -220,20 +220,24 @@ public void ShiftTab_SwitchesControlFocusBackward()
220220
}
221221

222222
[Fact]
223-
public void Escape_OnOverlayWindow_DismissesOverlay()
223+
public void Escape_OnDesktopPortal_DismissesPortal()
224224
{
225225
var system = TestWindowSystemBuilder.CreateTestSystem();
226-
var overlay = new Windows.OverlayWindow(system);
226+
var label = new SharpConsoleUI.Controls.MarkupControl(new List<string> { "Portal Content" });
227227

228-
system.WindowStateService.AddWindow(overlay);
229-
system.WindowStateService.SetActiveWindow(overlay);
228+
system.DesktopPortalService.CreatePortal(new Core.DesktopPortalOptions(
229+
Content: label,
230+
Bounds: new System.Drawing.Rectangle(0, 0, 40, 10),
231+
DismissOnClickOutside: true));
232+
233+
Assert.True(system.DesktopPortalService.HasPortals);
230234

231235
var escapeKey = new ConsoleKeyInfo('\0', ConsoleKey.Escape, false, false, false);
232236
system.InputStateService.EnqueueKey(escapeKey);
233237
system.Input.ProcessInput();
234238

235-
// Overlay might close on Escape
236-
// This test verifies it doesn't crash
239+
// Portal should be dismissed by Escape
240+
Assert.False(system.DesktopPortalService.HasPortals);
237241
}
238242

239243
[Fact]

‎SharpConsoleUI.Tests/Rendering/DesktopPortalTests.cs‎

Lines changed: 339 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,339 @@
1+
using SharpConsoleUI;
2+
using SharpConsoleUI.Controls;
3+
using SharpConsoleUI.Core;
4+
using SharpConsoleUI.Layout;
5+
using SharpConsoleUI.Tests.Infrastructure;
6+
using System.Drawing;
7+
using Xunit;
8+
9+
namespace SharpConsoleUI.Tests.Rendering;
10+
11+
public class DesktopPortalTests
12+
{
13+
#region Portal Lifecycle
14+
15+
[Fact]
16+
public void CreatePortal_SetsUpPortalState()
17+
{
18+
var system = TestWindowSystemBuilder.CreateTestSystem();
19+
var content = new MarkupControl(new List<string> { "Hello" });
20+
21+
var portal = system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
22+
Content: content,
23+
Bounds: new Rectangle(5, 5, 30, 10)));
24+
25+
Assert.NotNull(portal);
26+
Assert.True(system.DesktopPortalService.HasPortals);
27+
Assert.Equal(content, portal.Content);
28+
Assert.Equal(new Rectangle(5, 5, 30, 10), portal.Bounds);
29+
Assert.True(portal.IsDirty);
30+
}
31+
32+
[Fact]
33+
public void CreatePortal_ComputesInitialControlBounds()
34+
{
35+
var system = TestWindowSystemBuilder.CreateTestSystem();
36+
var content = new MarkupControl(new List<string> { "Hello" });
37+
38+
var portal = system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
39+
Content: content,
40+
Bounds: new Rectangle(0, 0, 40, 10)));
41+
42+
// Control bounds should be computed immediately (not waiting for first render)
43+
Assert.NotEmpty(portal.ControlBounds);
44+
}
45+
46+
[Fact]
47+
public void CreatePortal_RootNodeIsMeasuredAndArranged()
48+
{
49+
var system = TestWindowSystemBuilder.CreateTestSystem();
50+
var content = new MarkupControl(new List<string> { "Test content" });
51+
52+
var portal = system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
53+
Content: content,
54+
Bounds: new Rectangle(0, 0, 40, 10)));
55+
56+
// RootNode should have non-empty bounds after arrange
57+
Assert.False(portal.RootNode.AbsoluteBounds.IsEmpty);
58+
Assert.Equal(40, portal.RootNode.AbsoluteBounds.Width);
59+
Assert.Equal(10, portal.RootNode.AbsoluteBounds.Height);
60+
}
61+
62+
[Fact]
63+
public void RemovePortal_ClearsPortalState()
64+
{
65+
var system = TestWindowSystemBuilder.CreateTestSystem();
66+
var content = new MarkupControl(new List<string> { "Hello" });
67+
68+
var portal = system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
69+
Content: content,
70+
Bounds: new Rectangle(5, 5, 30, 10)));
71+
72+
system.DesktopPortalService.RemovePortal(portal);
73+
74+
Assert.False(system.DesktopPortalService.HasPortals);
75+
}
76+
77+
[Fact]
78+
public void DismissAllPortals_RemovesAllPortals()
79+
{
80+
var system = TestWindowSystemBuilder.CreateTestSystem();
81+
82+
system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
83+
Content: new MarkupControl(new List<string> { "One" }),
84+
Bounds: new Rectangle(0, 0, 20, 5)));
85+
system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
86+
Content: new MarkupControl(new List<string> { "Two" }),
87+
Bounds: new Rectangle(0, 0, 20, 5)));
88+
89+
Assert.Equal(2, system.DesktopPortalService.Portals.Count);
90+
91+
system.DesktopPortalService.DismissAllPortals();
92+
93+
Assert.False(system.DesktopPortalService.HasPortals);
94+
}
95+
96+
[Fact]
97+
public void RemovePortal_InvokesOnDismissCallback()
98+
{
99+
var system = TestWindowSystemBuilder.CreateTestSystem();
100+
bool dismissed = false;
101+
102+
var portal = system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
103+
Content: new MarkupControl(new List<string> { "Hello" }),
104+
Bounds: new Rectangle(0, 0, 20, 5),
105+
OnDismiss: () => dismissed = true));
106+
107+
system.DesktopPortalService.RemovePortal(portal);
108+
109+
Assert.True(dismissed);
110+
}
111+
112+
#endregion
113+
114+
#region Dirty Tracking
115+
116+
[Fact]
117+
public void NewPortal_IsMarkedDirty()
118+
{
119+
var system = TestWindowSystemBuilder.CreateTestSystem();
120+
121+
var portal = system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
122+
Content: new MarkupControl(new List<string> { "Hello" }),
123+
Bounds: new Rectangle(0, 0, 20, 5)));
124+
125+
Assert.True(system.DesktopPortalService.AnyPortalDirty());
126+
}
127+
128+
#endregion
129+
130+
#region Rendering
131+
132+
[Fact]
133+
public void RenderDesktopPortals_CreatesBufferAndPaintsContent()
134+
{
135+
var system = TestWindowSystemBuilder.CreateTestSystem(80, 24);
136+
var content = new MarkupControl(new List<string> { "Portal Text" });
137+
138+
var portal = system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
139+
Content: content,
140+
Bounds: new Rectangle(0, 0, 40, 10)));
141+
142+
// Trigger render
143+
system.Render.UpdateDisplay();
144+
145+
// Buffer should have been created
146+
Assert.NotNull(portal.Buffer);
147+
// Portal should be clean after render
148+
Assert.False(portal.IsDirty);
149+
// Control bounds should be populated
150+
Assert.NotEmpty(portal.ControlBounds);
151+
}
152+
153+
[Fact]
154+
public void RenderDesktopPortals_PaintsToBuffer()
155+
{
156+
var system = TestWindowSystemBuilder.CreateTestSystem(80, 24);
157+
var content = new MarkupControl(new List<string> { "Visible Text" });
158+
159+
var portal = system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
160+
Content: content,
161+
Bounds: new Rectangle(0, 0, 40, 10)));
162+
163+
// Trigger render
164+
system.Render.UpdateDisplay();
165+
166+
// Buffer should exist and have non-default content
167+
Assert.NotNull(portal.Buffer);
168+
Assert.True(portal.Buffer.Width > 0);
169+
Assert.True(portal.Buffer.Height > 0);
170+
171+
// Check that the buffer has actual content (not all default bg)
172+
bool hasContent = false;
173+
var defaultBg = system.Theme.DesktopBackgroundColor;
174+
for (int x = 0; x < Math.Min(portal.Buffer.Width, 12); x++)
175+
{
176+
var cell = portal.Buffer.GetCell(x, 0);
177+
if (cell.Character != new System.Text.Rune(' ') || cell.Background != defaultBg)
178+
{
179+
hasContent = true;
180+
break;
181+
}
182+
}
183+
Assert.True(hasContent, "Buffer should contain non-empty content from MarkupControl painting");
184+
}
185+
186+
[Fact]
187+
public void RenderDesktopPortals_CollectsControlBoundsAfterRender()
188+
{
189+
var system = TestWindowSystemBuilder.CreateTestSystem(80, 24);
190+
var content = new MarkupControl(new List<string> { "Hello" });
191+
192+
var portal = system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
193+
Content: content,
194+
Bounds: new Rectangle(0, 0, 40, 10)));
195+
196+
system.Render.UpdateDisplay();
197+
198+
Assert.NotEmpty(portal.ControlBounds);
199+
// Bounds should have positive width and height
200+
var firstBound = portal.ControlBounds[0];
201+
Assert.True(firstBound.Width > 0, $"Control bound width should be > 0, got {firstBound.Width}");
202+
Assert.True(firstBound.Height > 0, $"Control bound height should be > 0, got {firstBound.Height}");
203+
}
204+
205+
#endregion
206+
207+
#region Hit Testing
208+
209+
[Fact]
210+
public void HitTest_ReturnsPortal_WhenPointInsideControlBounds()
211+
{
212+
var system = TestWindowSystemBuilder.CreateTestSystem(80, 24);
213+
var content = new MarkupControl(new List<string> { "Click Me" });
214+
215+
var portal = system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
216+
Content: content,
217+
Bounds: new Rectangle(10, 5, 30, 10)));
218+
219+
// The control bounds should be at offset (0,0) relative to portal
220+
// In screen space that's (10,5)
221+
Assert.NotEmpty(portal.ControlBounds);
222+
223+
var firstBound = portal.ControlBounds[0];
224+
int screenX = portal.Bounds.X + firstBound.X;
225+
int screenY = portal.Bounds.Y + firstBound.Y;
226+
227+
var hit = system.DesktopPortalService.HitTest(new Point(screenX, screenY));
228+
Assert.Equal(portal, hit);
229+
}
230+
231+
[Fact]
232+
public void HitTest_ReturnsNull_WhenPointOutsideControlBounds()
233+
{
234+
var system = TestWindowSystemBuilder.CreateTestSystem(80, 24);
235+
var content = new MarkupControl(new List<string> { "Hello" });
236+
237+
system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
238+
Content: content,
239+
Bounds: new Rectangle(10, 5, 30, 10)));
240+
241+
// Point way outside the portal
242+
var hit = system.DesktopPortalService.HitTest(new Point(79, 23));
243+
Assert.Null(hit);
244+
}
245+
246+
#endregion
247+
248+
#region Input Routing
249+
250+
[Fact]
251+
public void KeyboardInput_WhenNoPortals_ReachesWindow()
252+
{
253+
var system = TestWindowSystemBuilder.CreateTestSystem();
254+
var window = new Window(system);
255+
var button = new ButtonControl { Text = "Test" };
256+
window.AddControl(button);
257+
system.WindowStateService.AddWindow(window);
258+
system.WindowStateService.SetActiveWindow(window);
259+
window.FocusManager.SetFocus(button, FocusReason.Programmatic);
260+
261+
bool clicked = false;
262+
button.Click += (s, e) => clicked = true;
263+
264+
system.InputStateService.EnqueueKey(new ConsoleKeyInfo('\r', ConsoleKey.Enter, false, false, false));
265+
system.Input.ProcessInput();
266+
267+
Assert.True(clicked, "Key should reach button when no portals exist");
268+
}
269+
270+
[Fact]
271+
public void KeyboardInput_WhenPortalOpen_DoesNotReachWindow()
272+
{
273+
var system = TestWindowSystemBuilder.CreateTestSystem();
274+
var window = new Window(system);
275+
var button = new ButtonControl { Text = "Test" };
276+
window.AddControl(button);
277+
system.WindowStateService.AddWindow(window);
278+
system.WindowStateService.SetActiveWindow(window);
279+
window.FocusManager.SetFocus(button, FocusReason.Programmatic);
280+
281+
bool clicked = false;
282+
button.Click += (s, e) => clicked = true;
283+
284+
// Open a portal
285+
system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
286+
Content: new MarkupControl(new List<string> { "Portal" }),
287+
Bounds: new Rectangle(0, 0, 20, 5)));
288+
289+
system.InputStateService.EnqueueKey(new ConsoleKeyInfo('\r', ConsoleKey.Enter, false, false, false));
290+
system.Input.ProcessInput();
291+
292+
Assert.False(clicked, "Key should NOT reach button when portal is open");
293+
}
294+
295+
[Fact]
296+
public void EscapeKey_DismissesPortal()
297+
{
298+
var system = TestWindowSystemBuilder.CreateTestSystem();
299+
300+
system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
301+
Content: new MarkupControl(new List<string> { "Portal" }),
302+
Bounds: new Rectangle(0, 0, 20, 5)));
303+
304+
Assert.True(system.DesktopPortalService.HasPortals);
305+
306+
system.InputStateService.EnqueueKey(new ConsoleKeyInfo('\0', ConsoleKey.Escape, false, false, false));
307+
system.Input.ProcessInput();
308+
309+
Assert.False(system.DesktopPortalService.HasPortals);
310+
}
311+
312+
#endregion
313+
314+
#region IPortalHost
315+
316+
[Fact]
317+
public void Window_ImplementsIPortalHost()
318+
{
319+
var system = TestWindowSystemBuilder.CreateTestSystem();
320+
var window = new Window(system);
321+
322+
Assert.IsAssignableFrom<IPortalHost>(window);
323+
}
324+
325+
[Fact]
326+
public void DesktopPortalContainer_ImplementsIPortalHost()
327+
{
328+
var system = TestWindowSystemBuilder.CreateTestSystem();
329+
var content = new MarkupControl(new List<string> { "Hello" });
330+
331+
var portal = system.DesktopPortalService.CreatePortal(new DesktopPortalOptions(
332+
Content: content,
333+
Bounds: new Rectangle(0, 0, 20, 5)));
334+
335+
Assert.IsAssignableFrom<IPortalHost>(portal.Container);
336+
}
337+
338+
#endregion
339+
}

‎SharpConsoleUI/Configuration/ControlDefaults.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,22 @@ public static class ControlDefaults
290290
/// </summary>
291291
public const int MenuItemDropdownPadding = 10;
292292

293+
/// <summary>Submenu indicator arrow (narrow, reliably 1-wide).</summary>
294+
public const string MenuSubmenuIndicator = "\u25B8"; // ▸ (small right-pointing triangle)
295+
296+
/// <summary>Delay in ms before switching top-level menu items when a submenu is open.</summary>
297+
public const int MenuAimDelayMs = 300;
298+
299+
/// <summary>
300+
/// Text padding inside dropdown menu items: 2 left + 2 right (default: 4).
301+
/// </summary>
302+
public const int MenuDropdownItemTextPadding = 4;
303+
304+
/// <summary>
305+
/// Width reserved for the submenu indicator character plus spacing (default: 2).
306+
/// </summary>
307+
public const int MenuSubmenuIndicatorWidth = 2;
308+
293309
// Notification defaults
294310
/// <summary>
295311
/// Horizontal padding added to notification window width beyond message length (default: 8)

0 commit comments

Comments
 (0)