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

feat: Add Cursor() methods to Select and MultiSelect fields #407

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kralicky
Copy link

Hi! This PR addresses a small limitation I ran into while building out a UI with this library. I was using a MultiSelect field to display a list of options, and wanted to use a Note to show additional details/context information for the option under the cursor. However, there was no way to know which option that was.

With this change, you can now write the following:

items := []string{...}

ms := huh.NewMultiSelect[string]().
	Options(huh.NewOptions(items...)...)

huh.NewNote().
	DescriptionFunc(func() string {
		currentItem := items[ms.Cursor()]
		// do something with currentItem
	}, onCursorUpdate{ms}) // <- see below

There is still the small issue of knowing when to invalidate the DescriptionFunc - I solved this by using the value of Cursor() as a hash:

type onCursorUpdate struct {
	Field interface{ Cursor() int }
}

// Hash implements hashstructure.Hashable
func (u onCursorUpdate) Hash() (uint64, error) {
	return uint64(u.Field.Cursor()), nil
}

This relies on implementation details however, so if this use case is common, something like it (but with a more idiomatic api) could be a good candidate for a built-in utility.

@kralicky kralicky changed the title Add Cursor() methods to Select and MultiSelect fields feat: Add Cursor() methods to Select and MultiSelect fields Sep 10, 2024
@Siri-chan
Copy link

Siri-chan commented Sep 18, 2024

+1 because I actually also have this use-case, more or less - but with a map[string]string rather than a []string.

Some comments on the change:
Wouldn't this make more sense to return the [T comparable] key that the MultiSelect is built with?
Just exporting cursor as readonly doesn't seem to fit the way the rest of the API returns items from the list of options (see MultiSelect.Value(*[]T), for example - it doesn't just insert the indices of the options, but rather their keys).
Maybe something of this form would be more flexible and appropriate?

func (m *MultiSelect[T]) Hovered() T {
	return m.options.val[m.cursor]
}

@kralicky
Copy link
Author

kralicky commented Sep 18, 2024

@Siri-chan I agree, good idea. Plus, the lists can be filtered, so just having the cursor index might not be sufficient to know what option is hovered.

Here's what I came up with that works for my use case, what do you think?

// Hovered returns the value of the option under the cursor, and a bool
// indicating whether one was found. If there are no visible options, returns
// a zero-valued T and false.
func (m *MultiSelect[T]) Hovered() (T, bool) {
	if len(m.filteredOptions) == 0 || m.cursor >= len(m.filteredOptions) {
		var zero T
		return zero, false
	}
	return m.filteredOptions[m.cursor].Value, true
}

@Siri-chan
Copy link

Siri-chan commented Sep 19, 2024

Seems fine by me - your branch works for my use-case and doesn't break any of the demo tapes as best I can tell
.

@kralicky
Copy link
Author

After some more testing, it looks like there is a possible race condition here. Commands are evaluated in a separate goroutine from the one that handles messages, and the DescriptionFunc/TitleFunc/etc functions are called during command evaluation in the background, so they can't read anything that could be modified in another component's Update(). Not sure the best way to fix 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 this pull request may close these issues.

2 participants