Fix bug where selecting a row while filtered would lose all other rows

.github/workflows/lint.yml

@@ -13,9 +13,14 @@ jobs:
     name: lint
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
-      - name: golangci-lint
-        uses: golangci/golangci-lint-action@v3
+      # For whatever reason, 1.21.9 blows up because it can't
+      # find 'max' in some go lib... pinning to 1.21.4 fixes this
+      - name: Install Go
+        uses: actions/setup-go@v4
         with:
-          version: v1.51.1
+          go-version: 1.21.4
+      - name: Checkout code
+        uses: actions/checkout@v4
+      - name: Lint
+        run: make lint
 

examples/filter/main.go

@@ -30,6 +30,7 @@ func NewModel() Model {
 			Filtered(true).
 			Focused(true).
 			WithPageSize(10).
+			SelectableRows(true).
 			WithRows([]table.Row{
 				table.NewRow(table.RowData{
 					columnKeyTitle:       "Computer Systems : A Programmer's Perspective",

table/row.go

@@ -2,6 +2,7 @@ package table
 
 import (
 	"fmt"
+	"sync/atomic"
 
 	"github.com/charmbracelet/lipgloss"
 	"github.com/muesli/reflow/wordwrap"
@@ -21,14 +22,22 @@ type Row struct {
 	Data  RowData
 
 	selected bool
+
+	// id is an internal unique ID to match rows after they're copied
+	id uint32
 }
 
+var lastRowID uint32 = 1
+
 // NewRow creates a new row and copies the given row data.
 func NewRow(data RowData) Row {
 	row := Row{
 		Data: make(map[string]interface{}),
+		id:   lastRowID,
 	}
 
+	atomic.AddUint32(&lastRowID, 1)
+
 	for key, val := range data {
 		// Doesn't deep copy val, but close enough for now...
 		row.Data[key] = val

table/update.go

@@ -30,14 +30,19 @@ func (m *Model) toggleSelect() {
 		return
 	}
 
-	rows := make([]Row, len(m.GetVisibleRows()))
-	copy(rows, m.GetVisibleRows())
+	rows := m.GetVisibleRows()
 
-	currentSelectedState := rows[m.rowCursorIndex].selected
+	rowID := rows[m.rowCursorIndex].id
 
-	rows[m.rowCursorIndex].selected = !currentSelectedState
+	currentSelectedState := false
+
+	for i := range m.rows {
+		if m.rows[i].id == rowID {
+			currentSelectedState = m.rows[i].selected
+			m.rows[i].selected = !m.rows[i].selected
+		}
+	}
 
-	m.rows = rows
 	m.visibleRowCacheUpdated = false
 
 	m.appendUserEvent(UserEventRowSelectToggled{

table/update_test.go

@@ -330,3 +330,82 @@ func TestFilterWithKeypresses(t *testing.T) {
 
 	assert.Len(t, visible, 2)
 }
+
+// This is a long test with a lot of movement keys pressed, that's okay because
+// it's simply repetitive and tracking the same kind of state change many times
+//
+//nolint:funlen
+func TestSelectOnFilteredTableDoesntLoseRows(t *testing.T) {
+	// Issue: https://github.com/Evertras/bubble-table/issues/170
+	//
+	// Basically, if you filter a table and then select a row, then
+	// clear the filter, then all the other rows should still exist.
+
+	cols := []Column{
+		NewColumn("name", "Name", 10).WithFiltered(true),
+	}
+
+	model := New(cols).WithRows([]Row{
+		NewRow(RowData{"name": "Charmander"}),
+		NewRow(RowData{"name": "Pikachu"}),
+	}).Focused(true).Filtered(true).SelectableRows(true)
+
+	hitKey := func(key rune) {
+		model, _ = model.Update(tea.KeyMsg{
+			Type:  tea.KeyRunes,
+			Runes: []rune{key},
+		})
+	}
+
+	hitEnter := func() {
+		model, _ = model.Update(tea.KeyMsg{Type: tea.KeyEnter})
+	}
+
+	hitEscape := func() {
+		model, _ = model.Update(tea.KeyMsg{Type: tea.KeyEscape})
+	}
+
+	hitSpacebar := func() {
+		model, _ = model.Update(tea.KeyMsg{Type: tea.KeySpace})
+	}
+
+	// First, apply the filter
+	//
+	// Note that we try and filter for the second row, "Pikachu"
+	// so that we can better ensure everything is stably intact
+	visible := model.GetVisibleRows()
+
+	assert.Len(t, visible, 2)
+	hitKey(rune(model.KeyMap().Filter.Keys()[0][0]))
+	assert.Len(t, visible, 2)
+	hitKey('p')
+	hitKey('i')
+	hitKey('k')
+
+	visible = model.GetVisibleRows()
+
+	assert.Len(t, visible, 1)
+
+	hitEnter()
+
+	// Now apply the selection toggle
+	hitSpacebar()
+
+	visible = model.GetVisibleRows()
+	assert.Len(t, visible, 1)
+	assert.True(t, visible[0].selected)
+
+	// Now clear the filter and make sure everything is intact
+	hitEscape()
+
+	visible = model.GetVisibleRows()
+
+	assert.Len(t, visible, 2)
+
+	if t.Failed() {
+		return
+	}
+
+	assert.False(t, visible[0].selected)
+	assert.True(t, visible[1].selected)
+}