From 3e18da56ec60fa96c497aee5c49d962a534ccc77 Mon Sep 17 00:00:00 2001 From: Kujtim Hoxha Date: Mon, 21 Jul 2025 09:56:05 +0200 Subject: [PATCH] chore: more tests --- internal/tui/exp/list/list.go | 82 ++++++-- internal/tui/exp/list/list_test.go | 197 ++++++++++++++---- .../should_move_at_max_to_the_top.golden | 2 +- .../TestBackwardList/should_move_up.golden | 2 +- .../TestBackwardList/within_height.golden | 2 - ...> should_move_at_max_to_the_bottom.golden} | 2 +- .../TestForwardList/should_move_down.golden | 2 +- .../TestForwardList/within_height.golden | 2 - ...kip_none_selectable_items_initially.golden | 6 + 9 files changed, 239 insertions(+), 58 deletions(-) rename internal/tui/exp/list/testdata/TestForwardList/{should_move_at_max_to_the_top.golden => should_move_at_max_to_the_bottom.golden} (58%) create mode 100644 internal/tui/exp/list/testdata/TestListSelection/should_skip_none_selectable_items_initially.golden diff --git a/internal/tui/exp/list/list.go b/internal/tui/exp/list/list.go index e9e304fac8463cfce42dc02e71962879abaa5643..92c9a38c2a2fbd4c6338a8873e73450bf64c8d70 100644 --- a/internal/tui/exp/list/list.go +++ b/internal/tui/exp/list/list.go @@ -1,6 +1,7 @@ package list import ( + "fmt" "strings" tea "github.com/charmbracelet/bubbletea/v2" @@ -132,19 +133,20 @@ func (l *list) View() string { view := l.rendered lines := strings.Split(view, "\n") - start, end := l.viewPosition(len(lines)) - lines = lines[start:end] + start, end := l.viewPosition() + lines = lines[start : end+1] return strings.Join(lines, "\n") } -func (l *list) viewPosition(total int) (int, int) { +func (l *list) viewPosition() (int, int) { start, end := 0, 0 + renderedLines := lipgloss.Height(l.rendered) - 1 if l.direction == Forward { start = max(0, l.offset) - end = min(l.offset+l.listHeight(), total) + end = min(l.offset+l.listHeight()-1, renderedLines) } else { - start = max(0, total-l.offset-l.listHeight()) - end = max(0, total-l.offset) + start = max(0, renderedLines-l.offset-l.listHeight()+1) + end = max(0, renderedLines-l.offset) } return start, end } @@ -200,20 +202,71 @@ func (l *list) decrementOffset(n int) { } } -func (l *list) MoveUp(n int) { +// changeSelectedWhenNotVisible is called so we make sure we move to the next available selected that is visible +func (l *list) changeSelectedWhenNotVisible() tea.Cmd { + var cmds []tea.Cmd + start, end := l.viewPosition() + currentPosition := 0 + itemWithinView := NotFound + needsMove := false + + for i, item := range l.items { + rendered := l.renderedItems[i] + itemStart := currentPosition + // we remove 1 so that we actually have the row, e.x 1 row => height 1 => start 0, end 0 + itemEnd := itemStart + rendered.height - 1 + if itemStart >= start && itemEnd <= end { + itemWithinView = i + } + if item.ID() == l.selectedItem { + // item is completely above the viewport + if itemStart < start && itemEnd < start { + needsMove = true + } + // item is completely below the viewport + if itemStart > end && itemEnd > end { + needsMove = true + } + if needsMove { + if focusable, ok := item.(layout.Focusable); ok { + cmds = append(cmds, focusable.Blur()) + } + l.renderedItems[i] = l.renderItem(item) + } else { + return nil + } + } + if itemWithinView != NotFound && needsMove { + newSelection := l.items[itemWithinView] + l.selectedItem = newSelection.ID() + if focusable, ok := newSelection.(layout.Focusable); ok { + cmds = append(cmds, focusable.Focus()) + } + l.renderedItems[itemWithinView] = l.renderItem(newSelection) + break + } + currentPosition += rendered.height + l.gap + } + l.renderView() + return tea.Batch(cmds...) +} + +func (l *list) MoveUp(n int) tea.Cmd { if l.direction == Forward { l.decrementOffset(n) } else { l.incrementOffset(n) } + return l.changeSelectedWhenNotVisible() } -func (l *list) MoveDown(n int) { +func (l *list) MoveDown(n int) tea.Cmd { if l.direction == Forward { l.incrementOffset(n) } else { l.decrementOffset(n) } + return l.changeSelectedWhenNotVisible() } func (l *list) firstSelectableItemBefore(inx int) int { @@ -239,10 +292,10 @@ func (l *list) moveToSelected() { return } currentPosition := 0 - start, end := l.viewPosition(lipgloss.Height(l.rendered)) + start, end := l.viewPosition() for _, item := range l.renderedItems { if item.id == l.selectedItem { - if start <= currentPosition && currentPosition <= end { + if start <= currentPosition && (currentPosition+item.height) <= end { return } // we need to go up @@ -354,7 +407,7 @@ func (l *list) renderForward() tea.Cmd { currentIndex := 0 for i, item := range l.items { currentIndex = i - if currentHeight > l.listHeight() { + if currentHeight-1 > l.listHeight() { break } rendered := l.renderItem(item) @@ -387,6 +440,7 @@ func (l *list) renderBackward() tea.Cmd { currentHeight := 0 currentIndex := 0 for i := len(l.items) - 1; i >= 0; i-- { + fmt.Printf("rendering item %d\n", i) currentIndex = i if currentHeight > l.listHeight() { break @@ -397,12 +451,13 @@ func (l *list) renderBackward() tea.Cmd { } // initial render l.renderView() - if currentIndex == len(l.items)-1 { + if currentIndex == 0 { l.isReady = true return nil } return func() tea.Msg { for i := currentIndex; i >= 0; i-- { + fmt.Printf("rendering item after %d\n", i) rendered := l.renderItem(l.items[i]) l.renderedItems = append([]renderedItem{rendered}, l.renderedItems...) } @@ -451,6 +506,9 @@ func (l *list) renderItems() tea.Cmd { l.selectLastItem() } } + if l.direction == Forward { + return l.renderForward() + } return l.renderBackward() } diff --git a/internal/tui/exp/list/list_test.go b/internal/tui/exp/list/list_test.go index 54f6f3fcac9878e29f2e309e1aee902cefdf46c0..4623d7e025dda586e78853cd4b26889705eb40de 100644 --- a/internal/tui/exp/list/list_test.go +++ b/internal/tui/exp/list/list_test.go @@ -12,6 +12,93 @@ import ( "github.com/stretchr/testify/assert" ) +func TestListPosition(t *testing.T) { + type positionOffsetTest struct { + dir direction + test string + width int + height int + numItems int + + moveUp int + moveDown int + + expectedStart int + expectedEnd int + } + tests := []positionOffsetTest{ + { + dir: Forward, + test: "should have correct position initially when forward", + moveUp: 0, + moveDown: 0, + width: 10, + height: 20, + numItems: 100, + expectedStart: 0, + expectedEnd: 19, + }, + { + dir: Forward, + test: "should offset start and end by one when moving down by one", + moveUp: 0, + moveDown: 1, + width: 10, + height: 20, + numItems: 100, + expectedStart: 1, + expectedEnd: 20, + }, + { + dir: Backward, + test: "should have correct position initially when backward", + moveUp: 0, + moveDown: 0, + width: 10, + height: 20, + numItems: 100, + expectedStart: 80, + expectedEnd: 99, + }, + { + dir: Backward, + test: "should offset the start and end by one when moving up by one", + moveUp: 1, + moveDown: 0, + width: 10, + height: 20, + numItems: 100, + expectedStart: 79, + expectedEnd: 98, + }, + } + for _, c := range tests { + t.Run(c.test, func(t *testing.T) { + l := New(WithDirection(c.dir)).(*list) + l.SetSize(c.width, c.height) + items := []Item{} + for i := range c.numItems { + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) + items = append(items, item) + } + cmd := l.SetItems(items) + if cmd != nil { + cmd() + } + + if c.moveUp > 0 { + l.MoveUp(c.moveUp) + } + if c.moveDown > 0 { + l.MoveDown(c.moveDown) + } + start, end := l.viewPosition() + assert.Equal(t, c.expectedStart, start) + assert.Equal(t, c.expectedEnd, end) + }) + } +} + func TestBackwardList(t *testing.T) { t.Run("within height", func(t *testing.T) { t.Parallel() @@ -19,7 +106,7 @@ func TestBackwardList(t *testing.T) { l.SetSize(10, 20) items := []Item{} for i := range 5 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -36,7 +123,7 @@ func TestBackwardList(t *testing.T) { t.Parallel() items := []Item{} for i := range 5 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } l := New(WithDirection(Backward), WithGap(1), WithSelectedItem(items[2].ID())).(*list) @@ -54,7 +141,7 @@ func TestBackwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -70,7 +157,7 @@ func TestBackwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d\nLine2", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d\nLine2", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -86,7 +173,7 @@ func TestBackwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -97,13 +184,13 @@ func TestBackwardList(t *testing.T) { l.MoveUp(1) golden.RequireEqual(t, []byte(l.View())) }) + t.Run("should move at max to the top", func(t *testing.T) { - t.Parallel() l := New(WithDirection(Backward)).(*list) l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -121,7 +208,7 @@ func TestBackwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -138,7 +225,7 @@ func TestBackwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -156,7 +243,7 @@ func TestBackwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -167,15 +254,15 @@ func TestBackwardList(t *testing.T) { selectedInx := len(l.items) - 2 currentItem := items[len(l.items)-1] nextItem := items[selectedInx] - assert.False(t, nextItem.(SimpleItem).IsFocused()) - assert.True(t, currentItem.(SimpleItem).IsFocused()) + assert.False(t, nextItem.(SelectableItem).IsFocused()) + assert.True(t, currentItem.(SelectableItem).IsFocused()) cmd = l.SelectItemAbove() if cmd != nil { cmd() } assert.Equal(t, l.selectedItem, l.items[selectedInx].ID()) - assert.True(t, l.items[selectedInx].(SimpleItem).IsFocused()) + assert.True(t, l.items[selectedInx].(SelectableItem).IsFocused()) golden.RequireEqual(t, []byte(l.View())) }) @@ -185,7 +272,7 @@ func TestBackwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -210,7 +297,7 @@ func TestForwardList(t *testing.T) { l.SetSize(10, 20) items := []Item{} for i := range 5 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -227,7 +314,7 @@ func TestForwardList(t *testing.T) { t.Parallel() items := []Item{} for i := range 5 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } l := New(WithDirection(Forward), WithGap(1), WithSelectedItem(items[2].ID())).(*list) @@ -245,7 +332,7 @@ func TestForwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -261,7 +348,7 @@ func TestForwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d\nLine2", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d\nLine2", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -277,7 +364,7 @@ func TestForwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -288,13 +375,13 @@ func TestForwardList(t *testing.T) { l.MoveDown(1) golden.RequireEqual(t, []byte(l.View())) }) - t.Run("should move at max to the top", func(t *testing.T) { + t.Run("should move at max to the bottom", func(t *testing.T) { t.Parallel() l := New(WithDirection(Forward)).(*list) l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -312,7 +399,7 @@ func TestForwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -329,7 +416,7 @@ func TestForwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -347,7 +434,7 @@ func TestForwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -358,15 +445,15 @@ func TestForwardList(t *testing.T) { selectedInx := 1 currentItem := items[0] nextItem := items[selectedInx] - assert.False(t, nextItem.(SimpleItem).IsFocused()) - assert.True(t, currentItem.(SimpleItem).IsFocused()) + assert.False(t, nextItem.(SelectableItem).IsFocused()) + assert.True(t, currentItem.(SelectableItem).IsFocused()) cmd = l.SelectItemBelow() if cmd != nil { cmd() } assert.Equal(t, l.selectedItem, l.items[selectedInx].ID()) - assert.True(t, l.items[selectedInx].(SimpleItem).IsFocused()) + assert.True(t, l.items[selectedInx].(SelectableItem).IsFocused()) golden.RequireEqual(t, []byte(l.View())) }) @@ -376,7 +463,7 @@ func TestForwardList(t *testing.T) { l.SetSize(10, 5) items := []Item{} for i := range 10 { - item := NewSimpleItem(fmt.Sprintf("Item %d", i)) + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) items = append(items, item) } cmd := l.SetItems(items) @@ -394,7 +481,28 @@ func TestForwardList(t *testing.T) { }) } -type SimpleItem interface { +func TestListSelection(t *testing.T) { + t.Run("should skip none selectable items initially", func(t *testing.T) { + t.Parallel() + l := New(WithDirection(Forward)).(*list) + l.SetSize(100, 10) + items := []Item{} + items = append(items, NewSimpleItem("None Selectable")) + for i := range 5 { + item := NewSelectsableItem(fmt.Sprintf("Item %d", i)) + items = append(items, item) + } + cmd := l.SetItems(items) + if cmd != nil { + cmd() + } + + assert.Equal(t, items[1].ID(), l.selectedItem) + golden.RequireEqual(t, []byte(l.View())) + }) +} + +type SelectableItem interface { Item layout.Focusable } @@ -403,15 +511,24 @@ type simpleItem struct { width int content string id string +} +type selectableItem struct { + *simpleItem focused bool } -func NewSimpleItem(content string) SimpleItem { +func NewSimpleItem(content string) *simpleItem { return &simpleItem{ + id: uuid.NewString(), width: 0, content: content, - focused: false, - id: uuid.NewString(), + } +} + +func NewSelectsableItem(content string) SelectableItem { + return &selectableItem{ + simpleItem: NewSimpleItem(content), + focused: false, } } @@ -428,9 +545,6 @@ func (s *simpleItem) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } func (s *simpleItem) View() string { - if s.focused { - return lipgloss.NewStyle().BorderLeft(true).BorderStyle(lipgloss.NormalBorder()).Width(s.width).Render(s.content) - } return lipgloss.NewStyle().Width(s.width).Render(s.content) } @@ -444,19 +558,26 @@ func (s *simpleItem) SetSize(width int, height int) tea.Cmd { return nil } +func (s *selectableItem) View() string { + if s.focused { + return lipgloss.NewStyle().BorderLeft(true).BorderStyle(lipgloss.NormalBorder()).Width(s.width).Render(s.content) + } + return lipgloss.NewStyle().Width(s.width).Render(s.content) +} + // Blur implements SimpleItem. -func (s *simpleItem) Blur() tea.Cmd { +func (s *selectableItem) Blur() tea.Cmd { s.focused = false return nil } // Focus implements SimpleItem. -func (s *simpleItem) Focus() tea.Cmd { +func (s *selectableItem) Focus() tea.Cmd { s.focused = true return nil } // IsFocused implements SimpleItem. -func (s *simpleItem) IsFocused() bool { +func (s *selectableItem) IsFocused() bool { return s.focused } diff --git a/internal/tui/exp/list/testdata/TestBackwardList/should_move_at_max_to_the_top.golden b/internal/tui/exp/list/testdata/TestBackwardList/should_move_at_max_to_the_top.golden index b811a8818ab2e87fdd09b64f69dcca8694a5b77c..a92d5cd50b42ac4e59b2fac2fc21355b30d4c1d0 100644 --- a/internal/tui/exp/list/testdata/TestBackwardList/should_move_at_max_to_the_top.golden +++ b/internal/tui/exp/list/testdata/TestBackwardList/should_move_at_max_to_the_top.golden @@ -2,4 +2,4 @@ Item 0 Item 1 Item 2 Item 3 -Item 4 \ No newline at end of file +│Item 4 \ No newline at end of file diff --git a/internal/tui/exp/list/testdata/TestBackwardList/should_move_up.golden b/internal/tui/exp/list/testdata/TestBackwardList/should_move_up.golden index 97a5e679968d701bfd0c4df1ab01e4d64891bbfe..b34ef9acef9960d727b203566011bb66953079d4 100644 --- a/internal/tui/exp/list/testdata/TestBackwardList/should_move_up.golden +++ b/internal/tui/exp/list/testdata/TestBackwardList/should_move_up.golden @@ -2,4 +2,4 @@ Item 4 Item 5 Item 6 Item 7 -Item 8 \ No newline at end of file +│Item 8 \ No newline at end of file diff --git a/internal/tui/exp/list/testdata/TestBackwardList/within_height.golden b/internal/tui/exp/list/testdata/TestBackwardList/within_height.golden index a4b10135f97530577ee977b4325e30fe34184f21..4406faf046ad8229b1dc8908091ad47d555ddaf6 100644 --- a/internal/tui/exp/list/testdata/TestBackwardList/within_height.golden +++ b/internal/tui/exp/list/testdata/TestBackwardList/within_height.golden @@ -1,7 +1,5 @@ Item 0 -Item 0 - Item 1 Item 2 diff --git a/internal/tui/exp/list/testdata/TestForwardList/should_move_at_max_to_the_top.golden b/internal/tui/exp/list/testdata/TestForwardList/should_move_at_max_to_the_bottom.golden similarity index 58% rename from internal/tui/exp/list/testdata/TestForwardList/should_move_at_max_to_the_top.golden rename to internal/tui/exp/list/testdata/TestForwardList/should_move_at_max_to_the_bottom.golden index 33aff077aeecc3b1f74ca93b7c7589b908abf16d..d5091ddac1b9d427f257f37dd7fe57ebf871da62 100644 --- a/internal/tui/exp/list/testdata/TestForwardList/should_move_at_max_to_the_top.golden +++ b/internal/tui/exp/list/testdata/TestForwardList/should_move_at_max_to_the_bottom.golden @@ -1,4 +1,4 @@ -Item 5 +│Item 5 Item 6 Item 7 Item 8 diff --git a/internal/tui/exp/list/testdata/TestForwardList/should_move_down.golden b/internal/tui/exp/list/testdata/TestForwardList/should_move_down.golden index f01acdd8274b7ebb8307ba004b05ab205868cf71..691521bf35b5d15776b6c7cef93c0c1bbd4a26ba 100644 --- a/internal/tui/exp/list/testdata/TestForwardList/should_move_down.golden +++ b/internal/tui/exp/list/testdata/TestForwardList/should_move_down.golden @@ -1,4 +1,4 @@ -Item 1 +│Item 1 Item 2 Item 3 Item 4 diff --git a/internal/tui/exp/list/testdata/TestForwardList/within_height.golden b/internal/tui/exp/list/testdata/TestForwardList/within_height.golden index 1450d9e9ac0bc14266454379b8736df4b7775bb0..676da068c53cadc771497892ae66daeb786aaaa2 100644 --- a/internal/tui/exp/list/testdata/TestForwardList/within_height.golden +++ b/internal/tui/exp/list/testdata/TestForwardList/within_height.golden @@ -1,7 +1,5 @@ │Item 0 -│Item 0 - Item 1 Item 2 diff --git a/internal/tui/exp/list/testdata/TestListSelection/should_skip_none_selectable_items_initially.golden b/internal/tui/exp/list/testdata/TestListSelection/should_skip_none_selectable_items_initially.golden new file mode 100644 index 0000000000000000000000000000000000000000..12d86d00139c82ff088421a1dfac9b66d82747cc --- /dev/null +++ b/internal/tui/exp/list/testdata/TestListSelection/should_skip_none_selectable_items_initially.golden @@ -0,0 +1,6 @@ +None Selectable +│Item 0 +Item 1 +Item 2 +Item 3 +Item 4 \ No newline at end of file