Skip to content

fix #983: When scrolling down, consider the height of the horizontal … - #1393

Open
Daniel-Tr wants to merge 1 commit into
masterfrom
fix/issue-983-consider-scrollbar-height-in-scrollrange
Open

fix #983: When scrolling down, consider the height of the horizontal …#1393
Daniel-Tr wants to merge 1 commit into
masterfrom
fix/issue-983-consider-scrollbar-height-in-scrollrange

Conversation

@Daniel-Tr

Copy link
Copy Markdown
Collaborator

…scrollbar to calculate whether the bar should be visible or not. This effectively enlarges the view port by one (or more) rows, so that we can calculate the need for the scrollbar as if it was not visible.

Downside of this approach: If the first invisible row would require the scrollbar to be visible, but no other visible row would, we would still show the scrollbar.

This is an alternative proposed solution for #983 and this pull request: #1391

…scrollbar to calculate whether the bar should be visible or not. This effectively enlarges the view port by one (or more) rows, so that we can calculate the need for the scrollbar as if it was not visible.

Downside of this approach: If the first invisible row would require the scrollbar to be visible, but no other visible row would, we would still show the scrollbar.
@Daniel-Tr

Copy link
Copy Markdown
Collaborator Author

@TetzkatLipHoka What do you think about this approach? I think it's a little less intrusive and hides an unneccessary horizontal scrollbar in most of the situations.

@TetzkatLipHoka

Copy link
Copy Markdown
Contributor

Nice approach — I like that it keeps the range semantics untouched and just bridges exactly the flip-flop window (the scroll bar height). I ran the three tests from #1391 against your branch: all green for the flat-list case from the issue.

There is one measurable hole though: GetPreviousSibling walks siblings, not the visually preceding nodes, so a wide child node under a narrow parent is skipped by the look-back. Repro geometry (18 px rows, ~104 px client height): a narrow root with one wide child, followed by 6 narrow roots — scrolling down gets stuck at offset −23 of −40 and the last node stays clipped, i.e. the original #983 symptom returns for hierarchies. The geometry is edge-shaped on purpose: it needs the wide node to sit within scroll-bar-height above the viewport exactly when the bottom clamp kicks in.

Verified fix for that: walk GetPreviousVisible(PrevNode, True) instead of GetPreviousSibling, and use NodeHeight[PrevNode] instead of the raw field (scaled node heights). With those two changes your branch passes both the hierarchy repro (reaches the bottom at −40, no scroll bar flips) and the flat original repro including SB_ENDSCROLL stability and the recompute-on-resize behavior.

With that change I'd be happy to close #1391 in favor of this PR — and glad to contribute its DUnitX tests here, they are approach-neutral (symptom + end-scroll + recompute-on-widen).

🤖 Generated with Claude Code

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.

Vertical Scroll Bar Cannot Scroll To Bottom In Select Circumstances

2 participants