fix: table.SetCursor cursor render offscreen

When using table.SetCursor to modify the location of the cursor in a table, the viewport wasn't getting updated in the same way as MoveUp/MoveDown which was causing the resulting View() to sometimes not include the cursor. This changeset fixes this by internally converting any SetCursor calls to re-use the logic in MoveUp/MoveDown by calling those functions directly
charmbracelet · Jan 18, 2024 · 9ced347 · 9ced347
1 parent afd7868
commit 9ced347
Show file tree

Hide file tree

Showing 2 changed files with 196 additions and 4 deletions.
diff --git a/table/table.go b/table/table.go
@@ -252,6 +252,7 @@ func (m *Model) UpdateViewport() {
 		m.start = 0
 	}
 	m.end = clamp(m.cursor+m.viewport.Height, m.cursor, len(m.rows))
+
 	for i := m.start; i < m.end; i++ {
 		renderedRows = append(renderedRows, m.renderRow(i))
 	}
@@ -317,14 +318,25 @@ func (m Model) Cursor() int {
 
 // SetCursor sets the cursor position in the table.
 func (m *Model) SetCursor(n int) {
-	m.cursor = clamp(n, 0, len(m.rows)-1)
-	m.UpdateViewport()
+	n = clamp(n, 0, len(m.rows)-1)
+	if m.cursor == n {
+		return
+	}
+
+	if m.cursor < n {
+		m.MoveDown(n - m.cursor)
+		return
+	}
+
+	m.MoveUp(m.cursor - n)
 }
 
 // MoveUp moves the selection up by any number of rows.
 // It can not go above the first row.
 func (m *Model) MoveUp(n int) {
 	m.cursor = clamp(m.cursor-n, 0, len(m.rows)-1)
+	m.UpdateViewport()
+
 	switch {
 	case m.start == 0:
 		m.viewport.SetYOffset(clamp(m.viewport.YOffset, 0, m.cursor))
@@ -333,7 +345,6 @@ func (m *Model) MoveUp(n int) {
 	case m.viewport.YOffset >= 1:
 		m.viewport.YOffset = clamp(m.viewport.YOffset+n, 1, m.viewport.Height)
 	}
-	m.UpdateViewport()
 }
 
 // MoveDown moves the selection down by any number of rows.

diff --git a/table/table_test.go b/table/table_test.go
@@ -1,6 +1,11 @@
 package table
 
-import "testing"
+import (
+	"fmt"
+	"testing"
+
+	"github.com/charmbracelet/lipgloss"
+)
 
 func TestFromValues(t *testing.T) {
 	input := "foo1,bar1\nfoo2,bar2\nfoo3,bar3"
@@ -39,6 +44,182 @@ func TestFromValuesWithTabSeparator(t *testing.T) {
 	}
 }
 
+func TestSetCursorAlwaysVisibleHeight1(t *testing.T) {
+	input := "foo1,bar1\nfoo2,bar2\nfoo3,bar3\nfoo4,bar4"
+	table := New(WithColumns([]Column{{Title: "Foo", Width: 4}, {Title: "Bar", Width: 4}}))
+	table.FromValues(input, ",")
+	table.SetStyles(Styles{
+		Header:   lipgloss.NewStyle(),
+		Cell:     lipgloss.NewStyle(),
+		Selected: lipgloss.NewStyle(),
+	})
+
+	expected := []string{
+		"Foo Bar \nfoo1bar1",
+		"Foo Bar \nfoo2bar2",
+		"Foo Bar \nfoo3bar3",
+	}
+
+	table.SetHeight(1)
+
+	for i := range expected {
+		var (
+			cursor   = i
+			expected = expected[i]
+		)
+		t.Run(fmt.Sprintf("SetCursor(%d) Moving Down", i), func(t *testing.T) {
+			table.SetCursor(cursor)
+			t.Logf("m.cursor = %d", table.cursor)
+			t.Logf("m.start  = %d", table.start)
+			t.Logf("m.end    = %d", table.end)
+			if table.View() != expected {
+				t.Fatalf(`
+expected: %q
+     got: %q`, expected, table.View())
+			}
+		})
+	}
+
+	expected = []string{
+		"Foo Bar \nfoo2bar2",
+		"Foo Bar \nfoo1bar1",
+	}
+
+	for i := range expected {
+		var (
+			cursor   = table.cursor - 1
+			expected = expected[i]
+		)
+		t.Run(fmt.Sprintf("SetCursor(%d) Moving Up", cursor), func(t *testing.T) {
+			table.SetCursor(cursor)
+			t.Logf("m.cursor = %d", table.cursor)
+			t.Logf("m.start  = %d", table.start)
+			t.Logf("m.end    = %d", table.end)
+			if table.View() != expected {
+				t.Fatalf(`
+expected: %q
+     got: %q`, expected, table.View())
+			}
+		})
+	}
+
+	jumps := []struct {
+		i int
+		s string
+	}{
+		{3, "Foo Bar \nfoo4bar4"},
+		{0, "Foo Bar \nfoo1bar1"},
+	}
+
+	for i := range jumps {
+		var (
+			cursor   = jumps[i].i
+			expected = jumps[i].s
+		)
+		t.Run(fmt.Sprintf("SetCursor(%d->%d)", table.cursor, cursor), func(t *testing.T) {
+			table.SetCursor(cursor)
+			t.Logf("m.cursor = %d", table.cursor)
+			t.Logf("m.start  = %d", table.start)
+			t.Logf("m.end    = %d", table.end)
+			if table.View() != expected {
+				t.Fatalf(`
+expected: %q
+     got: %q`, expected, table.View())
+			}
+		})
+	}
+}
+
+func TestSetCursorAlwaysVisibleHeight2(t *testing.T) {
+	input := "foo1,bar1\nfoo2,bar2\nfoo3,bar3\nfoo4,bar4"
+	table := New(WithColumns([]Column{{Title: "Foo", Width: 4}, {Title: "Bar", Width: 4}}))
+	table.FromValues(input, ",")
+	table.SetStyles(Styles{
+		Header:   lipgloss.NewStyle(),
+		Cell:     lipgloss.NewStyle(),
+		Selected: lipgloss.NewStyle(),
+	})
+
+	expected := []string{
+		"Foo Bar \nfoo1bar1\nfoo2bar2",
+		"Foo Bar \nfoo1bar1\nfoo2bar2",
+		"Foo Bar \nfoo2bar2\nfoo3bar3",
+		"Foo Bar \nfoo3bar3\nfoo4bar4",
+	}
+
+	table.SetHeight(2)
+
+	for i := range expected {
+		var (
+			cursor   = i
+			expected = expected[i]
+		)
+		t.Run(fmt.Sprintf("SetCursor(%d) Moving Down", i), func(t *testing.T) {
+			table.SetCursor(cursor)
+			t.Logf("m.cursor = %d", table.cursor)
+			t.Logf("m.start  = %d", table.start)
+			t.Logf("m.end    = %d", table.end)
+			if table.View() != expected {
+				t.Fatalf(`
+expected: %q
+     got: %q`, expected, table.View())
+			}
+		})
+	}
+
+	expected = []string{
+		"Foo Bar \nfoo2bar2\nfoo3bar3",
+		"Foo Bar \nfoo2bar2\nfoo3bar3",
+		"Foo Bar \nfoo1bar1\nfoo2bar2",
+	}
+
+	for i := range expected {
+		var (
+			cursor   = table.cursor - 1
+			expected = expected[i]
+		)
+		t.Run(fmt.Sprintf("SetCursor(%d) Moving Up", cursor), func(t *testing.T) {
+			table.SetCursor(cursor)
+			t.Logf("m.cursor = %d", table.cursor)
+			t.Logf("m.start  = %d", table.start)
+			t.Logf("m.end    = %d", table.end)
+			if table.View() != expected {
+				t.Fatalf(`
+expected: %q
+     got: %q`, expected, table.View())
+			}
+		})
+	}
+
+	jumps := []struct {
+		i int
+		s string
+	}{
+		{3, "Foo Bar \nfoo3bar3\nfoo4bar4"},
+		{0, "Foo Bar \nfoo1bar1\nfoo2bar2"},
+		{2, "Foo Bar \nfoo2bar2\nfoo3bar3"},
+		{0, "Foo Bar \nfoo1bar1\nfoo2bar2"},
+	}
+
+	for i := range jumps {
+		var (
+			cursor   = jumps[i].i
+			expected = jumps[i].s
+		)
+		t.Run(fmt.Sprintf("SetCursor(%d->%d)", table.cursor, cursor), func(t *testing.T) {
+			table.SetCursor(cursor)
+			t.Logf("m.cursor = %d", table.cursor)
+			t.Logf("m.start  = %d", table.start)
+			t.Logf("m.end    = %d", table.end)
+			if table.View() != expected {
+				t.Fatalf(`
+expected: %q
+     got: %q`, expected, table.View())
+			}
+		})
+	}
+}
+
 func deepEqual(a, b []Row) bool {
 	if len(a) != len(b) {
 		return false