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

Default value doesn't work for NewSelect #444

Open
puyt opened this issue Oct 27, 2024 · 2 comments
Open

Default value doesn't work for NewSelect #444

puyt opened this issue Oct 27, 2024 · 2 comments
Labels
bug Something isn't working select

Comments

@puyt
Copy link

puyt commented Oct 27, 2024

Describe the bug

The default value isn't selected, instead the 1st available option is selected. Seems this is only happening when using the OptionsFunc instead of Options.

To Reproduce

The following code example works:

package main

import "github.com/charmbracelet/huh"

func main() {

	foo := "two"
	form := huh.NewForm(
		huh.NewGroup(
			huh.NewSelect[string]().
				Title("foo").
				Options(huh.NewOptions("one", "two", "three")...).
				Value(&foo),
		),
	)

	form.Run()
}

The following example uses OptionsFunc and the default value is not working:

package main

import "github.com/charmbracelet/huh"

func main() {

	foo := "two"
	form := huh.NewForm(
		huh.NewGroup(
			huh.NewSelect[string]().
				Title("foo").
				OptionsFunc(func() []huh.Option[string] {
				 	return getOptions()
				}, nil).
				Value(&foo),
		),
	)

	form.Run()
}

func getOptions() []huh.Option[string] {
	var options []huh.Option[string]

	options = append(options, huh.NewOption("one", "one"))
	options = append(options, huh.NewOption("two", "two"))
	options = append(options, huh.NewOption("three", "three"))

	return options
}

Expected behavior

Expect two to be selected by default in both cases.

Related issue: #70

@prithvijj
Copy link

prithvijj commented Nov 4, 2024

Just providing more context (if i understand it right)

When the OptionsFunc is called, it sets the options.fn field

huh/field_select.go

Lines 218 to 219 in 20a4d21

func (s *Select[T]) OptionsFunc(f func() []Option[T], bindings any) *Select[T] {
s.options.fn = f

Which eventually is triggered here

huh/field_select.go

Lines 349 to 351 in 20a4d21

cmds = append(cmds, func() tea.Msg {
return updateOptionsMsg[T]{id: s.id, hash: hash, options: s.options.fn()}
}, s.spinner.Tick)

and when the updateOptionsMsg cmd is sent, it ends up executing this

huh/field_select.go

Lines 371 to 380 in 20a4d21

case updateOptionsMsg[T]:
if msg.id == s.id && msg.hash == s.options.bindingsHash {
s.options.update(msg.options)
// since we're updating the options, we need to update the selected cursor
// position and filteredOptions.
s.selected = clamp(s.selected, 0, len(msg.options)-1)
s.filteredOptions = msg.options
s.updateValue()
}

Which ends up setting the "dynamic" options and based on the code it just shows that it just set's the options, and doesn't really take into consideration anything about selection of options, as seen in the Options func

huh/field_select.go

Lines 172 to 187 in 20a4d21

func (s *Select[T]) Options(options ...Option[T]) *Select[T] {
if len(options) <= 0 {
return s
}
s.options.val = options
s.filteredOptions = options
// Set the cursor to the existing value or the last selected option.
for i, option := range options {
if option.Value == s.accessor.Get() {
s.selected = i
break
} else if option.selected {
s.selected = i
}
}

So taking the same code over,( i.e. ensuring the option is selected) and putting it within case updateOptionsMsg[T]:

diff --git a/field_select.go b/field_select.go
index 81e57d9..3d90d67 100644
--- a/field_select.go
+++ b/field_select.go
@@ -372,6 +372,15 @@ func (s *Select[T]) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
                if msg.id == s.id && msg.hash == s.options.bindingsHash {
                        s.options.update(msg.options)
 
+                       // Set the cursor to the existing value or the last selected option.
+                       for i, option := range msg.options {
+                               if option.Value == s.accessor.Get() {
+                                       s.selected = i
+                                       break
+                               } else if option.selected {
+                                       s.selected = i
+                               }
+                       }
                        // since we're updating the options, we need to update the selected cursor
                        // position and filteredOptions.
                        s.selected = clamp(s.selected, 0, len(msg.options)-1)

Was able to get the "default" value set

image

Can probably look at creating a PR for it, if that seems to make sense, or validates the thinking

@sujithktkm
Copy link

As #70 is closed, posting here -

@maaslalani looks like the selection of default value for NewSelect happens if if this was hidden with WithHide. I think we should NOT pick a default if the prompt is hidden. What are your thoughts ?

@caarlos0 caarlos0 added bug Something isn't working select labels Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working select
Projects
None yet
Development

No branches or pull requests

4 participants