Skip to content

Commit b16de90

Browse files
nickprotopnickprotop
authored
Merge pull request #9 from nickprotop/fix/click-focus-scroll-jump
Fix click-to-focus scroll jump in ScrollablePanelControl
2 parents 9a35712 + 3d42c34 commit b16de90

3 files changed

Lines changed: 280 additions & 7 deletions

File tree

‎SharpConsoleUI.Tests/FocusManagement/ClickFocusScrollTests.cs‎

Lines changed: 273 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,273 @@
1+
// -----------------------------------------------------------------------
2+
// ConsoleEx - A simple console window system for .NET Core
3+
//
4+
// Author: Nikolaos Protopapas
5+
// Email: nikolaos.protopapas@gmail.com
6+
// License: MIT
7+
// -----------------------------------------------------------------------
8+
9+
using SharpConsoleUI;
10+
using SharpConsoleUI.Controls;
11+
using SharpConsoleUI.Core;
12+
using SharpConsoleUI.Layout;
13+
using SharpConsoleUI.Tests.Infrastructure;
14+
using Xunit;
15+
16+
namespace SharpConsoleUI.Tests.FocusManagement;
17+
18+
/// <summary>
19+
/// Tests that clicking to focus a child inside a ScrollablePanelControl
20+
/// does not cause a scroll jump. Verifies the fix for IFocusScope delegation
21+
/// and ScrollChildIntoView being skipped for Mouse reason.
22+
/// </summary>
23+
public class ClickFocusScrollTests
24+
{
25+
/// <summary>
26+
/// Builds a scrolled-down SPC with two buttons (one at top, one at bottom).
27+
/// Returns the system pre-scrolled so button2 is visible and button1 is off-screen above.
28+
/// </summary>
29+
private static (ConsoleWindowSystem system, Window window,
30+
ScrollablePanelControl panel, ButtonControl button1, ButtonControl button2)
31+
BuildScrolledPanel()
32+
{
33+
var panel = new ScrollablePanelControl
34+
{
35+
VerticalAlignment = VerticalAlignment.Fill,
36+
AutoScroll = false
37+
};
38+
39+
var button1 = new ButtonControl { Text = "Top Button" };
40+
panel.AddControl(button1);
41+
42+
// Tall filler content to force scrolling
43+
var lines = new List<string>();
44+
for (int i = 0; i < 50; i++)
45+
lines.Add($"Line {i}");
46+
panel.AddControl(new MarkupControl(lines));
47+
48+
var button2 = new ButtonControl { Text = "Bottom Button" };
49+
panel.AddControl(button2);
50+
51+
var system = TestWindowSystemBuilder.CreateTestSystem(80, 20);
52+
var window = new Window(system)
53+
{
54+
Title = "Test", Left = 0, Top = 0, Width = 80, Height = 20
55+
};
56+
window.AddControl(panel);
57+
system.AddWindow(window);
58+
system.Render.UpdateDisplay();
59+
system.Render.UpdateDisplay();
60+
61+
return (system, window, panel, button1, button2);
62+
}
63+
64+
#region Step 1: IFocusScope delegation skipped for Mouse
65+
66+
/// <summary>
67+
/// SetFocus(spc, Mouse) should focus SPC itself, NOT delegate to first child.
68+
/// This is the core fix: mouse clicks let ProcessMouseEvent handle child targeting.
69+
/// </summary>
70+
[Fact]
71+
public void SetFocus_Mouse_OnSPC_DoesNotDelegateToFirstChild()
72+
{
73+
var (system, window, panel, button1, button2) = BuildScrolledPanel();
74+
75+
// Clear any auto-focus
76+
window.FocusManager.SetFocus(null, FocusReason.Programmatic);
77+
78+
// Focus SPC with Mouse reason — should NOT delegate to first child
79+
window.FocusManager.SetFocus(panel, FocusReason.Mouse);
80+
81+
Assert.True(window.FocusManager.IsFocused(panel),
82+
"SetFocus(SPC, Mouse) should focus the SPC directly");
83+
Assert.False(window.FocusManager.IsFocused(button1),
84+
"SetFocus(SPC, Mouse) should NOT delegate to first child button");
85+
}
86+
87+
/// <summary>
88+
/// SetFocus(spc, Keyboard) should delegate to first child (existing behavior preserved).
89+
/// </summary>
90+
[Fact]
91+
public void SetFocus_Keyboard_OnSPC_DelegatesToFirstChild()
92+
{
93+
var (system, window, panel, button1, button2) = BuildScrolledPanel();
94+
95+
// Clear any auto-focus
96+
window.FocusManager.SetFocus(null, FocusReason.Programmatic);
97+
98+
// Focus SPC with Keyboard reason — should delegate to first child
99+
window.FocusManager.SetFocus(panel, FocusReason.Keyboard);
100+
101+
Assert.True(window.FocusManager.IsFocused(button1),
102+
"SetFocus(SPC, Keyboard) should delegate to first focusable child");
103+
}
104+
105+
#endregion
106+
107+
#region Step 2: ScrollChildIntoView skipped for Mouse
108+
109+
/// <summary>
110+
/// When focusing via Mouse, scroll offset should not change.
111+
/// The clicked control is already visible — no scroll needed.
112+
/// </summary>
113+
[Fact]
114+
public void SetFocus_Mouse_SkipsScrollChildIntoView()
115+
{
116+
var (system, window, panel, button1, button2) = BuildScrolledPanel();
117+
118+
// Scroll down so button2 is visible
119+
panel.ScrollVerticalBy(40);
120+
system.Render.UpdateDisplay();
121+
int scrollBefore = panel.VerticalScrollOffset;
122+
123+
// Focus button2 via Mouse — should NOT trigger ScrollChildIntoView
124+
window.FocusManager.SetFocus(button2, FocusReason.Mouse);
125+
126+
Assert.Equal(scrollBefore, panel.VerticalScrollOffset);
127+
}
128+
129+
/// <summary>
130+
/// When focusing via Keyboard, ScrollChildIntoView should adjust scroll offset
131+
/// (existing behavior preserved).
132+
/// </summary>
133+
[Fact]
134+
public void SetFocus_Keyboard_ScrollsChildIntoView()
135+
{
136+
var (system, window, panel, button1, button2) = BuildScrolledPanel();
137+
138+
// Ensure we're at top
139+
panel.ScrollToTop();
140+
system.Render.UpdateDisplay();
141+
142+
// Clear focus first
143+
window.FocusManager.SetFocus(null, FocusReason.Programmatic);
144+
145+
// Focus button2 via Keyboard — button2 is off-screen, should scroll
146+
window.FocusManager.SetFocus(button2, FocusReason.Keyboard);
147+
148+
Assert.True(panel.VerticalScrollOffset > 0,
149+
$"Keyboard focus on off-screen button should trigger scroll. Offset: {panel.VerticalScrollOffset}");
150+
}
151+
152+
#endregion
153+
154+
#region HandleClick integration
155+
156+
/// <summary>
157+
/// HandleClick on a focusable child inside SPC should focus the child directly,
158+
/// not the SPC (which would then delegate to the wrong child).
159+
/// </summary>
160+
[Fact]
161+
public void HandleClick_OnFocusableChild_FocusesDirectly()
162+
{
163+
var (system, window, panel, button1, button2) = BuildScrolledPanel();
164+
165+
// Scroll down so button2 is visible
166+
panel.ScrollVerticalBy(40);
167+
system.Render.UpdateDisplay();
168+
int scrollBefore = panel.VerticalScrollOffset;
169+
170+
// Simulate HandleClick on button2
171+
window.FocusManager.HandleClick(button2);
172+
173+
Assert.True(window.FocusManager.IsFocused(button2),
174+
"HandleClick on button should focus the button directly");
175+
Assert.Equal(scrollBefore, panel.VerticalScrollOffset);
176+
}
177+
178+
/// <summary>
179+
/// HandleClick on a non-focusable control (e.g. MarkupControl) walks up to
180+
/// the nearest focusable ancestor (SPC). With the fix, SetFocus(SPC, Mouse)
181+
/// focuses SPC directly without delegating to first child — no scroll jump.
182+
/// </summary>
183+
[Fact]
184+
public void HandleClick_OnNonFocusable_FocusesSPC()
185+
{
186+
var panel = new ScrollablePanelControl
187+
{
188+
VerticalAlignment = VerticalAlignment.Fill,
189+
AutoScroll = false
190+
};
191+
192+
var markup = new MarkupControl(new List<string> { "Some text content" });
193+
panel.AddControl(markup);
194+
195+
var button = new ButtonControl { Text = "Button" };
196+
panel.AddControl(button);
197+
198+
var system = TestWindowSystemBuilder.CreateTestSystem(80, 20);
199+
var window = new Window(system)
200+
{
201+
Title = "Test", Left = 0, Top = 0, Width = 80, Height = 20
202+
};
203+
window.AddControl(panel);
204+
system.AddWindow(window);
205+
system.Render.UpdateDisplay();
206+
system.Render.UpdateDisplay();
207+
208+
// Clear auto-focus
209+
window.FocusManager.SetFocus(null, FocusReason.Programmatic);
210+
211+
// HandleClick on non-focusable MarkupControl — walks up to SPC
212+
window.FocusManager.HandleClick(markup);
213+
214+
// SPC should be focused directly (not button via GetInitialFocus delegation)
215+
Assert.True(window.FocusManager.IsFocused(panel),
216+
"HandleClick on non-focusable child should focus the SPC directly (Mouse reason)");
217+
Assert.False(window.FocusManager.IsFocused(button),
218+
"HandleClick should NOT delegate to first child via GetInitialFocus");
219+
}
220+
221+
#endregion
222+
223+
#region Step 4: Tab after mouse click enters normally
224+
225+
/// <summary>
226+
/// After a mouse click on SPC empty space (which focuses SPC directly),
227+
/// Tab should enter SPC's first child normally — NOT enter scroll mode.
228+
/// This verifies the _enterScrollModeOnNextInitialFocus flag removal.
229+
/// </summary>
230+
[Fact]
231+
public void Tab_AfterMouseClick_EntersNormally()
232+
{
233+
var panel = new ScrollablePanelControl
234+
{
235+
VerticalAlignment = VerticalAlignment.Fill,
236+
AutoScroll = false
237+
};
238+
239+
var button1 = new ButtonControl { Text = "First" };
240+
panel.AddControl(button1);
241+
242+
var button2 = new ButtonControl { Text = "Second" };
243+
panel.AddControl(button2);
244+
245+
var system = TestWindowSystemBuilder.CreateTestSystem(80, 20);
246+
var window = new Window(system)
247+
{
248+
Title = "Test", Left = 0, Top = 0, Width = 80, Height = 20
249+
};
250+
window.AddControl(panel);
251+
system.AddWindow(window);
252+
system.Render.UpdateDisplay();
253+
system.Render.UpdateDisplay();
254+
255+
// Simulate: SetFocus(SPC, Mouse) — as if user clicked empty space
256+
window.FocusManager.SetFocus(panel, FocusReason.Mouse);
257+
Assert.True(window.FocusManager.IsFocused(panel),
258+
"SPC should be directly focused after mouse click");
259+
260+
// Now Tab — should enter SPC normally and focus first child
261+
var tabKey = new ConsoleKeyInfo('\t', ConsoleKey.Tab, false, false, false);
262+
window.SwitchFocus(backward: false);
263+
264+
// After Tab from SPC, focus should move to first child (button1)
265+
// or at least not stay on SPC in scroll mode
266+
var focused = window.FocusManager.FocusedControl;
267+
Assert.True(
268+
window.FocusManager.IsFocused(button1) || window.FocusManager.IsFocused(button2),
269+
$"Tab after mouse-focused SPC should enter a child. Focused: {focused?.GetType().Name ?? "null"}");
270+
}
271+
272+
#endregion
273+
}

‎SharpConsoleUI/Controls/ScrollablePanelControl/ScrollablePanelControl.Mouse.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,6 @@ public bool ProcessMouseEvent(MouseEventArgs args)
377377
var emptyClickWindow = (this as IWindowControl).GetParentWindow();
378378
if (emptyClickWindow != null && CanReceiveFocus)
379379
{
380-
_enterScrollModeOnNextInitialFocus = true;
381380
emptyClickWindow.FocusManager.SetFocus(this, FocusReason.Mouse);
382381
}
383382
_lastInternalFocusedChild = null;

‎SharpConsoleUI/Core/FocusManager.cs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,14 @@ public void SetFocus(IFocusableControl? control, FocusReason reason)
4848
// Transparent scopes (CanReceiveFocus=false, e.g. HGrid) are NOT entered here —
4949
// they are handled by the CanReceiveFocus guard below, which rejects them.
5050
// HGrid children are reached via MoveFocus/BuildFlatList traversal instead.
51-
if (control is IFocusScope scope && control.CanReceiveFocus)
51+
if (reason != FocusReason.Mouse
52+
&& control is IFocusScope scope && control.CanReceiveFocus)
5253
{
5354
var backward = false;
5455
var child = scope.GetInitialFocus(backward);
5556
if (child != null && !ReferenceEquals(child, control))
5657
{
57-
EnterOrFocus(child, backward);
58+
EnterOrFocus(child, backward, reason);
5859
return;
5960
}
6061
// child == null, or child == control (self-sentinel) → fall through to focus scope itself
@@ -75,7 +76,7 @@ public void SetFocus(IFocusableControl? control, FocusReason reason)
7576
// Scroll any IScrollableContainer ancestor to show the newly focused control.
7677
// Walk the focus path from the focused control upward; for each scrollable container,
7778
// scroll its direct child in the path into view.
78-
if (control is IWindowControl focusedWc)
79+
if (reason != FocusReason.Mouse && control is IWindowControl focusedWc)
7980
{
8081
var path = FocusPath;
8182
for (int i = path.Count - 1; i >= 1; i--)
@@ -182,18 +183,18 @@ private static void ClearSavedFocusChain(IFocusScope? scope)
182183
}
183184
}
184185

185-
private void EnterOrFocus(IFocusableControl target, bool backward)
186+
private void EnterOrFocus(IFocusableControl target, bool backward, FocusReason reason = FocusReason.Keyboard)
186187
{
187188
if (target is IFocusScope scope)
188189
{
189190
var child = scope.GetInitialFocus(backward);
190191
if (child != null && !ReferenceEquals(child, target))
191192
{
192-
EnterOrFocus(child, backward);
193+
EnterOrFocus(child, backward, reason);
193194
return;
194195
}
195196
}
196-
SetFocus(target, FocusReason.Keyboard);
197+
SetFocus(target, reason);
197198
}
198199

199200
// NOTE: FindInnermostScope starts at control.Container (not the control itself).

0 commit comments

Comments
 (0)