Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List Model Index returns index offset based on filtered list, but SetItem expects global offset in items list. #550

Open
blkt opened this issue Jul 11, 2024 · 2 comments · May be fixed by #574

Comments

@blkt
Copy link

blkt commented Jul 11, 2024

Describe the bug
The index value retrieved by using Index returns an offset that implicitly takes into account whether filtering is active, so an hypothetical value o 0 is actually the first item in the filtered list of items.

On the other hand, SetItem expects an index to be used with the global list of items, both filtered and unfiltered.

This makes it hard (if not impossible) to update an item within a filtered list.

I definitely might be using bubbletea list wrong, please let me know if that's the case.

Setup
Please complete the following information along with version numbers, if applicable.

  • OS macOS
  • Shell zsh
  • Terminal Emulator iterm
  • Terminal Multiplexer none
  • Locale en_US.UTF-8

To Reproduce
Using code below:

  1. using / filter by b
  2. push space on the first item
  3. remove filtering
  4. item at position 0, i.e. a, was replaced instead of b

Source Code
Here is a minimal model usable to reproduce the issue, also found here.

package main

import (
	"cmp"
	"fmt"
	"io"
	"slices"
	"strings"

	"github.com/charmbracelet/bubbles/key"
	"github.com/charmbracelet/bubbles/list"
	tea "github.com/charmbracelet/bubbletea"
	"github.com/charmbracelet/lipgloss"
)

func MultiSelect(choices []string) ([]string, error) {
	items := make([]list.Item, 0, len(choices))
	for _, c := range choices {
		items = append(items, item{title: c})
	}

	slices.SortFunc(items, func(a, b list.Item) int {
		return cmp.Compare(a.(item).title, b.(item).title)
	})

	l := list.New(items, itemDelegate{}, 0, 0)
	l.Title = "Select repos to register"
	l.AdditionalShortHelpKeys = extraKeys
	l.AdditionalFullHelpKeys = extraKeys

	height := 30 // 20 + 10, 10 is a magic number to show 20 items
	if size := len(items); size < 20 {
		height = size + 10
	}
	model := model{list: l, height: height}
	p := tea.NewProgram(model)
	if _, err := p.Run(); err != nil {
		return nil, err
	}

	selection := make([]string, 0, len(items))
	for _, listItem := range items {
		item := listItem.(item)
		if item.checked {
			selection = append(selection, item.title)
		}
	}

	return selection, nil
}

var (
	itemStyle         = lipgloss.NewStyle().PaddingLeft(4)
	selectedItemStyle = lipgloss.NewStyle().PaddingLeft(2)
)

type item struct {
	title   string
	checked bool
}

func (i item) Title() string       { return i.title }
func (_ item) Description() string { return "" }
func (i item) FilterValue() string { return i.title }

type itemDelegate struct{}

func (_ itemDelegate) Height() int                             { return 1 }
func (_ itemDelegate) Spacing() int                            { return 0 }
func (_ itemDelegate) Update(_ tea.Msg, _ *list.Model) tea.Cmd { return nil }
func (_ itemDelegate) Render(w io.Writer, m list.Model, index int, listItem list.Item) {
	i, ok := listItem.(item)
	if !ok {
		return
	}

	fn := itemStyle.Render
	if index == m.Index() {
		fn = func(s ...string) string {
			return selectedItemStyle.Render("> " + strings.Join(s, " "))
		}
	}

	checked := "[ ]"
	if i.checked {
		checked = "[x]"
	}

	fmt.Fprint(w, fn(checked, i.title))
}

type model struct {
	list   list.Model
	height int
}

func (_ model) Init() tea.Cmd {
	return nil
}

func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	switch msg := msg.(type) {
	case tea.WindowSizeMsg:
		m.list.SetSize(0, m.height)

	case tea.KeyMsg:
		switch msg.String() {
		case "ctrl+c", "enter":
			return m, tea.Quit
		case " ":
			idx := m.list.Index()
			oldItem := m.list.SelectedItem().(item)
			cmd := m.list.SetItem(idx, item{
				title:   oldItem.title,
				checked: !oldItem.checked,
			})
			return m, cmd
		}
	}

	var cmd tea.Cmd
	m.list, cmd = m.list.Update(msg)
	return m, cmd
}

func (m model) View() string {
	return m.list.View()
}

func extraKeys() []key.Binding {
	return []key.Binding{
		key.NewBinding(
			key.WithKeys("space"),
			key.WithHelp("space", "select item"),
		),
	}
}

func main() {
	MultiSelect([]string{"a", "b1", "b2", "b3", "c"})
}

Expected behavior
The item under selection is modified and not an arbitrary one.

nobe4 added a commit to nobe4/bubbles that referenced this issue Jul 31, 2024
This commit introduces a new `filteredItem` field called `item` which
stores the index of the filtered item in the unfiltered list. This
allows to get at runtime the unfiltered list index for the selected (and
possibly filtered) item.

This is the only solution I found to use `SetItem` with a filtered
list.

The name `GlobalIndex` might not be ideal, I'm happy to change it to
something else. (`UnfilteredIndex`?)

Fixes: charmbracelet#550
@nobe4 nobe4 linked a pull request Jul 31, 2024 that will close this issue
@nobe4
Copy link

nobe4 commented Jul 31, 2024

👋 @blkt I have potentially found a solution for this, and implemented a fix in #574

nobe4 added a commit to nobe4/bubbles that referenced this issue Jul 31, 2024
This commit introduces a new `filteredItem` field called `index` which
stores the index of the filtered item in the unfiltered list. This
allows to get at runtime the unfiltered list index for the selected (and
possibly filtered) item.

This is the only solution I found to use `SetItem` with a filtered
list.

The name `GlobalIndex` might not be ideal, I'm happy to change it to
something else. (`UnfilteredIndex`?)

Fixes: charmbracelet#550
@blkt
Copy link
Author

blkt commented Jul 31, 2024

That's great @nobe4, thank you for looking into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants