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.

Sign up for GitHub

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

Update init experience #3933

Merged
merged 4 commits into from
Sep 27, 2022
Merged

Update init experience #3933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4a9bd84
Fix double semicolon and cursor location
AaronCrawfis Sep 27, 2022
7b3ca1f
Update kubeconfig selection logic
AaronCrawfis Sep 27, 2022
a292c36
Auto formatting
AaronCrawfis Sep 27, 2022
61f10b7
Correct integration tests
Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions pkg/cli/cmd/provider/common/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ func SelectEnvironmentName(cmd *cobra.Command, defaultVal string, interactive bo
return "", err
}
if interactive && envStr == "" {
promptMsg := fmt.Sprintf("Enter an environment name [%s]:", defaultVal)
envStr, err = prompter.RunPrompt(prompt.TextPromptWithDefault(promptMsg, defaultVal, prompt.ResourceName))
envStr, err = prompter.RunPrompt(prompt.TextPromptWithDefault("Enter an environment name", defaultVal, prompt.ResourceName))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because promptui already has a default experience, we don't need to manually build the prompt message with the default

if err != nil {
return "", err
}
Expand All @@ -62,9 +61,8 @@ func SelectNamespace(cmd *cobra.Command, defaultVal string, interactive bool, pr
var val string
var err error
if interactive {
promptMsg := fmt.Sprintf("Enter a namespace name to deploy apps into [%s]:", defaultVal)
namespaceSelector := promptui.Prompt {
Label: promptMsg,
namespaceSelector := promptui.Prompt{
Label: "Enter a namespace name to deploy apps into",
Default: defaultVal,
Validate: func(s string) error {
valid, msg, err := prompt.EmptyValidator(s)
Expand All @@ -76,6 +74,7 @@ func SelectNamespace(cmd *cobra.Command, defaultVal string, interactive bool, pr
}
return nil
},
AllowEdit: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This places the cursor at the end of the line instead of the beginning

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the other case, the user can either press enter or start typing. In this case the user will have to clear the default first?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, in the old case you could start typing and it would clear the current entry. In this new case you have to hit backspace to clear it out manually and then type the new name.

Given that the cursor blocks out the first character, I think this change is preferrable.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an alternative we can switch to https://github.com/charmbracelet/bubbletea, which has better support for default values (greyed out text)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree cursor blocking is not great. LGTM.

}
val, err = prompter.RunPrompt(namespaceSelector)
if err != nil {
Expand Down
19 changes: 6 additions & 13 deletions pkg/cli/cmd/radInit/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,22 +235,15 @@ func installRadius(ctx context.Context, r *Runner) error {
func selectKubeContext(currentContext string, kubeContexts map[string]*api.Context, interactive bool, prompter prompt.Interface) (string, error) {
values := []string{}
if interactive {
confirmDefaultContext, err := prompt.YesOrNoPrompter(
fmt.Sprintf("Confirm default context: %s, %s", currentContext, "[Y/n]"),
"Y",
prompter,
)
if err != nil {
return "", err
}
if strings.ToLower(confirmDefaultContext) == "y" {
return currentContext, nil
}
defaultValue := fmt.Sprintf("%s (current)", currentContext)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplify selection process by placing current at the top and appending (current), and just using a selection prompt

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it. This is one of the ideas we discussed a few weeks ago, glad to see it come back.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nice!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests might fail though because it is one skipped terminal interaction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, getting this error:

=== RUN   Test_Validate/Valid_Init_Command
    shared.go:73: assertion failed: expected error to contain "cluster unreachable", got "KubeContext not specified": Error should be: kubernetes cluster unreachable, got: KubeContext not specified

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be good now

values = append(values, defaultValue)
for k := range kubeContexts {
values = append(values, k)
if k != currentContext {
values = append(values, k)
}
}
index, _, err := prompter.RunSelect(prompt.SelectionPrompter(
"Select the context for radius installation",
"Select the kubeconfig context to install Radius installation",
values,
))
if err != nil {
Expand Down
16 changes: 8 additions & 8 deletions pkg/cli/cmd/radInit/init_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func Test_Run(t *testing.T) {

func initMocksWithoutCloudProvider(kubernetesMock *kubernetes.MockInterface, prompterMock *prompt.MockInterface, helmMock *helm.MockInterface) {
initGetKubeContextSuccess(kubernetesMock)
initDefaultKubeContextPromptYes(prompterMock)
initKubeContextWithKind(prompterMock)
initEnvNamePrompt(prompterMock)
initNameSpacePrompt(prompterMock)
initAddCloudProviderPromptNo(prompterMock)
Expand All @@ -179,13 +179,13 @@ func initMocksWithKubeContextSelectionError(kubernetesMock *kubernetes.MockInter

func initMocksWithErrorEnvNameRead(kubernetesMock *kubernetes.MockInterface, prompterMock *prompt.MockInterface) {
initGetKubeContextSuccess(kubernetesMock)
initDefaultKubeContextPromptYes(prompterMock)
initKubeContextWithKind(prompterMock)
initEnvNamePromptError(prompterMock)
}

func initMocksWithErrorNamespaceRead(kubernetesMock *kubernetes.MockInterface, prompterMock *prompt.MockInterface) {
initGetKubeContextSuccess(kubernetesMock)
initDefaultKubeContextPromptYes(prompterMock)
initKubeContextWithKind(prompterMock)
initEnvNamePrompt(prompterMock)
initNameSpacePromptError(prompterMock)
}
Expand Down Expand Up @@ -214,16 +214,16 @@ func getTestKubeConfig() *api.Config {
}
}

func initDefaultKubeContextPromptYes(prompter *prompt.MockInterface) {
func initDefaultKubeContextPromptNo(prompter *prompt.MockInterface) {
prompter.EXPECT().
RunPrompt(gomock.Any()).
Return("Y", nil).Times(1)
Return("N", nil).Times(1)
}

func initDefaultKubeContextPromptNo(prompter *prompt.MockInterface) {
func initKubeContextWithKind(prompter *prompt.MockInterface) {
prompter.EXPECT().
RunPrompt(gomock.Any()).
Return("N", nil).Times(1)
RunSelect(gomock.Any()).
Return(2, "", nil).Times(1)
}

func initKubeContextSelectionError(prompter *prompt.MockInterface) {
Expand Down
13 changes: 7 additions & 6 deletions pkg/cli/prompt/prompt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,16 +166,16 @@ func (i *Impl) RunPrompt(prompter promptui.Prompt) (string, error) {
return prompter.Run()
}

//Prompts user to select from a list of values
// Prompts user to select from a list of values
func (i *Impl) RunSelect(selector promptui.Select) (int, string, error) {
return selector.Run()
}

// Creates a Yes or No prompts where user has to enter either a Y or N as input
func YesOrNoPrompter(label string, defaultValue string, prompter Interface) (string, error) {
textPrompt := promptui.Prompt{
Label: label,
Default: defaultValue,
Label: label,
Default: defaultValue,
Validate: validateYesOrNo,
}
result, err := prompter.RunPrompt(textPrompt)
Expand All @@ -197,8 +197,8 @@ func validateYesOrNo(s string) error {
// Creates a prompt where a user is asked for a value suggesting a default Val
func TextPromptWithDefault(label string, defaultVal string, f func(s string) (bool, string, error)) promptui.Prompt {
return promptui.Prompt{
Label: label,
Default: defaultVal,
Label: label,
Default: defaultVal,
Validate: func(s string) error {
valid, msg, err := f(s)
if err != nil {
Expand All @@ -209,6 +209,7 @@ func TextPromptWithDefault(label string, defaultVal string, f func(s string) (bo
}
return nil
},
AllowEdit: true,
}
}

Expand All @@ -218,7 +219,7 @@ func SelectionPrompter(label string, items []string) promptui.Select {
Label: label,
Items: items,
Searcher: func(input string, index int) bool {
return strings.HasPrefix(items[index],input)
return strings.HasPrefix(items[index], input)
},
}
}