-
Notifications
You must be signed in to change notification settings - Fork 94
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
Allow selecting all options in a multiselect
#147
Allow selecting all options in a multiselect
#147
Conversation
Hey @duncanmcclean, This is pretty cool! My only real issue is the "All" option is included in the returned array, which could cause problems depending on what the user is doing with the response. We'd need to be careful filtering it out, though, in case someone already has their own "All" option which they expect to be returned. I don't love that it gets injected into the I think the new parameter should also accept a string, allowing folks to change or localize the wording (similar to the |
How about toggling when [Ctrl]+[A] is pressed? |
Ah yeah, that's a problem. I can probably refactor the "All" option to live in the renderer instead, rather than being merged into the actual options of the prompt.
Good idea 👍 I also don't completely love the name of the parameter but it's the best I could come up with at the time. I'll see if I can think of a better name.
That's a good idea. In fact, I think I actually prefer it to what I've done in this PR in adding a separate option. However, it seems like there's already a keybinding for |
# Conflicts: # src/helpers.php
I've finally managed to carve out some time to work on refactoring this PR. I've renamed the property to I've also started to dive into moving the "select all" logic into the renderer, as suggested. However, I'm struggling to figure out the best way to prepend an option to the list without actually merging it into I'm likely missing something obvious, so if there's any pointers you can give me, that'd be great. Like I mentioned in my previous comment, I actually quite like (or might even prefer) it being a keyboard shortcut, rather than a separate visual option. It would actually be very simple to implement (I built it in 5 minutes as a POC 😆 ). However, it looks like |
Keyboard shortcuts can be funny in the terminal. I believe it technically only applies to typed input (i.e. horizontal movement), so I'm open to remapping it. I'm thinking the following could be good:
I'm hoping this is pretty straight-forward and would also remove the need for a parameter to be added, as this feature would always be active. |
Although... Ctrl+A should only select all options when the user is in the context of the options, not when they're in the context of searching.
@jessarcher Thanks for your suggestions - I've updated this PR to use keyboard shortcuts, rather than the separate "All" option. This PR is ready for review, whenever you're able. |
Thanks @duncanmcclean! I've made a few minor tweaks to the key ordering just to keep related things together in my head. Just need to figure out the right approach for that |
…mpts into multiselect-select-all
(I've fixed the phpstan issue in the |
Co-authored-by: Jess Archer <jess@jessarcher.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Really dig this feature.
There is an example in the documentation for a Great feature anyway! |
It looks like this parameter was reverted and replaced with a keyboard shortcut. See the PR here: laravel/prompts#147
It looks like this parameter was reverted and replaced with a keyboard shortcut. See the PR here: laravel/prompts#147
Currently, when using the
multiselect
component, if you want to select all of the available options, you have to manually hitspace + down
until you've toggled all the options.This is mostly fine. However, if you have 10 options, you might not want to force your users to manually select all the options. Instead, you might want to let them toggle a single option.
This PR adds a new
canSelectAll
property to theMultiSelectPrompt
class, which iftrue
, will show an "All" option at the top of the options:When enabled, it'll select all the options and when disabled it'll deselect them all. In the case the user enables the "All" option but subsequently goes to select one of the options, the "All" option will be deselected.
An alternative UX for this could be to somehow disable all of the other options when the "All" option is toggled so they can't be changed.
Let me know if you'd like me to make any changes and I'll do my best to do them. Otherwise, if this isn't something you want to add to Prompts, then that's fine as well, just an idea I had. ❤️