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

fix: set YOffset + cursor based on first selected item #331

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

Conversation

bashbunni
Copy link
Member

@bashbunni bashbunni commented Jul 25, 2024

image
tested against gum @ main-huh (basically just made it depend on this branch of huh)
image

examples all work normally

you can test with Gum (after a lil go get -u github.com/charmbracelet/huh@refactor-select) then try seq 22 | go run . choose --selected=2,21 --limit=2 (or set limit to one or none for single select)

@maaslalani
Copy link
Contributor

Nice! This is a great UX improvement!

@bashbunni bashbunni linked an issue Jul 25, 2024 that may be closed by this pull request
@meowgorithm
Copy link
Member

Totally, would almost call this a feature versus a fix. @bashbunni, can you add example Go code we can use to vet this?

@bashbunni
Copy link
Member Author

@meowgorithm Just added some instructions. I mainly tested it with Gum.

  1. go to gum and replace the huh dep with this branch
  2. seq 22 | go run . choose --selected=2,21 --limit=2

@meowgorithm
Copy link
Member

We should test with huh the library itself as well. It looks like there's still a bug when navigating through groups with tab. Example here:

package main

import (
	"fmt"
	"os"

	"github.com/charmbracelet/huh"
)

func main() {
	f := huh.NewForm(
		huh.NewGroup(
			huh.NewSelect[string]().
				Options(
					huh.NewOption("A", "a"),
					huh.NewOption("B", "b"),
					huh.NewOption("C", "c"),
					huh.NewOption("D", "d"),
					huh.NewOption("E", "e"),
					huh.NewOption("F", "f"),
					huh.NewOption("G", "g"),
					huh.NewOption("H", "h"),
					huh.NewOption("I", "i"),
					huh.NewOption("J", "j"),
					huh.NewOption("K", "k").Selected(true),
					huh.NewOption("L", "l"),
					huh.NewOption("M", "m"),
					huh.NewOption("N", "n"),
					huh.NewOption("O", "o"),
					huh.NewOption("P", "p"),
				),
		).WithHeight(8),
		huh.NewGroup(
			huh.NewMultiSelect[string]().
				Options(
					huh.NewOption("A", "a"),
					huh.NewOption("B", "b"),
					huh.NewOption("C", "c"),
					huh.NewOption("D", "d"),
					huh.NewOption("E", "e"),
					huh.NewOption("F", "f"),
					huh.NewOption("G", "g"),
					huh.NewOption("H", "h"),
					huh.NewOption("I", "i"),
					huh.NewOption("K", "k").Selected(true),
					huh.NewOption("L", "l"),
					huh.NewOption("M", "m"),
					huh.NewOption("N", "n"),
					huh.NewOption("O", "o").Selected(true),
					huh.NewOption("P", "p"),
				),
		).WithHeight(8),
	)

	if err := f.Run(); err != nil {
		fmt.Fprintf(os.Stderr, "Oof: %v", err)
	}
}

@bashbunni
Copy link
Member Author

Going to test more edge cases before merging this one :)

@meowgorithm
Copy link
Member

Yeah, please do look at the example I posted above as well

@maaslalani
Copy link
Contributor

You may also need to test with Dynamic Huh? That is, what happens when options are swapped out and some are selected.

@bashbunni bashbunni self-assigned this Jul 31, 2024
@bashbunni
Copy link
Member Author

The group-related issue is actually caused by a different bug I found and documented here. WithHeight wasn't setting the viewport height and was instead just cutting content. That's why the calculations in this branch weren't working properly. Will continue trying to break it though :)

@bashbunni
Copy link
Member Author

Maybe I'm doing something wrong, but I can't seem to get pre-selected options to work in dynamic huh. You can try for yourself by checking out the dynamic-selection-example branch and running the dynamic/dynamic-selections example.
https://github.com/charmbracelet/huh/blob/dynamic-selection-example/examples/dynamic/dynamic-selections/main.go

I created this example to test pre-selected elements in dynamic huh, but the cursor always starts at the top of the options anyway. Let me know if this is by design!

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.

gum choose --selected=: cursor do not appear
4 participants