From 0be1e93bb71148ef23eb1466f5d70f6e9a1144bd Mon Sep 17 00:00:00 2001 From: mirivlad Date: Thu, 28 May 2026 10:03:14 +0800 Subject: [PATCH] fix: keep server list usable with many servers --- .../2026-05-28-server-list-scalability.md | 26 +++--- internal/tui/app.go | 47 +++++++++- internal/tui/app_test.go | 89 +++++++++++++++++++ 3 files changed, 148 insertions(+), 14 deletions(-) diff --git a/docs/superpowers/plans/2026-05-28-server-list-scalability.md b/docs/superpowers/plans/2026-05-28-server-list-scalability.md index fb3d98b..8b64c3f 100644 --- a/docs/superpowers/plans/2026-05-28-server-list-scalability.md +++ b/docs/superpowers/plans/2026-05-28-server-list-scalability.md @@ -15,7 +15,7 @@ **Files:** - Modify: `internal/tui/app_test.go` -- [ ] **Step 1: Add a test for a constrained terminal height** +- [x] **Step 1: Add a test for a constrained terminal height** Add a test that creates more servers than can fit on screen, sets a small terminal size, renders the list, and verifies the selected details and footer are still visible. @@ -54,7 +54,7 @@ func TestServerListViewKeepsDetailsVisibleWithManyServers(t *testing.T) { } ``` -- [ ] **Step 2: Run the focused test and confirm it fails** +- [x] **Step 2: Run the focused test and confirm it fails** Run: @@ -70,7 +70,7 @@ Expected: FAIL because the current table renders every server and can push the d - Modify: `internal/tui/app.go` - Modify: `internal/tui/app_test.go` -- [ ] **Step 1: Add focused tests for visible range calculation** +- [x] **Step 1: Add focused tests for visible range calculation** Add tests for a helper that computes the inclusive start and exclusive end indexes for rendered rows. @@ -101,7 +101,7 @@ func TestVisibleServerRangeKeepsSelectionInsideWindow(t *testing.T) { } ``` -- [ ] **Step 2: Implement `visibleServerRange`** +- [x] **Step 2: Implement `visibleServerRange`** Add a small helper near `selectedServer`. @@ -133,7 +133,7 @@ func visibleServerRange(total, selected, available int) (int, int) { } ``` -- [ ] **Step 3: Run helper tests** +- [x] **Step 3: Run helper tests** Run: @@ -149,7 +149,7 @@ Expected: PASS. - Modify: `internal/tui/app.go` - Modify: `internal/tui/app_test.go` -- [ ] **Step 1: Reserve terminal space for fixed UI blocks** +- [x] **Step 1: Reserve terminal space for fixed UI blocks** Add a helper that decides how many server rows may be rendered while keeping the selected details and footer visible. @@ -168,7 +168,7 @@ func (m *tuiModel) visibleServerRows() int { } ``` -- [ ] **Step 2: Use the visible range in `viewServerList`** +- [x] **Step 2: Use the visible range in `viewServerList`** In `viewServerList`, replace the loop over all servers with a bounded loop: @@ -189,7 +189,7 @@ if len(m.servers) > end-start { } ``` -- [ ] **Step 3: Run long-list regression test** +- [x] **Step 3: Run long-list regression test** Run: @@ -204,7 +204,7 @@ Expected: PASS. **Files:** - Modify: `internal/tui/app_test.go` -- [ ] **Step 1: Add a test for moving selection beyond the first window** +- [x] **Step 1: Add a test for moving selection beyond the first window** Use the existing `m.list.Update` path by sending `tea.KeyDown` messages and confirm the rendered window follows the selected server. @@ -240,7 +240,7 @@ func TestServerListViewScrollsWithSelection(t *testing.T) { } ``` -- [ ] **Step 2: Run TUI tests** +- [x] **Step 2: Run TUI tests** Run: @@ -255,7 +255,7 @@ Expected: PASS. **Files:** - No source edits expected. -- [ ] **Step 1: Run the full test suite** +- [x] **Step 1: Run the full test suite** Run: @@ -265,7 +265,7 @@ env GOCACHE=/tmp/sshkeeper-go-cache go test ./... Expected: all packages pass. -- [ ] **Step 2: Rebuild the project binary** +- [x] **Step 2: Rebuild the project binary** Run: @@ -275,7 +275,7 @@ env GOCACHE=/tmp/sshkeeper-go-cache go build -o bin/sshkeeper . Expected: exit code 0 and updated `bin/sshkeeper`. -- [ ] **Step 3: Commit the implementation** +- [x] **Step 3: Commit the implementation** Run: diff --git a/internal/tui/app.go b/internal/tui/app.go index ff5b5c7..17bb608 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -417,7 +417,9 @@ func (m *tuiModel) viewServerList() string { b.WriteString(helpStyle.Render(" No servers yet. Press Ctrl+A to add one.")) b.WriteString("\n") } else { - for _, server := range m.servers { + selectedIndex := m.list.Index() + start, end := visibleServerRange(len(m.servers), selectedIndex, m.visibleServerRows()) + for _, server := range m.servers[start:end] { marker := " " rowStyle := normalStyle if server.Alias == selectedAlias { @@ -445,6 +447,10 @@ func (m *tuiModel) viewServerList() string { b.WriteString(rowStyle.Render(row)) b.WriteString("\n") } + if len(m.servers) > end-start { + b.WriteString(helpStyle.Render(fmt.Sprintf(" Showing %d-%d of %d", start+1, end, len(m.servers)))) + b.WriteString("\n") + } } b.WriteString("\n") @@ -465,6 +471,45 @@ func (m *tuiModel) selectedServer() *model.Server { return nil } +func (m *tuiModel) visibleServerRows() int { + if m.height <= 0 { + return len(m.servers) + } + + const fixedRows = 16 + rows := m.height - fixedRows + if rows < 3 { + return 3 + } + return rows +} + +func visibleServerRange(total, selected, available int) (int, int) { + if total <= 0 || available <= 0 { + return 0, 0 + } + if available >= total { + return 0, total + } + if selected < 0 { + selected = 0 + } + if selected >= total { + selected = total - 1 + } + + start := selected - available + 1 + if start < 0 { + start = 0 + } + end := start + available + if end > total { + end = total + start = end - available + } + return start, end +} + func (m *tuiModel) viewSelectedServer(server *model.Server) string { displayName := server.DisplayName if displayName == "" { diff --git a/internal/tui/app_test.go b/internal/tui/app_test.go index 8aece7e..486d1b3 100644 --- a/internal/tui/app_test.go +++ b/internal/tui/app_test.go @@ -1,6 +1,7 @@ package tui import ( + "fmt" "strings" "testing" "time" @@ -68,6 +69,94 @@ func TestServerListViewUsesDashboardLayout(t *testing.T) { } } +func TestServerListViewKeepsDetailsVisibleWithManyServers(t *testing.T) { + servers := make([]*model.Server, 45) + for i := range servers { + servers[i] = &model.Server{ + Alias: fmt.Sprintf("server-%02d", i+1), + DisplayName: fmt.Sprintf("Server %02d", i+1), + Host: fmt.Sprintf("host-%02d.example.org", i+1), + Port: 22, + User: "mirivlad", + AuthMethod: model.AuthKey, + LastTestStatus: model.TestUnknown, + } + } + + m := New(servers) + updated, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 18}) + model := updated.(*tuiModel) + + view := model.View() + if !strings.Contains(view, "Server 01") { + t.Fatalf("expected first selected server to be visible:\n%s", view) + } + if !strings.Contains(view, "Selected") { + t.Fatalf("expected selected server details to remain visible:\n%s", view) + } + if !strings.Contains(view, "Enter connect") { + t.Fatalf("expected footer to remain visible:\n%s", view) + } + if count := strings.Count(view, "server-"); count >= len(servers) { + t.Fatalf("expected bounded row rendering, rendered %d server aliases", count) + } +} + +func TestVisibleServerRangeKeepsSelectionInsideWindow(t *testing.T) { + tests := []struct { + name string + total int + selected int + available int + wantStart int + wantEnd int + }{ + {name: "first page", total: 40, selected: 0, available: 10, wantStart: 0, wantEnd: 10}, + {name: "middle page", total: 40, selected: 20, available: 10, wantStart: 11, wantEnd: 21}, + {name: "last page", total: 40, selected: 39, available: 10, wantStart: 30, wantEnd: 40}, + {name: "all fit", total: 5, selected: 3, available: 10, wantStart: 0, wantEnd: 5}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + start, end := visibleServerRange(tt.total, tt.selected, tt.available) + if start != tt.wantStart || end != tt.wantEnd { + t.Fatalf("visibleServerRange() = %d, %d; want %d, %d", start, end, tt.wantStart, tt.wantEnd) + } + }) + } +} + +func TestServerListViewScrollsWithSelection(t *testing.T) { + servers := make([]*model.Server, 45) + for i := range servers { + servers[i] = &model.Server{ + Alias: fmt.Sprintf("server-%02d", i+1), + DisplayName: fmt.Sprintf("Server %02d", i+1), + Host: fmt.Sprintf("host-%02d.example.org", i+1), + Port: 22, + User: "mirivlad", + AuthMethod: model.AuthKey, + } + } + + m := New(servers) + updated, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 18}) + model := updated.(*tuiModel) + for i := 0; i < 20; i++ { + updated, _ = model.Update(tea.KeyMsg{Type: tea.KeyDown}) + model = updated.(*tuiModel) + } + + view := model.View() + if !strings.Contains(view, "Server 21") { + t.Fatalf("expected selected server to be visible after navigation:\n%s", view) + } + if !strings.Contains(view, "Showing") { + t.Fatalf("expected range hint for long server list:\n%s", view) + } +} + func TestEscClosesGroupListBeforeLeavingForm(t *testing.T) { oldGetGroups := GetGroups GetGroups = func() ([]string, error) {