fix: scroll viewport (need to remove logs yet

Raphael Amorim created

Change summary

internal/tui/exp/list/list.go      |  83 +++++++-
internal/tui/exp/list/list_test.go | 305 ++++++++++++++++++++++++++++++++
2 files changed, 376 insertions(+), 12 deletions(-)

Detailed changes

internal/tui/exp/list/list.go 🔗

@@ -1,6 +1,7 @@
 package list
 
 import (
+	"log"
 	"slices"
 	"strings"
 	"sync"
@@ -552,22 +553,32 @@ func (l *list[T]) viewPosition() (int, int) {
 	// View position in the virtual space
 	start, end := 0, 0
 	if l.direction == DirectionForward {
-		start = l.offset
+		// Ensure offset doesn't exceed the maximum valid offset
+		maxOffset := max(0, l.virtualHeight-l.height)
+		actualOffset := min(l.offset, maxOffset)
+		
+		start = actualOffset
 		if l.virtualHeight > 0 {
-			end = min(l.offset+l.height-1, l.virtualHeight-1)
+			end = min(actualOffset+l.height-1, l.virtualHeight-1)
 		} else {
-			end = l.offset + l.height - 1
+			end = actualOffset + l.height - 1
 		}
 	} else {
 		// For backward direction
 		if l.virtualHeight > 0 {
-			end = l.virtualHeight - l.offset - 1
+			// Ensure offset doesn't exceed the maximum valid offset
+			maxOffset := max(0, l.virtualHeight-l.height)
+			actualOffset := min(l.offset, maxOffset)
+			
+			end = l.virtualHeight - actualOffset - 1
 			start = max(0, end-l.height+1)
 		} else {
 			end = 0
 			start = 0
 		}
 	}
+	log.Printf("[viewPosition] direction=%v, offset=%d, height=%d, virtualHeight=%d -> start=%d, end=%d",
+		l.direction, l.offset, l.height, l.virtualHeight, start, end)
 	return start, end
 }
 
@@ -943,10 +954,13 @@ func (l *list[T]) renderVirtualScrolling() string {
 
 	// Calculate viewport bounds
 	viewStart, viewEnd := l.viewPosition()
+	log.Printf("[renderVirtualScrolling] START: viewStart=%d, viewEnd=%d, offset=%d, height=%d, virtualHeight=%d, direction=%v, selectedIndex=%d",
+		viewStart, viewEnd, l.offset, l.height, l.virtualHeight, l.direction, l.selectedIndex)
 
 	// Check if we have any positions calculated
 	if len(l.itemPositions) == 0 {
 		// No positions calculated yet, return empty viewport
+		log.Printf("[renderVirtualScrolling] No item positions calculated yet")
 		return ""
 	}
 
@@ -958,8 +972,10 @@ func (l *list[T]) renderVirtualScrolling() string {
 	}
 
 	itemsLen := l.items.Len()
+	log.Printf("[renderVirtualScrolling] Checking %d items for visibility", itemsLen)
 	for i := 0; i < itemsLen; i++ {
 		if i >= len(l.itemPositions) {
+			log.Printf("[renderVirtualScrolling] Skipping item %d: no position data", i)
 			continue
 		}
 
@@ -969,44 +985,56 @@ func (l *list[T]) renderVirtualScrolling() string {
 		if pos.end >= viewStart && pos.start <= viewEnd {
 			item, ok := l.items.Get(i)
 			if !ok {
+				log.Printf("[renderVirtualScrolling] Item %d not found in items slice", i)
 				continue
 			}
+			log.Printf("[renderVirtualScrolling] Item %d (id=%s) is visible: pos.start=%d, pos.end=%d, height=%d",
+				i, item.ID(), pos.start, pos.end, pos.height)
 			visibleItems = append(visibleItems, struct {
 				item  T
 				pos   itemPosition
 				index int
 			}{item, pos, i})
+		} else {
+			log.Printf("[renderVirtualScrolling] Item %d not visible: pos.start=%d, pos.end=%d (viewport: %d-%d)",
+				i, pos.start, pos.end, viewStart, viewEnd)
 		}
 
 		// Early exit if we've passed the viewport
 		if pos.start > viewEnd {
+			log.Printf("[renderVirtualScrolling] Early exit at item %d: pos.start=%d > viewEnd=%d", i, pos.start, viewEnd)
 			break
 		}
 	}
 
 	// Build the rendered output
+	log.Printf("[renderVirtualScrolling] Found %d visible items", len(visibleItems))
 	var lines []string
 	currentLine := viewStart
 
-	for _, vis := range visibleItems {
+	for idx, vis := range visibleItems {
 		// Get or render the item's view
 		var view string
 		if cached, ok := l.viewCache.Get(vis.item.ID()); ok {
 			view = cached
+			log.Printf("[renderVirtualScrolling] Using cached view for item %d (id=%s)", vis.index, vis.item.ID())
 		} else {
 			view = vis.item.View()
 			l.viewCache.Set(vis.item.ID(), view)
+			log.Printf("[renderVirtualScrolling] Rendered new view for item %d (id=%s)", vis.index, vis.item.ID())
 		}
 
 		itemLines := strings.Split(view, "\n")
+		log.Printf("[renderVirtualScrolling] Item %d has %d lines, currentLine=%d", vis.index, len(itemLines), currentLine)
 
-		// Add gap lines before item if needed (except for first item)
-		if vis.index > 0 && currentLine < vis.pos.start {
+		// Add gap lines before item if needed (except for first visible item)
+		if idx > 0 && currentLine < vis.pos.start {
 			gapLines := vis.pos.start - currentLine
+			log.Printf("[renderVirtualScrolling] Adding %d gap lines before item %d", gapLines, vis.index)
 			for i := 0; i < gapLines; i++ {
 				lines = append(lines, "")
-				currentLine++
 			}
+			currentLine = vis.pos.start
 		}
 
 		// Determine which lines of this item to include
@@ -1014,30 +1042,61 @@ func (l *list[T]) renderVirtualScrolling() string {
 		if vis.pos.start < viewStart {
 			// Item starts before viewport, skip some lines
 			startLine = viewStart - vis.pos.start
+			log.Printf("[renderVirtualScrolling] Item %d starts before viewport, skipping %d lines", vis.index, startLine)
 		}
 
 		// Add the item's visible lines
-		for i := startLine; i < len(itemLines) && currentLine <= viewEnd; i++ {
-			lines = append(lines, itemLines[i])
-			currentLine++
+		linesAdded := 0
+		maxLinesToAdd := len(itemLines) - startLine
+		for i := 0; i < maxLinesToAdd && len(lines) < l.height; i++ {
+			lines = append(lines, itemLines[startLine + i])
+			linesAdded++
+		}
+		
+		// Update currentLine to track our position in virtual space
+		if vis.pos.start < viewStart {
+			// Item started before viewport, we're now at viewStart + linesAdded
+			currentLine = viewStart + linesAdded
+		} else {
+			// Normal case: we're at the item's start position + lines added
+			currentLine = vis.pos.start + linesAdded
 		}
+		
+		log.Printf("[renderVirtualScrolling] Added %d lines from item %d (visible item #%d), currentLine now=%d",
+			linesAdded, vis.index, idx, currentLine)
 	}
 
 	// For content that fits entirely in viewport, don't pad with empty lines
 	// Only pad if we have scrolled or if content is larger than viewport
+	log.Printf("[renderVirtualScrolling] Before padding: %d lines, virtualHeight=%d, height=%d, offset=%d",
+		len(lines), l.virtualHeight, l.height, l.offset)
+	
 	if l.virtualHeight > l.height || l.offset > 0 {
 		// Fill remaining viewport with empty lines if needed
+		initialLen := len(lines)
 		for len(lines) < l.height {
 			lines = append(lines, "")
 		}
+		if len(lines) > initialLen {
+			log.Printf("[renderVirtualScrolling] Added %d padding lines", len(lines)-initialLen)
+		}
 
 		// Trim to viewport height
 		if len(lines) > l.height {
+			log.Printf("[renderVirtualScrolling] Trimming from %d to %d lines", len(lines), l.height)
 			lines = lines[:l.height]
 		}
 	}
 
-	return strings.Join(lines, "\n")
+	result := strings.Join(lines, "\n")
+	resultHeight := lipgloss.Height(result)
+	log.Printf("[renderVirtualScrolling] FINAL: Returning %d lines (height=%d), expected viewport height=%d",
+		len(lines), resultHeight, l.height)
+	if resultHeight < l.height && len(visibleItems) > 0 {
+		log.Printf("[renderVirtualScrolling] WARNING: Rendered fewer lines than viewport! Missing %d lines",
+			l.height-resultHeight)
+	}
+	return result
 }
 
 // AppendItem implements List.

internal/tui/exp/list/list_test.go 🔗

@@ -14,6 +14,311 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestViewPosition(t *testing.T) {
+	t.Parallel()
+	
+	t.Run("forward direction - normal scrolling", func(t *testing.T) {
+		t.Parallel()
+		items := []Item{createItem("test", 1)}
+		l := New(items, WithDirectionForward(), WithSize(20, 10)).(*list[Item])
+		l.virtualHeight = 50
+		
+		// At the top
+		l.offset = 0
+		start, end := l.viewPosition()
+		assert.Equal(t, 0, start)
+		assert.Equal(t, 9, end)
+		
+		// In the middle
+		l.offset = 20
+		start, end = l.viewPosition()
+		assert.Equal(t, 20, start)
+		assert.Equal(t, 29, end)
+		
+		// Near the bottom
+		l.offset = 40
+		start, end = l.viewPosition()
+		assert.Equal(t, 40, start)
+		assert.Equal(t, 49, end)
+		
+		// Past the maximum valid offset (should be clamped)
+		l.offset = 45
+		start, end = l.viewPosition()
+		assert.Equal(t, 40, start) // Clamped to max valid offset
+		assert.Equal(t, 49, end)
+		
+		// Way past the end (should be clamped)
+		l.offset = 100
+		start, end = l.viewPosition()
+		assert.Equal(t, 40, start) // Clamped to max valid offset
+		assert.Equal(t, 49, end)
+	})
+	
+	t.Run("forward direction - edge case with exact fit", func(t *testing.T) {
+		t.Parallel()
+		items := []Item{createItem("test", 1)}
+		l := New(items, WithDirectionForward(), WithSize(20, 10)).(*list[Item])
+		l.virtualHeight = 10
+		
+		l.offset = 0
+		start, end := l.viewPosition()
+		assert.Equal(t, 0, start)
+		assert.Equal(t, 9, end)
+		
+		// Offset beyond valid range should be clamped
+		l.offset = 5
+		start, end = l.viewPosition()
+		assert.Equal(t, 0, start)
+		assert.Equal(t, 9, end)
+	})
+	
+	t.Run("forward direction - content smaller than viewport", func(t *testing.T) {
+		t.Parallel()
+		items := []Item{createItem("test", 1)}
+		l := New(items, WithDirectionForward(), WithSize(20, 10)).(*list[Item])
+		l.virtualHeight = 5
+		
+		l.offset = 0
+		start, end := l.viewPosition()
+		assert.Equal(t, 0, start)
+		assert.Equal(t, 4, end)
+		
+		// Any offset should be clamped to 0
+		l.offset = 10
+		start, end = l.viewPosition()
+		assert.Equal(t, 0, start)
+		assert.Equal(t, 4, end)
+	})
+	
+	t.Run("backward direction - normal scrolling", func(t *testing.T) {
+		t.Parallel()
+		items := []Item{createItem("test", 1)}
+		l := New(items, WithDirectionBackward(), WithSize(20, 10)).(*list[Item])
+		l.virtualHeight = 50
+		
+		// At the bottom (offset 0 in backward mode)
+		l.offset = 0
+		start, end := l.viewPosition()
+		assert.Equal(t, 40, start)
+		assert.Equal(t, 49, end)
+		
+		// In the middle
+		l.offset = 20
+		start, end = l.viewPosition()
+		assert.Equal(t, 20, start)
+		assert.Equal(t, 29, end)
+		
+		// Near the top
+		l.offset = 40
+		start, end = l.viewPosition()
+		assert.Equal(t, 0, start)
+		assert.Equal(t, 9, end)
+		
+		// Past the maximum valid offset (should be clamped)
+		l.offset = 45
+		start, end = l.viewPosition()
+		assert.Equal(t, 0, start)
+		assert.Equal(t, 9, end)
+	})
+	
+	t.Run("backward direction - edge cases", func(t *testing.T) {
+		t.Parallel()
+		items := []Item{createItem("test", 1)}
+		l := New(items, WithDirectionBackward(), WithSize(20, 10)).(*list[Item])
+		l.virtualHeight = 5
+		
+		// Content smaller than viewport
+		l.offset = 0
+		start, end := l.viewPosition()
+		assert.Equal(t, 0, start)
+		assert.Equal(t, 4, end)
+		
+		// Any offset should show all content
+		l.offset = 10
+		start, end = l.viewPosition()
+		assert.Equal(t, 0, start)
+		assert.Equal(t, 4, end)
+	})
+}
+
+// Helper to create a test item with specific height
+func createItem(id string, height int) Item {
+		content := strings.Repeat(id+"\n", height)
+		if height > 0 {
+			content = strings.TrimSuffix(content, "\n")
+		}
+		item := &testItem{
+			id:      id,
+			content: content,
+		}
+	return item
+}
+
+func TestRenderVirtualScrolling(t *testing.T) {
+	t.Parallel()
+	
+	t.Run("should handle partially visible items at top", func(t *testing.T) {
+		t.Parallel()
+		items := []Item{
+			createItem("A", 1),
+			createItem("B", 5),
+			createItem("C", 1),
+			createItem("D", 3),
+		}
+		
+		l := New(items, WithDirectionForward(), WithSize(20, 3)).(*list[Item])
+		execCmd(l, l.Init())
+		
+		// Position B partially visible at top
+		l.offset = 2 // Start viewing from line 2 (middle of B)
+		l.calculateItemPositions()
+		
+		// Item positions: A(0-0), B(1-5), C(6-6), D(7-9)
+		// Viewport: lines 2-4 (height=3)
+		// Should show: lines 2-4 of B (3 lines from B)
+		
+		rendered := l.renderVirtualScrolling()
+		lines := strings.Split(rendered, "\n")
+		assert.Equal(t, 3, len(lines))
+		assert.Equal(t, "B", lines[0])
+		assert.Equal(t, "B", lines[1])
+		assert.Equal(t, "B", lines[2])
+	})
+	
+	t.Run("should handle gaps between items correctly", func(t *testing.T) {
+		t.Parallel()
+		items := []Item{
+			createItem("A", 1),
+			createItem("B", 1),
+			createItem("C", 1),
+		}
+		
+		l := New(items, WithDirectionForward(), WithSize(5, 20), WithGap(1)).(*list[Item])
+		execCmd(l, l.Init())
+		
+		// Item positions: A(0-0), gap(1), B(2-2), gap(3), C(4-4)
+		// Viewport: lines 0-4 (height=5)
+		// Should show all items with gaps
+		
+		rendered := l.renderVirtualScrolling()
+		lines := strings.Split(rendered, "\n")
+		assert.Equal(t, 5, len(lines))
+		assert.Equal(t, "A", lines[0])
+		assert.Equal(t, "", lines[1]) // gap
+		assert.Equal(t, "B", lines[2])
+		assert.Equal(t, "", lines[3]) // gap
+		assert.Equal(t, "C", lines[4])
+	})
+	
+	t.Run("should not show empty lines when scrolled to bottom", func(t *testing.T) {
+		t.Parallel()
+		items := []Item{
+			createItem("A", 2),
+			createItem("B", 2),
+			createItem("C", 2),
+			createItem("D", 2),
+			createItem("E", 2),
+		}
+		
+		l := New(items, WithDirectionForward(), WithSize(4, 20)).(*list[Item])
+		execCmd(l, l.Init())
+		l.calculateItemPositions()
+		
+		// Total height: 10 lines (5 items * 2 lines each)
+		// Scroll to show last 4 lines
+		l.offset = 6
+		
+		rendered := l.renderVirtualScrolling()
+		lines := strings.Split(rendered, "\n")
+		assert.Equal(t, 4, len(lines))
+		// Should show last 2 items completely
+		assert.Equal(t, "D", lines[0])
+		assert.Equal(t, "D", lines[1])
+		assert.Equal(t, "E", lines[2])
+		assert.Equal(t, "E", lines[3])
+	})
+	
+	t.Run("should handle offset at maximum boundary", func(t *testing.T) {
+		t.Parallel()
+		items := []Item{
+			createItem("A", 3),
+			createItem("B", 3),
+			createItem("C", 3),
+			createItem("D", 3),
+		}
+		
+		l := New(items, WithDirectionForward(), WithSize(5, 20)).(*list[Item])
+		execCmd(l, l.Init())
+		l.calculateItemPositions()
+		
+		// Total height: 12 lines
+		// Max valid offset: 12 - 5 = 7
+		l.offset = 7
+		
+		rendered := l.renderVirtualScrolling()
+		lines := strings.Split(rendered, "\n")
+		assert.Equal(t, 5, len(lines))
+		// Should show from line 7 to 11
+		assert.Contains(t, rendered, "C")
+		assert.Contains(t, rendered, "D")
+		
+		// Try setting offset beyond max - should be clamped
+		l.offset = 20
+		rendered = l.renderVirtualScrolling()
+		lines = strings.Split(rendered, "\n")
+		assert.Equal(t, 5, len(lines))
+		// Should still show the same content as offset=7
+		assert.Contains(t, rendered, "C")
+		assert.Contains(t, rendered, "D")
+	})
+}
+
+// testItem is a simple implementation of Item for testing
+type testItem struct {
+	id      string
+	content string
+}
+
+func (t *testItem) ID() string {
+	return t.id
+}
+
+func (t *testItem) View() string {
+	return t.content
+}
+
+func (t *testItem) Selectable() bool {
+	return true
+}
+
+func (t *testItem) Height() int {
+	return lipgloss.Height(t.content)
+}
+
+func (t *testItem) Init() tea.Cmd {
+	return nil
+}
+
+func (t *testItem) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
+	return t, nil
+}
+
+func (t *testItem) SetSize(width, height int) tea.Cmd {
+	return nil
+}
+
+func (t *testItem) GetSize() (int, int) {
+	return 0, lipgloss.Height(t.content)
+}
+
+func (t *testItem) SetFocused(focused bool) tea.Cmd {
+	return nil
+}
+
+func (t *testItem) Focused() bool {
+	return false
+}
+
 func TestList(t *testing.T) {
 	t.Parallel()
 	t.Run("should have correct positions in list that fits the items", func(t *testing.T) {