Unify TUI windowing on pane.Window, drop the dashboard scrollWindow (closes #8)#10
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR consolidates TUI viewport/windowing behavior by introducing a shared pane.Window helper and routing the dashboard modal/detail scrolling through it, while also removing the previously-unused Filterable/Sortable pane APIs.
Changes:
- Added
pane.Window(mode, lines, offset, sel, height)as a shared windowing routine with themed overflow affordances. - Updated dashboard modal + detail rendering to use
pane.Windowand deleted the dashboard’s duplicatescrollWindow/keepSelVisiblehelpers (and their dedicated test). - Removed unused pane filter/sort interfaces and related state/logic from
pane.Pane, and trimmed associated tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| CHANGELOG.md | Documents the Unreleased change to modal/detail scrolling + API removals. |
| app/internal/tui/pane/window.go | Introduces the shared exported windowing helper and affordance rendering. |
| app/internal/tui/pane/window_test.go | Adds unit tests for the new shared windowing helper. |
| app/internal/tui/pane/pane.go | Removes filter/sort APIs and updates affordance rendering; still contains its own windowing path. |
| app/internal/tui/pane/pane_test.go | Removes tests tied to the dropped filter/sort behavior and simplifies fake sources. |
| app/internal/tui/dashboard/modal.go | Routes modal body windowing through pane.Window and deletes local helpers. |
| app/internal/tui/dashboard/detail.go | Routes detail-body windowing through pane.Window. |
| app/internal/tui/dashboard/modal_selection_test.go | Deletes tests for removed dashboard keepSelVisible helper. |
Comment on lines
+18
to
+42
| total := len(lines) | ||
| if height < 1 { | ||
| height = 1 | ||
| } | ||
| if total <= height { | ||
| return append([]string(nil), lines...), 0 | ||
| } | ||
| // Keep the selection clear of the row an affordance will overwrite, so the | ||
| // cursor never hides under a "▲/▼ more" marker. | ||
| if sel >= 0 { | ||
| if sel <= offset { | ||
| offset = sel - 1 | ||
| } else if sel >= offset+height-1 { | ||
| offset = sel - height + 2 | ||
| } | ||
| } | ||
| offset = clampOffset(offset, total, height) | ||
| out := append([]string(nil), lines[offset:offset+height]...) | ||
| if offset > 0 { | ||
| out[0] = affordanceLine(m, offset+1, total, true) | ||
| } | ||
| if offset+height < total { | ||
| out[len(out)-1] = affordanceLine(m, offset+height, total, false) | ||
| } | ||
| return out, offset |
Comment on lines
11
to
15
| // affordances. Capabilities are opt-in interfaces so a static panel gets neither | ||
| // filter nor sort. | ||
| package pane | ||
|
|
||
| import ( |
| and the dashboard's duplicate `scrollWindow`/`keepSelVisible` are gone — one windowing | ||
| implementation for the whole TUI, so the modals show the same `▲/▼ more · y/total` | ||
| affordance as the top view. The pane's unused `Filterable`/`Sortable` API was dropped; | ||
| the dashboard keeps its own filter/sort idiom. No user-facing change (closes #8). |
| t.Fatalf("sel %d sits on the bottom ▼more row (off=%d)", sel, off) | ||
| } | ||
| } | ||
| } |
Comment on lines
242
to
246
| out := append([]string(nil), lines[offset:offset+height]...) | ||
| out = p.markCursor(m, out, offset) | ||
| if offset > 0 { | ||
| out[0] = p.affordance(m, offset, offset+1, total, true) | ||
| out[0] = affordanceLine(m, offset+1, total, true) | ||
| } |
kinncj
added a commit
that referenced
this pull request
Jul 3, 2026
) Fix the height==1 edge-overwrite bug (viewports <3 rows now show content, not a lone marker), route Pane.window through the shared Window so the cursor is never overwritten at a scrolled edge, and fix stale docs. Addresses the #10 review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #8.
What
The dashboard modals + detail view carried their own
scrollWindow/keepSelVisible, a duplicate of the windowing thepanepackage already does. This routes them through a single sharedpane.Window, deletes the dashboard copies, and removes the pane'sFilterable/SortableAPI that had no consumer.Changes
pane.Window(m, lines, offset, sel, height)— one exported windowing routine (clamp + keep-selection-visible + themed▲/▼ more · y/totalaffordance).Pane/Groupand the dashboard both use it.detail.go+modal.gonow callpane.Window;scrollWindowandkeepSelVisibleare deleted. Modals now show the same affordance as the top view.Filterable/Sortableinterfaces and the/-filter + sort-cycle code — nothing consumed them (the dashboard's own filter/sort idiom won). Recoverable from history if a future full-pane migration wants it.window(different behaviour — out of scope).Not changed
No user-facing behaviour. Log/command/process filters and top sort work exactly as before.
Tests
pane.Windowtests (fit, overflow affordances, selection-stays-visible invariant).▼ more · 8/18).go vetclean.