Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

Add a RemoveSelectAll and RemoveSelectNone config to multi-select #439

Merged
merged 3 commits into from
Sep 9, 2022

Conversation

ces131
Copy link
Contributor

@ces131 ces131 commented Aug 5, 2022

Adds a config to prevent users from selecting all of the survey options and a config to remove the "select none/de-select all" option in multiselect. Using survey.WithRemoveSelectAll() will disable the ability for the right arrow key to select all of the options. It will also remove the instructions from the multi-select prompt.
Similarly, survey.WithRemoveSelectNone() will disable not allow a user to select none/de-select all options and the instruction won't show up in the prompt.

Aurora Innovation has a patch with this work in their repo, and I figured I would make the change upstream. I filed a ticket quite awhile ago and finally got around to adding the change: #393

Test Plan

Ran all of the tests. Added a new test that verifies the correct prompt and checks that using the right arrow key doesn't select all the options and using the left arrow key doesn't de-select all the options.

@AlecAivazis
Copy link
Owner

Thanks for putting this together @ces131! Should removing the select all also remove the select none behavior? Maybe we want a second option while we're here?

@ces131
Copy link
Contributor Author

ces131 commented Aug 15, 2022

Thanks for putting this together @ces131! Should removing the select all also remove the select none behavior? Maybe we want a second option while we're here?

If you'd like, I can add an additional option RemoveSelectNone while I'm here - for our use case I think we'd like to keep the "none" option even with the SelectAll removed. Sometimes our mulit-select question is "preloaded"/has a few options pre-selected so a quick way for the user to remove all of those is a nice to have

@AlecAivazis
Copy link
Owner

Yea i think we should just for completeness. thanks!

@ces131 ces131 changed the title Add a RemoveSelectAll config to multi-select Add a RemoveSelectAll and RemoveSelectNone config to multi-select Aug 23, 2022
Copy link
Owner

@AlecAivazis AlecAivazis left a comment

Choose a reason for hiding this comment

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

Sorry this review is coming in two pieces but I've been struggling to find the time. Anyway, we're almost there - one small tweak and this is good to go

survey.go Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@ces131 ces131 requested a review from AlecAivazis September 8, 2022 14:56
Copy link
Owner

@AlecAivazis AlecAivazis left a comment

Choose a reason for hiding this comment

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

Looks good! thanks for updating the docs (and the ping 😅 )

@AlecAivazis AlecAivazis merged commit 55474c3 into AlecAivazis:master Sep 9, 2022
@ces131
Copy link
Contributor Author

ces131 commented Sep 9, 2022

Looks good! thanks for updating the docs (and the ping 😅 )

No problem at all! Thanks for the review 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants