Skip to content

Commit f7a2173

Browse files
nickprotopnickprotop
committed
Fix ListControl scroll indicator logic and ScrollablePanel hit-test/scroll height
ListControl: only reserve a scroll-indicator row when items genuinely overflow the viewport. Previously the -1 was applied unconditionally, causing a blank bottom row for lists that fit entirely. Affects MeasureDOM (bounded Fill branch), PaintDOM (availableContentHeight, item loop guards, fill-empty section), and GetEffectiveVisibleItems in Navigation. ScrollablePanelControl: pass _viewportHeight as MaxHeight to both MeasureChildHeight and CalculateContentHeight (from PaintDOM). This ensures Fill-aligned children measure to their true rendered height rather than the unbounded fallback of Math.Min(10, count), fixing two bugs: - GetClickTargetChild hit-test height now matches rendered height, so clicks on rows beyond the default-10 no longer fall through to the "empty space" path and lose focus. - _contentHeight now reflects the actual rendered content, so the panel can scroll the full range when fixed-height siblings push total content past the viewport.
1 parent ce07864 commit f7a2173

3 files changed

Lines changed: 86 additions & 38 deletions

File tree

‎SharpConsoleUI/Controls/ListControl/ListControl.Navigation.cs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,15 @@ private int GetEffectiveVisibleItems()
3333

3434
if (actualVisibleHeight.HasValue && actualVisibleHeight.Value > 0)
3535
{
36-
// Account for title bar and scroll indicator
36+
// Account for title bar
3737
int titleHeight = string.IsNullOrEmpty(_title) ? 0 : 1;
38-
int scrollIndicatorHeight = 1; // Assume scroll indicator present when scrolling
39-
int availableForItems = Math.Max(1, actualVisibleHeight.Value - titleHeight - scrollIndicatorHeight);
38+
int fullAvailableHeight = actualVisibleHeight.Value - titleHeight;
39+
40+
// Check if all items fit without a scroll indicator
41+
int totalItemsHeight = 0;
42+
for (int j = 0; j < _items.Count; j++) totalItemsHeight += _items[j].Lines.Count;
43+
bool needsScrollIndicator = totalItemsHeight > fullAvailableHeight;
44+
int availableForItems = Math.Max(1, needsScrollIndicator ? fullAvailableHeight - 1 : fullAvailableHeight);
4045

4146
// Count how many items actually fit based on their line heights
4247
// Start from current scroll offset to match what's actually visible

‎SharpConsoleUI/Controls/ListControl/ListControl.Rendering.cs‎

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -200,20 +200,34 @@ public LayoutSize MeasureDOM(LayoutConstraints constraints)
200200
else
201201
{
202202
// When VerticalAlignment.Fill with bounded constraints, use available height
203-
int availableContentHeight = constraints.MaxHeight - titleHeight - _margin.Top - _margin.Bottom - 1;
204-
effectiveMaxVisibleItems = 0;
205-
int heightUsed = 0;
206-
for (int i = scrollOffset; i < _items.Count; i++)
203+
int fullAvailableHeight = constraints.MaxHeight - titleHeight - _margin.Top - _margin.Bottom;
204+
205+
// Check if ALL items fit without needing a scroll indicator
206+
int totalItemsHeight = 0;
207+
for (int j = 0; j < _items.Count; j++) totalItemsHeight += _items[j].Lines.Count;
208+
bool needsScrollIndicator = totalItemsHeight > fullAvailableHeight;
209+
210+
if (!needsScrollIndicator)
211+
{
212+
effectiveMaxVisibleItems = _items.Count;
213+
}
214+
else
207215
{
208-
int itemHeight = _items[i].Lines.Count;
209-
if (heightUsed + itemHeight <= availableContentHeight)
216+
int availableContentHeight = fullAvailableHeight - 1;
217+
effectiveMaxVisibleItems = 0;
218+
int heightUsed = 0;
219+
for (int i = scrollOffset; i < _items.Count; i++)
210220
{
211-
effectiveMaxVisibleItems++;
212-
heightUsed += itemHeight;
221+
int itemHeight = _items[i].Lines.Count;
222+
if (heightUsed + itemHeight <= availableContentHeight)
223+
{
224+
effectiveMaxVisibleItems++;
225+
heightUsed += itemHeight;
226+
}
227+
else break;
213228
}
214-
else break;
229+
effectiveMaxVisibleItems = Math.Max(1, effectiveMaxVisibleItems);
215230
}
216-
effectiveMaxVisibleItems = Math.Max(1, effectiveMaxVisibleItems);
217231
}
218232
}
219233
else
@@ -326,7 +340,13 @@ public void PaintDOM(CharacterBuffer buffer, LayoutRect bounds, LayoutRect clipR
326340
}
327341

328342
// Calculate effective visible items
329-
int availableContentHeight = bounds.Height - _margin.Top - _margin.Bottom - (hasTitle ? 1 : 0) - 1;
343+
int fullAvailableContentHeight = bounds.Height - _margin.Top - _margin.Bottom - (hasTitle ? 1 : 0);
344+
345+
// Check if all items fit without scroll indicator
346+
int totalItemsHeight = 0;
347+
for (int j = 0; j < _items.Count; j++) totalItemsHeight += _items[j].Lines.Count;
348+
bool needsScrollIndicator = totalItemsHeight > fullAvailableContentHeight;
349+
int availableContentHeight = needsScrollIndicator ? fullAvailableContentHeight - 1 : fullAvailableContentHeight;
330350
int effectiveMaxVisibleItems;
331351

332352
if (_maxVisibleItems.HasValue)
@@ -355,14 +375,14 @@ public void PaintDOM(CharacterBuffer buffer, LayoutRect bounds, LayoutRect clipR
355375
int itemsToShow = Math.Min(effectiveMaxVisibleItems, _items.Count - scrollOffset);
356376

357377
// Render each visible item
358-
for (int i = 0; i < itemsToShow && currentY < bounds.Bottom - _margin.Bottom - 1; i++)
378+
for (int i = 0; i < itemsToShow && currentY < bounds.Bottom - _margin.Bottom - (needsScrollIndicator ? 1 : 0); i++)
359379
{
360380
int itemIndex = i + scrollOffset;
361381
if (itemIndex >= _items.Count) break;
362382

363383
List<string> itemLines = _items[itemIndex].Lines;
364384

365-
for (int lineIndex = 0; lineIndex < itemLines.Count && currentY < bounds.Bottom - _margin.Bottom - 1; lineIndex++)
385+
for (int lineIndex = 0; lineIndex < itemLines.Count && currentY < bounds.Bottom - _margin.Bottom - (needsScrollIndicator ? 1 : 0); lineIndex++)
366386
{
367387
if (currentY >= clipRect.Y && currentY < clipRect.Bottom)
368388
{
@@ -472,7 +492,7 @@ public void PaintDOM(CharacterBuffer buffer, LayoutRect bounds, LayoutRect clipR
472492
// Fill empty lines if VerticalAlignment.Fill
473493
if (_verticalAlignment == VerticalAlignment.Fill)
474494
{
475-
int scrollIndicatorY = bounds.Bottom - _margin.Bottom - 1;
495+
int scrollIndicatorY = bounds.Bottom - _margin.Bottom - (needsScrollIndicator ? 1 : 0);
476496
while (currentY < scrollIndicatorY)
477497
{
478498
if (currentY >= clipRect.Y && currentY < clipRect.Bottom)

‎SharpConsoleUI/Controls/ScrollablePanelControl.cs‎

Lines changed: 44 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -734,26 +734,40 @@ public bool ProcessMouseEvent(MouseEventArgs args)
734734
// Handle mouse wheel scrolling
735735
if (_enableMouseWheel)
736736
{
737-
if (args.HasFlag(Drivers.MouseFlags.WheeledUp))
737+
bool isWheel = args.HasFlag(Drivers.MouseFlags.WheeledUp) || args.HasFlag(Drivers.MouseFlags.WheeledDown);
738+
if (isWheel)
738739
{
739-
if (_verticalScrollOffset > 0)
740+
// Forward to focused child first (e.g. ListControl with internal scroll).
741+
// This lets the child handle its own item scrolling before SPC attempts
742+
// to scroll the panel viewport.
743+
if (_focusedChild is IMouseAwareControl childMouse && childMouse.WantsMouseEvents)
740744
{
741-
ScrollVerticalBy(-3);
742-
args.Handled = true;
743-
return true;
745+
if (childMouse.ProcessMouseEvent(args))
746+
return true;
744747
}
745-
return false;
746-
}
747-
else if (args.HasFlag(Drivers.MouseFlags.WheeledDown))
748-
{
749-
int maxScroll = Math.Max(0, _contentHeight - _viewportHeight);
750-
if (_verticalScrollOffset < maxScroll)
748+
749+
// Fall back to SPC viewport scroll
750+
if (args.HasFlag(Drivers.MouseFlags.WheeledUp))
751751
{
752-
ScrollVerticalBy(3);
753-
args.Handled = true;
754-
return true;
752+
if (_verticalScrollOffset > 0)
753+
{
754+
ScrollVerticalBy(-3);
755+
args.Handled = true;
756+
return true;
757+
}
758+
return false;
759+
}
760+
else if (args.HasFlag(Drivers.MouseFlags.WheeledDown))
761+
{
762+
int maxScroll = Math.Max(0, _contentHeight - _viewportHeight);
763+
if (_verticalScrollOffset < maxScroll)
764+
{
765+
ScrollVerticalBy(3);
766+
args.Handled = true;
767+
return true;
768+
}
769+
return false;
755770
}
756-
return false;
757771
}
758772
}
759773

@@ -880,6 +894,10 @@ public bool ProcessMouseEvent(MouseEventArgs args)
880894
focusable.SetFocus(true, FocusReason.Mouse);
881895
if (child is IInteractiveControl interactive)
882896
_focusedChild = interactive;
897+
// Mark panel as focused so ProcessKey routes keys to _focusedChild.
898+
// Tab focus calls SetFocus(true) which sets _hasFocus; mouse click
899+
// skips that path, leaving _hasFocus=false without this line.
900+
_hasFocus = true;
883901
_lastInternalFocusedChild = null;
884902
log?.LogTrace($"ScrollPanel.ProcessMouseEvent: focused child {child.GetType().Name}, _focusedChild={_focusedChild?.GetType().Name}", "Focus");
885903
}
@@ -941,7 +959,8 @@ private int MeasureChildHeight(IWindowControl child, int availableWidth)
941959
{
942960
var childNode = LayoutNodeFactory.CreateSubtree(child);
943961
childNode.IsVisible = true;
944-
var constraints = new LayoutConstraints(1, availableWidth, 1, int.MaxValue);
962+
int maxH = _viewportHeight > 0 ? _viewportHeight : int.MaxValue;
963+
var constraints = new LayoutConstraints(1, availableWidth, 1, maxH);
945964
childNode.Measure(constraints);
946965
return childNode.DesiredSize.Height;
947966
}
@@ -1292,7 +1311,7 @@ public void PaintDOM(CharacterBuffer buffer, LayoutRect bounds, LayoutRect clipR
12921311
_viewportWidth = bounds.Width - _margin.Left - _margin.Right;
12931312

12941313
// Calculate content dimensions from children
1295-
_contentHeight = CalculateContentHeight(_viewportWidth);
1314+
_contentHeight = CalculateContentHeight(_viewportWidth, _viewportHeight);
12961315
_contentWidth = CalculateContentWidth();
12971316

12981317
// AutoScroll: scroll to bottom on any repaint when enabled
@@ -1329,8 +1348,11 @@ public void PaintDOM(CharacterBuffer buffer, LayoutRect bounds, LayoutRect clipR
13291348
var childNode = LayoutNodeFactory.CreateSubtree(child);
13301349
childNode.IsVisible = true;
13311350

1332-
// Measure using full layout pipeline
1333-
var constraints = new LayoutConstraints(1, contentWidth, 1, int.MaxValue);
1351+
// Measure using full layout pipeline.
1352+
// Cap MaxHeight to _viewportHeight so Fill-aligned children measure to the
1353+
// visible area rather than falling back to an arbitrary small default.
1354+
int maxChildHeight = _viewportHeight > 0 ? _viewportHeight : int.MaxValue;
1355+
var constraints = new LayoutConstraints(1, contentWidth, 1, maxChildHeight);
13341356
childNode.Measure(constraints);
13351357
int childHeight = childNode.DesiredSize.Height;
13361358

@@ -1453,9 +1475,10 @@ private void DrawVerticalScrollbar(CharacterBuffer buffer, LayoutRect bounds, Co
14531475
}
14541476
}
14551477

1456-
private int CalculateContentHeight(int viewportWidth)
1478+
private int CalculateContentHeight(int viewportWidth, int maxHeight = 0)
14571479
{
14581480
int availableWidth = viewportWidth;
1481+
int maxH = maxHeight > 0 ? maxHeight : int.MaxValue;
14591482

14601483
// Reserve space for scrollbar if we might need it
14611484
// This is an approximation - we'll recalculate if needed
@@ -1469,7 +1492,7 @@ private int CalculateContentHeight(int viewportWidth)
14691492
{
14701493
var childNode = LayoutNodeFactory.CreateSubtree(child);
14711494
childNode.IsVisible = true;
1472-
var constraints = new LayoutConstraints(1, availableWidth, 1, int.MaxValue);
1495+
var constraints = new LayoutConstraints(1, availableWidth, 1, maxH);
14731496
childNode.Measure(constraints);
14741497
totalHeight += childNode.DesiredSize.Height;
14751498
}

0 commit comments

Comments
 (0)