Skip to content

feat(ui): vertical scrollbar thumb on navigation pane right border#152

Merged
umputun merged 2 commits intomasterfrom
feat/navigation-scrollbar
Apr 26, 2026
Merged

feat(ui): vertical scrollbar thumb on navigation pane right border#152
umputun merged 2 commits intomasterfrom
feat/navigation-scrollbar

Conversation

@umputun
Copy link
Copy Markdown
Owner

@umputun umputun commented Apr 26, 2026

extend the existing diff-pane vertical scrollbar thumb (introduced in #151) so the same thumb glyph also appears on the right border of the navigation pane (file tree and markdown TOC) when the content is taller than the visible window.

implementation

  • pull the post-render row-substitution math out of applyScrollbar into a shared applyPaneScrollbar(scrollbarSpec) helper. callers populate the spec from their own scroll-state source — applyScrollbar reads bubbletea viewport state, applyNavigationScrollbar reads sidepane.ScrollState from the file tree or TOC after Render() runs EnsureVisible
  • new sidepane.ScrollState{Total, Offset} struct exposed by FileTree.ScrollState() and TOC.ScrollState(). for TOC, when the diff pane is focused, Render aligns the offset to activeSection rather than the cursor, so ScrollState() reflects what the user actually sees
  • two pre-content-row constants: diffScrollbarFirstViewportRow = 2 (top border + single-line header) and navigationScrollbarFirstViewportRow = 1 (top border only). both panes share the shape-check + thumb-rune-replacement logic
  • ANSI envelope preservation is identical to the diff path: strings.LastIndex finds the right-border , the slice replacement swaps only that rune, and the thumb's added bold-SGR wrap (\x1b[1m...\x1b[22m) does not disturb the surrounding border fg/bg bytes
  • single-file mode without TOC and the tree-hidden toggle both go through treePaneHidden() and produce no navigation pane, so no scrollbar is applied there

testing

  • direct unit tests for FileTree.ScrollState and TOC.ScrollState, including the activeSection-driven offset path used when the diff pane has focus
  • TestApplyNavigationScrollbar covers top / bottom / fits-in-viewport, plus vh=1, overscrolled offset, empty input, proportional thumb sizing across YOffsets, and the unexpected-line-count bail
  • view-level tests assert the thumb appears on a long file tree, on a long markdown TOC, with both paneTree and paneDiff focus (different inactive-border ANSI envelope), and shifts position when the cursor moves to the last entry

reviewed via /code-review-loop (5 claude agents + codex GPT-5 adversarial). codex verdict: approve, 0 findings.

umputun added 2 commits April 26, 2026 02:27
Add direct unit tests for FileTree.ScrollState and TOC.ScrollState
(including the activeSection-driven offset path used when diff has
focus). Extend TestApplyNavigationScrollbar with vh=1, overscrolled
offset, empty input, proportional thumb sizing, and the
unexpected-line-count bail. Add paneDiff-focus regression for the
navigation thumb to cover the inactive-border ANSI envelope.

Document scrollbarSpec field semantics, point applyScrollbar /
applyNavigationScrollbar wrappers at applyPaneScrollbar's contract,
and name the ScrollState freshness precondition. Update
ARCHITECTURE.md interface method counts (15->17, 7->9) for the new
ScrollState method on FileTreeComponent / TOCComponent.
Copilot AI review requested due to automatic review settings April 26, 2026 07:47
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Extends the existing post-render vertical scrollbar thumb rendering so the same right-border indicator can appear on the navigation pane (file tree / markdown TOC) when that pane is scrollable, matching the diff pane’s behavior.

Changes:

  • Refactors diff scrollbar logic into a shared applyPaneScrollbar(scrollbarSpec) and adds applyNavigationScrollbar using sidepane scroll state.
  • Exposes sidepane.ScrollState{Total, Offset} via FileTree.ScrollState() and TOC.ScrollState(), and threads that state through View()/renderTwoPaneLayout.
  • Adds/extends unit and view-level tests covering navigation scrollbar placement, sizing, and focus-style ANSI envelope behavior; updates docs/README/CLAUDE.md accordingly.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
docs/ARCHITECTURE.md Updates UI pipeline/docs to include navigation scrollbar post-processing and interface method counts.
app/ui/view_test.go Adds view-level assertions for navigation-pane thumbs (tree + TOC) and updates constants.
app/ui/view.go Threads left-pane scroll state into two-pane rendering and applies navigation scrollbar post-render.
app/ui/sidepane/toc_test.go Adds tests validating TOC scroll-state reporting (incl. active-section-driven offset behavior).
app/ui/sidepane/toc.go Exposes TOC scroll state via ScrollState().
app/ui/sidepane/sidepane.go Introduces shared ScrollState struct for sidepane components.
app/ui/sidepane/filetree_test.go Adds tests validating file tree scroll-state reporting.
app/ui/sidepane/filetree.go Exposes file tree scroll state via ScrollState().
app/ui/scrollbar_test.go Adds navigation scrollbar unit tests and updates diff scrollbar constant references.
app/ui/scrollbar.go Refactors scrollbar thumb logic into shared helper; adds navigation scrollbar support + new constants.
app/ui/model.go Extends FileTree/TOC component interfaces with ScrollState().
README.md Updates feature bullet to reflect scrollbar thumbs on diff/tree/TOC panes.
CLAUDE.md Updates gotcha/docs for scrollbar behavior across diff + navigation panes and new constants.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@umputun umputun merged commit 3e142a2 into master Apr 26, 2026
9 checks passed
@umputun umputun deleted the feat/navigation-scrollbar branch April 26, 2026 07:53
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.

2 participants