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

Make interactive confirm optionally accept return key for confirmation #644

Open
elfrucool opened this issue Feb 19, 2024 · 7 comments · May be fixed by #648
Open

Make interactive confirm optionally accept return key for confirmation #644

elfrucool opened this issue Feb 19, 2024 · 7 comments · May be fixed by #648
Labels
proposal Proposal to add a new feature to pterm proposal-accepted Proposals which are accepted for implementation in the future

Comments

@elfrucool
Copy link

For some users, it is confusing that the interactive confirm component accepts their y or n answer immediately without requiring a confirmation through typing <enter> key; this is specially true when adopting pterm the first time.

It would be great to have something like pterm.DefaultInteractiveConfirm.WithWaitReturn(true).Show() to enable this behavior.

(note: this feature request came from this question: #643 )

@elfrucool elfrucool added the proposal Proposal to add a new feature to pterm label Feb 19, 2024
@MarvinJWendt MarvinJWendt added the proposal-accepted Proposals which are accepted for implementation in the future label Feb 19, 2024
@KarolosLykos KarolosLykos linked a pull request Mar 8, 2024 that will close this issue
@KarolosLykos
Copy link
Contributor

@elfrucool I've added a draft that includes this new functionality. Would you like to take a look and offer some feedback?

@elfrucool
Copy link
Author

I'll take a look once I get a moment

@KarolosLykos
Copy link
Contributor

Thanks for the feedback! I understand your point. Although, in situations where pressing Enter is associated with confirmation, it might be confusing if the default action is "No" or "Cancel".

Also, most CLI prompts don't include a "confirmation helper text", so it might not be necessary here.

In my view, removing the helper text seems like the better option. However, let's wait for the maintainers to decide if they prefer your suggestion (adds more complexity to the printer) or if it's best to remove "confirmation helper text" altogether.

@KarolosLykos
Copy link
Contributor

@MarvinJWendt, could you please share your thoughts on this?

@MarvinJWendt
Copy link
Member

What I could imagine is this:

  • New option in the InteractiveConfirmPrinter called InstantSelect
    • If InstantSelect is true: Pressing y or n will instantly confirm or abort the printer.
    • If QuickSelect is false: Return is needed after pressing y or n to select
      • While Return is not pressed, the user should be able to switch between y or n
  • InstantSelect should be true in DefaultInteractiveConfirm
    • That way, it can be disabled with .WithInstantSelect(false).

Any objections? :)

@elfrucool
Copy link
Author

elfrucool commented Mar 25, 2024 via email

@KarolosLykos
Copy link
Contributor

I had a similar idea, and I agree with @elfrucool. It seems more logical to have it disabled by default. In my PR, I added a new option WithConfirmation(true) to pterm.DefaultInteractiveConfirm to enable the behavior of waiting for the user to confirm their answer by pressing . Of course, naming is subject to change. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Proposal to add a new feature to pterm proposal-accepted Proposals which are accepted for implementation in the future
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants