From e29401fd8a09339ee6d4447dd406ee88d0fb24ef Mon Sep 17 00:00:00 2001 From: booleanmaybe Date: Thu, 29 Jan 2026 18:00:09 -0500 Subject: [PATCH] fix scroll after move --- controller/plugin.go | 10 ++- controller/plugin_selection_test.go | 127 ++++++++++++++++++++++++++++ main.go | 2 +- model/plugin_config.go | 22 +++++ model/plugin_config_test.go | 72 ++++++++++++++++ view/scrollable_list.go | 5 ++ view/scrollable_list_test.go | 29 +++++++ view/tiki_plugin_view.go | 3 + 8 files changed, 268 insertions(+), 2 deletions(-) diff --git a/controller/plugin.go b/controller/plugin.go index ea9419d..5c2beec 100644 --- a/controller/plugin.go +++ b/controller/plugin.go @@ -119,7 +119,15 @@ func (pc *PluginController) handlePaneSwitch(direction string) bool { tasks := pc.GetFilteredTasksForPane(nextPane) if len(tasks) > 0 { pc.pluginConfig.SetSelectedPane(nextPane) - pc.pluginConfig.ClampSelection(len(tasks)) + // Select the task at top of viewport (scroll offset) rather than keeping stale index + scrollOffset := pc.pluginConfig.GetScrollOffsetForPane(nextPane) + if scrollOffset >= len(tasks) { + scrollOffset = len(tasks) - 1 + } + if scrollOffset < 0 { + scrollOffset = 0 + } + pc.pluginConfig.SetSelectedIndexForPane(nextPane, scrollOffset) return true } switch direction { diff --git a/controller/plugin_selection_test.go b/controller/plugin_selection_test.go index 0ca442b..57920d3 100644 --- a/controller/plugin_selection_test.go +++ b/controller/plugin_selection_test.go @@ -1,6 +1,7 @@ package controller import ( + "fmt" "testing" "github.com/boolean-maybe/tiki/model" @@ -135,3 +136,129 @@ func TestEnsureFirstNonEmptyPaneSelectionNoTasks(t *testing.T) { t.Fatalf("expected selected index 2, got %d", pluginConfig.GetSelectedIndexForPane(1)) } } + +func TestPaneSwitchSelectsTopOfViewport(t *testing.T) { + taskStore := store.NewInMemoryStore() + // Create tasks for two panes + for i := 1; i <= 10; i++ { + status := task.StatusReady + if i > 5 { + status = task.StatusInProgress + } + if err := taskStore.CreateTask(&task.Task{ + ID: fmt.Sprintf("T-%d", i), + Title: "Task", + Status: status, + Type: task.TypeStory, + }); err != nil { + t.Fatalf("create task: %v", err) + } + } + + readyFilter, err := filter.ParseFilter("status = 'ready'") + if err != nil { + t.Fatalf("parse filter: %v", err) + } + inProgressFilter, err := filter.ParseFilter("status = 'in_progress'") + if err != nil { + t.Fatalf("parse filter: %v", err) + } + + pluginDef := &plugin.TikiPlugin{ + BasePlugin: plugin.BasePlugin{ + Name: "TestPlugin", + }, + Panes: []plugin.TikiPane{ + {Name: "Ready", Columns: 1, Filter: readyFilter}, + {Name: "InProgress", Columns: 1, Filter: inProgressFilter}, + }, + } + pluginConfig := model.NewPluginConfig("TestPlugin") + pluginConfig.SetPaneLayout([]int{1, 1}) + + // Start in pane 0 (Ready), with selection at index 2 + pluginConfig.SetSelectedPane(0) + pluginConfig.SetSelectedIndexForPane(0, 2) + + // Simulate that pane 1 has been scrolled to offset 3 + pluginConfig.SetScrollOffsetForPane(1, 3) + + pc := NewPluginController(taskStore, pluginConfig, pluginDef, nil) + + // Navigate right to pane 1 + pc.HandleAction(ActionNavRight) + + // Should be in pane 1 + if pluginConfig.GetSelectedPane() != 1 { + t.Fatalf("expected selected pane 1, got %d", pluginConfig.GetSelectedPane()) + } + + // Selection should be at scroll offset (top of viewport), not stale index + if pluginConfig.GetSelectedIndexForPane(1) != 3 { + t.Errorf("expected selection at scroll offset 3, got %d", pluginConfig.GetSelectedIndexForPane(1)) + } +} + +func TestPaneSwitchClampsScrollOffsetToTaskCount(t *testing.T) { + taskStore := store.NewInMemoryStore() + // Create 3 tasks in pane 1 only + for i := 1; i <= 3; i++ { + if err := taskStore.CreateTask(&task.Task{ + ID: fmt.Sprintf("T-%d", i), + Title: "Task", + Status: task.StatusInProgress, + Type: task.TypeStory, + }); err != nil { + t.Fatalf("create task: %v", err) + } + } + + // Pane 0 is empty, pane 1 has 3 tasks + emptyFilter, err := filter.ParseFilter("status = 'ready'") + if err != nil { + t.Fatalf("parse filter: %v", err) + } + inProgressFilter, err := filter.ParseFilter("status = 'in_progress'") + if err != nil { + t.Fatalf("parse filter: %v", err) + } + + pluginDef := &plugin.TikiPlugin{ + BasePlugin: plugin.BasePlugin{ + Name: "TestPlugin", + }, + Panes: []plugin.TikiPane{ + {Name: "Empty", Columns: 1, Filter: emptyFilter}, + {Name: "InProgress", Columns: 1, Filter: inProgressFilter}, + }, + } + pluginConfig := model.NewPluginConfig("TestPlugin") + pluginConfig.SetPaneLayout([]int{1, 1}) + + // Start in pane 1 + pluginConfig.SetSelectedPane(1) + pluginConfig.SetSelectedIndexForPane(1, 0) + + // Set a stale scroll offset that exceeds the task count + pluginConfig.SetScrollOffsetForPane(1, 10) + + pc := NewPluginController(taskStore, pluginConfig, pluginDef, nil) + + // Navigate left (to empty pane, will skip to... well, nowhere) + // Then try to go right from a fresh setup + pluginConfig.SetSelectedPane(0) + pluginConfig.SetScrollOffsetForPane(1, 10) // stale offset > task count + + pc.HandleAction(ActionNavRight) + + // Should be in pane 1 + if pluginConfig.GetSelectedPane() != 1 { + t.Fatalf("expected selected pane 1, got %d", pluginConfig.GetSelectedPane()) + } + + // Selection should be clamped to last valid index (2, since 3 tasks) + selectedIdx := pluginConfig.GetSelectedIndexForPane(1) + if selectedIdx < 0 || selectedIdx >= 3 { + t.Errorf("expected selection clamped to valid range [0,2], got %d", selectedIdx) + } +} diff --git a/main.go b/main.go index 24e653c..3d7097a 100644 --- a/main.go +++ b/main.go @@ -147,4 +147,4 @@ Usage: Run 'tiki init' to initialize this repository. `) -} \ No newline at end of file +} diff --git a/model/plugin_config.go b/model/plugin_config.go index 32bdcbe..ba6bcaa 100644 --- a/model/plugin_config.go +++ b/model/plugin_config.go @@ -18,6 +18,7 @@ type PluginConfig struct { selectedPane int selectedIndices []int paneColumns []int + scrollOffsets []int // per-pane viewport position (top visible row) preSearchPane int preSearchIndices []int viewMode ViewMode // compact or expanded display @@ -60,6 +61,7 @@ func (pc *PluginConfig) SetPaneLayout(columns []int) { pc.paneColumns = normalizePaneColumns(columns) pc.selectedIndices = ensureSelectionLength(pc.selectedIndices, len(pc.paneColumns)) pc.preSearchIndices = ensureSelectionLength(pc.preSearchIndices, len(pc.paneColumns)) + pc.scrollOffsets = ensureSelectionLength(pc.scrollOffsets, len(pc.paneColumns)) if pc.selectedPane < 0 || pc.selectedPane >= len(pc.paneColumns) { pc.selectedPane = 0 @@ -125,6 +127,26 @@ func (pc *PluginConfig) SetSelectedIndexForPane(pane int, idx int) { pc.notifyListeners() } +// GetScrollOffsetForPane returns the scroll offset (top visible row) for a pane. +func (pc *PluginConfig) GetScrollOffsetForPane(pane int) int { + pc.mu.RLock() + defer pc.mu.RUnlock() + if pane < 0 || pane >= len(pc.scrollOffsets) { + return 0 + } + return pc.scrollOffsets[pane] +} + +// SetScrollOffsetForPane sets the scroll offset for a specific pane. +func (pc *PluginConfig) SetScrollOffsetForPane(pane int, offset int) { + pc.mu.Lock() + defer pc.mu.Unlock() + if pane < 0 || pane >= len(pc.scrollOffsets) { + return + } + pc.scrollOffsets[pane] = offset +} + func (pc *PluginConfig) SetSelectedPaneAndIndex(pane int, idx int) { pc.mu.Lock() if pane < 0 || pane >= len(pc.selectedIndices) { diff --git a/model/plugin_config_test.go b/model/plugin_config_test.go index 3d57695..ea16d92 100644 --- a/model/plugin_config_test.go +++ b/model/plugin_config_test.go @@ -625,3 +625,75 @@ func TestPluginConfig_GridNavigation_AllCorners(t *testing.T) { }) } } + +func TestPluginConfig_ScrollOffset(t *testing.T) { + pc := NewPluginConfig("test") + pc.SetPaneLayout([]int{1, 1, 1}) // 3 panes + + // Initial scroll offsets should be 0 + for pane := 0; pane < 3; pane++ { + if offset := pc.GetScrollOffsetForPane(pane); offset != 0 { + t.Errorf("initial GetScrollOffsetForPane(%d) = %d, want 0", pane, offset) + } + } + + // Set scroll offset for pane 1 + pc.SetScrollOffsetForPane(1, 5) + if offset := pc.GetScrollOffsetForPane(1); offset != 5 { + t.Errorf("GetScrollOffsetForPane(1) = %d, want 5", offset) + } + + // Other panes should be unaffected + if offset := pc.GetScrollOffsetForPane(0); offset != 0 { + t.Errorf("GetScrollOffsetForPane(0) = %d, want 0", offset) + } + if offset := pc.GetScrollOffsetForPane(2); offset != 0 { + t.Errorf("GetScrollOffsetForPane(2) = %d, want 0", offset) + } + + // Set scroll offset for pane 2 + pc.SetScrollOffsetForPane(2, 10) + if offset := pc.GetScrollOffsetForPane(2); offset != 10 { + t.Errorf("GetScrollOffsetForPane(2) = %d, want 10", offset) + } +} + +func TestPluginConfig_ScrollOffset_OutOfBounds(t *testing.T) { + pc := NewPluginConfig("test") + pc.SetPaneLayout([]int{1, 1}) // 2 panes + + // Getting out of bounds should return 0 + if offset := pc.GetScrollOffsetForPane(-1); offset != 0 { + t.Errorf("GetScrollOffsetForPane(-1) = %d, want 0", offset) + } + if offset := pc.GetScrollOffsetForPane(5); offset != 0 { + t.Errorf("GetScrollOffsetForPane(5) = %d, want 0", offset) + } + + // Setting out of bounds should be a no-op (not panic) + pc.SetScrollOffsetForPane(-1, 10) + pc.SetScrollOffsetForPane(5, 10) + + // Valid panes should still be 0 + if offset := pc.GetScrollOffsetForPane(0); offset != 0 { + t.Errorf("GetScrollOffsetForPane(0) = %d after out-of-bounds set, want 0", offset) + } +} + +func TestPluginConfig_ScrollOffset_PreservedOnLayoutChange(t *testing.T) { + pc := NewPluginConfig("test") + pc.SetPaneLayout([]int{1, 1, 1}) + + // Set scroll offsets + pc.SetScrollOffsetForPane(0, 3) + pc.SetScrollOffsetForPane(1, 7) + + // Change layout to same size - should preserve offsets + pc.SetPaneLayout([]int{2, 2, 2}) + if offset := pc.GetScrollOffsetForPane(0); offset != 3 { + t.Errorf("GetScrollOffsetForPane(0) after same-size layout change = %d, want 3", offset) + } + if offset := pc.GetScrollOffsetForPane(1); offset != 7 { + t.Errorf("GetScrollOffsetForPane(1) after same-size layout change = %d, want 7", offset) + } +} diff --git a/view/scrollable_list.go b/view/scrollable_list.go index 963e47f..0efb6ff 100644 --- a/view/scrollable_list.go +++ b/view/scrollable_list.go @@ -53,6 +53,11 @@ func (s *ScrollableList) SetSelection(index int) { s.ensureSelectionVisible() } +// GetScrollOffset returns the current scroll offset (first visible item index) +func (s *ScrollableList) GetScrollOffset() int { + return s.scrollOffset +} + // ensureSelectionVisible adjusts scrollOffset to keep selectionIndex in view func (s *ScrollableList) ensureSelectionVisible() { // If no items, preserve scrollOffset (will be adjusted after items are added) diff --git a/view/scrollable_list_test.go b/view/scrollable_list_test.go index 53809f6..6bcf6a1 100644 --- a/view/scrollable_list_test.go +++ b/view/scrollable_list_test.go @@ -356,3 +356,32 @@ func TestLargeItemHeight(t *testing.T) { t.Errorf("At item 6 (moved up from 7), scrollOffset should be 6 (showing 6-8), got %d", list.scrollOffset) } } + +// TestGetScrollOffset tests the GetScrollOffset method +func TestGetScrollOffset(t *testing.T) { + list := createTestList(10, 5) + setListHeight(list, 25) // Can show 5 items + + // Initial scroll offset should be 0 + if offset := list.GetScrollOffset(); offset != 0 { + t.Errorf("Initial GetScrollOffset() = %d, want 0", offset) + } + + // After scrolling down, GetScrollOffset should reflect the internal state + list.SetSelection(7) // Should scroll to show items 3-7 + if offset := list.GetScrollOffset(); offset != 3 { + t.Errorf("After SetSelection(7), GetScrollOffset() = %d, want 3", offset) + } + + // Scroll to bottom + list.SetSelection(9) // Should scroll to show items 5-9 + if offset := list.GetScrollOffset(); offset != 5 { + t.Errorf("After SetSelection(9), GetScrollOffset() = %d, want 5", offset) + } + + // GetScrollOffset should match internal scrollOffset + if list.GetScrollOffset() != list.scrollOffset { + t.Errorf("GetScrollOffset() = %d doesn't match internal scrollOffset = %d", + list.GetScrollOffset(), list.scrollOffset) + } +} diff --git a/view/tiki_plugin_view.go b/view/tiki_plugin_view.go index 974b082..720d02a 100644 --- a/view/tiki_plugin_view.go +++ b/view/tiki_plugin_view.go @@ -171,6 +171,9 @@ func (pv *PluginView) refresh() { } else { paneContainer.SetSelection(-1) } + + // Sync scroll offset from view to model for later pane navigation + pv.pluginConfig.SetScrollOffsetForPane(paneIdx, paneContainer.GetScrollOffset()) } }