IngoMeyer441 / simple-term-menu

A Python package which creates simple interactive menus on the command line.
MIT License
505 stars 44 forks source link

Multi-Select and Search Interaction for Menus #25

Closed kctrey closed 3 years ago

kctrey commented 3 years ago

Currently, it doesn't look like search and multi-select work well together for searching large lists of entries and selecting them. For example, I have a list of all of the countries in the world. If I want to multi-select "China" and "United States", I can't search for them and select each. If I use the default 'space', it adds the space to the search string, obviously clearing the search results.

I have tried various 'multi_select_key' values to see if I can find one that won't be passed to the search but can't seem to find one. I keep thinking that I could pass the right arrow key, for example, as the multi_select_key, but simple_term_menu throws an exception if you press left/right arrow key during a search, regardless of what I have multi_select_key set to:

  File "/home/pi/.local/lib/python3.7/site-packages/simple_term_menu.py", line 1278, in show
    self._paint_menu()
  File "/home/pi/.local/lib/python3.7/site-packages/simple_term_menu.py", line 1185, in _paint_menu
    displayed_menu_height += print_search_line(displayed_menu_height)
  File "/home/pi/.local/lib/python3.7/site-packages/simple_term_menu.py", line 948, in print_search_line
    self._tty_out.write((num_cols - len(self._search) - 1) * " ")
ValueError: __len__() should return >= 0

My workaround is to chain menus together to allow a user to select, either single or multi, and then re-show the menu to allow a new search, but it seems like there should be a way to support this; some kind of "rolling selection list" after a search is performed and a new search is started.

IngoMeyer441 commented 3 years ago

Hey and thanks for using my Python package! 🙂. It is true, multi-select and searching do not work well together. I think it would be quite easy to implement a key combination like Ctrl-Space to select an item. It will be more work to add a second list with already selected items which won't be cleared when the search string is modified (this is what you have in mind with "rolling selection list"?).

kctrey commented 3 years ago

I haven't dug into your code much yet, but I was going to fork it and see what I could make work.

As a side note, it appears that the search function is very susceptible to errors. I threw that exception when I pressed an arrow key, but I also threw different ones by mistake as I was testing. Might be something to investigate.

If I fork it and solve my search/multi-select problem, I will submit a pull request, unless it is something that you think you'll be tackling soon.

YAMLcase commented 3 years ago

I ran into this issue today and am glad someone else found it. From a UX perspective, hitting space for multi-selection is so natural I wouldn't be changing that to anything else. What I think is needed is some way to freeze the filtered list and exit the search function, preferably a toggle (same key used to enter and exit search).

Here's some of my thoughts on UX from a heavy linux keyboard user:

Thanks for building such an awesome module!

IngoMeyer441 commented 3 years ago

@kctrey I can fix the exceptions which are thrown on arrow keys. I don't know yet when I have time to work on bigger UI changes.

@YAMLcase As a daily vim user I like the idea of different modes but I am not sure if this is the case for all users. In the current stable version it is for example possible to navigate the cursor with Ctrl + j and Ctrl + k in search mode (this kind of search / select mix is also possible in fzf which is another tool I use very often). In fzf the tab key is used for multi-selection which could also be an option for simple-term-menu.

So my current suggestion to fix this issue is:

Do we also need another list in the UI which shows already selected items? If yes, that could maybe implemented in a second step.

What do you think about this?

YAMLcase commented 3 years ago

@IngoMeyer441 I experimented with hard-coding a couple of changes to use tab and it worked reasonably well with the exception of a UI bug (details and changes below). I like the fzf style of navigation and multi-selection, so if you just emulated that I think we'd be golden. In fact, it didn't take me long to train my brain for ctrl+j/k and tab to make fzf feel natural, so I don't think you'd need the search mode if you went that route (thanks for turning me onto that app btw, it's quite neat!).

-DEFAULT_MULTI_SELECT_KEY = " "
+DEFAULT_MULTI_SELECT_KEY = "\t"

...

                string_to_key = {
                     " ": "space",
+                    "\t": "tab"
                 }
>>> menuitems = ['zero', 'one', 'two', 'three', 'four', 'five']
>>> menu = TerminalMenu(menuitems, multi_select=True, show_multi_select_hint=True)
>>> menu.show()

> zero
  one
  two
  three
  four
  five
Press <tab> for multi-selection and <enter> to select and accept

searching for "f" and selecting the found items results in the selection markers showing up in the original non-filtered position:

  four
> five

*
*
/f
Press <tab> for multi-selection and <enter> to select and accept

result:

(4, 5)
IngoMeyer441 commented 3 years ago

@YAMLcase Thanks for your feedback! :smiley: I adopted your changes, added some modifications and fixed the UI bug. So, the current development version should better integrate searching and multi-selections. Could you please run a test? You can install the development version directly from GitHub with:

python3 -m pip install git+https://github.com/IngoMeyer441/simple-term-menu@develop
IngoMeyer441 commented 3 years ago

@kctrey Using arrow keys or other non-printable characters in the search mode do also not throw exceptions any more in the latest develop.

YAMLcase commented 3 years ago

works just like I would expect and no UI bug, great work!

21 is probably the next UX challenge as I have been suffering the same friction as the reporter, but I'll make my comments there.

IngoMeyer441 commented 3 years ago

Thanks for testing. :+1: I thought #21 had been fixed. But of course feel free to reopen or open another issue if the patch still has problems.

YAMLcase commented 3 years ago

it works great, I missed that PR was accepted. that's what I get for not reading more thoroughly

IngoMeyer441 commented 3 years ago

The changes are now in the latest v1.2.0 release. Feel free to reopen if you still miss anything or find any bugs.