Skip to content

fix: ScrollableContainerNode ignores bounds.Y, overwriting layout above it#301

Merged
Aaronontheweb merged 3 commits into
devfrom
fix/scrollable-container-bounds-y
Jun 17, 2026
Merged

fix: ScrollableContainerNode ignores bounds.Y, overwriting layout above it#301
Aaronontheweb merged 3 commits into
devfrom
fix/scrollable-container-bounds-y

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Owner

Problem

ScrollableContainerNode.Render() passes the raw outer context directly to ScrolledRenderContext, causing content to always render at absolute terminal row 0 regardless of the node's actual position in the layout tree. This overwrites any content above it (headers, labels, etc.).

Root cause: Every other node calls context.CreateSubContext(bounds) first, which offsets the write origin to bounds.{X,Y}. ScrollableContainerNode was the outlier that skipped this step.

Fix

Call context.CreateSubContext(bounds) before constructing ScrolledRenderContext, consistent with the pattern used by every other node:

-var scrolledContext = new ScrolledRenderContext(context, 0, -_scrollOffset, contentWidth, _contentHeight);
+var boundsContext = context.CreateSubContext(bounds);
+var scrolledContext = new ScrolledRenderContext(boundsContext, 0, -_scrollOffset, contentWidth, _contentHeight);

Also updated the scrollbar drawing to use boundsContext instead of the raw context.

Testing

  • ✅ All 1420 existing tests pass (was 1414 + 6 new)
  • ✅ 6 new regression tests in ScrollableContainerNodeBoundsTests.cs:
    • Render_AtOffset_PutsContentAtCorrectRow — core fix test
    • Render_InVerticalLayout_PreservesHeaderAboveScrollable — full layout tree
    • Render_ScrollShiftsContentWithinBounds — scrolling still works
    • Render_WrappedInPanelNode_WorkaroundStillWorks — the workaround still works
    • Render_AtRow0_NoHeaderAbove_RendersContent — edge case: no content above
    • Render_WithScrollbar_DrawsScrollbarInBounds — scrollbar position correct

Closes #300

Fixes #300 — ScrollableContainerNode was passing the raw outer context
directly to ScrolledRenderContext, causing content to always render at
absolute terminal row 0. This overwrote any layout content above the
scrollable node (headers, labels, etc.).

The fix calls context.CreateSubContext(bounds) first, consistent with the
pattern used by every other node (PanelNode, StreamingTextNode,
SelectionListNode, etc.), so inner writes at row 0 actually land at
bounds.Y on screen.

Also includes 6 regression tests verifying:
- Content renders at correct row when scrollable is below a header
- Scrolling still works correctly with the bounds-relative fix
- The PanelNode workaround from #300 still works
- Scrollbar draws in the correct bounds
- Single-node-at-row-0 still renders correctly
…uard

The previous version called ScrollDown() before Measure(), so MaxScroll was 0
and CanScrollDown was false — the three ScrollDown() calls were no-ops and
_scrollOffset stayed 0. The assertion Contains("Line4") then passed trivially
(Line4 is already visible at offset 0) and would have passed against the old
buggy code too.

Now: Measure() first so the scroll takes effect, pin AutoScroll to None so the
offset comes from ScrollDown() (not the auto-tail policy), assert ScrollOffset
== 3, and assert the viewport actually shifted (Line8 visible, Line1/Line2
scrolled out). Verified this fails against the pre-fix behavior (header row 0 is
overwritten by scrolled content).
@Aaronontheweb Aaronontheweb merged commit 19c5039 into dev Jun 17, 2026
13 checks passed
@Aaronontheweb Aaronontheweb deleted the fix/scrollable-container-bounds-y branch June 17, 2026 19:49
@Aaronontheweb Aaronontheweb mentioned this pull request Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ScrollableContainerNode.Render() ignores bounds.Y, overwriting content above it in the layout

1 participant