Add support for multiple selection in prompt #38

lavoiesl · 2018-09-14T15:25:16Z

Returns an array of all selected options, potentially empty

Verified with scrolling, "Done" moves up.

Edit: Changed icons and style:

lavoiesl · 2018-09-14T15:33:51Z

Icon taken from https://github.com/piotrmurach/tty-prompt

lugray · 2018-09-14T15:37:17Z

lib/cli/ui/prompt.rb

@@ -131,23 +131,36 @@ def ask_interactive(question, options = nil)
          end

          raise(ArgumentError, 'insufficient options') if options.nil? || options.size < 2
-          puts_question("#{question} {{yellow:(choose with ↑ ↓ ⏎)}}")
-          resp = interactive_prompt(options)
+          instructions = (multiple ? "Toogle options. " : "") + "Choose with ↑ ↓ ⏎"


Lol, thanks.

lib/cli/ui/prompt/interactive_options.rb

lugray · 2018-09-14T15:50:27Z

lib/cli/ui/prompt/interactive_options.rb

@@ -4,6 +4,9 @@ module CLI
  module UI
    module Prompt
      class InteractiveOptions
+        DONE = "Done"
+        CHECKBOX_ICON = { false => "⬡", true => "⬢" }


Could we use CHECKBOX_ICON = { false => "☐", true => "☑" } instead? The hexagons look too much like circles => radio boxes => single selection.

The box is a little small though:

What do you think?

Not married to that pair in particular - open to other options, but I think it's better than something that looks like single selection.

I couldn't find a better option, so I went with the checkbox.

lugray · 2018-09-14T15:51:28Z

lib/cli/ui/prompt/interactive_options.rb

@@ -214,6 +246,7 @@ def render_options
          presented_options(recalculate: true).each do |choice, num|
            padding = ' ' * (max_num_length - num.to_s.length)
            message = "  #{num}#{num ? '.' : ' '}#{padding}"
+            message += " #{CHECKBOX_ICON[@chosen[num - 1]]} " if @multiple && num && num > 0


Can we add colour as well?

Not sure what's best. Yellow/Green? On the checkbox only or the whole line? Perhaps make the line's text bold only if selected?

I'd say whole line, but not super opinionated on how exactly we style it, just that some kind of styling seems like the dev way. 😛

I went for "standard vs bold+cyan", other options looked like a Christmas tree. See screenshot in description.

lib/cli/ui/prompt/interactive_options.rb

jules2689

Seems generally ok. Wouldnt mind comments surrounding the 1 vs 0 in some places, but otherwise lgtm

lugray · 2018-09-19T11:52:02Z

🎩 ❌
When the full set of options don't fit on screen (with multiple: true):

NoMethodError: undefined method `-' for nil:NilClass
	from /Users/lisaugray/src/github.com/Shopify/cli-ui/lib/cli/ui/prompt/interactive_options.rb:254:in `block in render_options'
	from /Users/lisaugray/src/github.com/Shopify/cli-ui/lib/cli/ui/prompt/interactive_options.rb:253:in `each'
	from /Users/lisaugray/src/github.com/Shopify/cli-ui/lib/cli/ui/prompt/interactive_options.rb:253:in `render_options'
	from /Users/lisaugray/src/github.com/Shopify/cli-ui/lib/cli/ui/prompt/interactive_options.rb:57:in `call'
	from /Users/lisaugray/src/github.com/Shopify/cli-ui/lib/cli/ui/prompt/interactive_options.rb:23:in `call'
	from /Users/lisaugray/src/github.com/Shopify/cli-ui/lib/cli/ui/prompt.rb:163:in `interactive_prompt'
	from /Users/lisaugray/src/github.com/Shopify/cli-ui/lib/cli/ui/prompt.rb:136:in `ask_interactive'
	from /Users/lisaugray/src/github.com/Shopify/cli-ui/lib/cli/ui/prompt.rb:80:in `ask'
	from (irb):14
	from bin/console:14:in `<main>'

lavoiesl · 2018-09-19T13:44:26Z

I had tested it, but I broke it during a refactor :(
Good catch, fixed.

lugray

🎩 🥗

Returns an array of all selected options, potentially empty

sander-m · 2018-09-20T21:20:32Z

👀 This is really cool

lavoiesl force-pushed the seb/multiple_prompt branch 2 times, most recently from 91300f5 to 6c02ea3 Compare September 14, 2018 15:28

lavoiesl requested review from jules2689 and lugray September 14, 2018 15:34

lugray reviewed Sep 14, 2018

View reviewed changes

jules2689 approved these changes Sep 17, 2018

View reviewed changes

lugray approved these changes Sep 19, 2018

View reviewed changes

Add support for multiple selection in prompt

405b40f

Returns an array of all selected options, potentially empty

lavoiesl force-pushed the seb/multiple_prompt branch from 3929ad1 to 405b40f Compare September 19, 2018 14:35

lavoiesl merged commit 721e9fc into master Sep 19, 2018

lavoiesl deleted the seb/multiple_prompt branch September 19, 2018 14:36

lugray temporarily deployed to production November 9, 2018 18:39 Inactive

lugray mentioned this pull request Nov 13, 2018

[Feature] Multiple Selection Prompt #47

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for multiple selection in prompt #38

Add support for multiple selection in prompt #38

lavoiesl commented Sep 14, 2018 •

edited

Loading

lavoiesl commented Sep 14, 2018

lugray Sep 14, 2018

lavoiesl Sep 14, 2018

lugray Sep 14, 2018

lavoiesl Sep 14, 2018

lugray Sep 17, 2018

lavoiesl Sep 18, 2018

lugray Sep 14, 2018

lavoiesl Sep 14, 2018

lugray Sep 17, 2018

lavoiesl Sep 18, 2018

jules2689 left a comment

lugray commented Sep 19, 2018 •

edited

Loading

lavoiesl commented Sep 19, 2018

lugray left a comment

sander-m commented Sep 20, 2018

Add support for multiple selection in prompt #38

Add support for multiple selection in prompt #38

Conversation

lavoiesl commented Sep 14, 2018 • edited Loading

lavoiesl commented Sep 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jules2689 left a comment

Choose a reason for hiding this comment

lugray commented Sep 19, 2018 • edited Loading

lavoiesl commented Sep 19, 2018

lugray left a comment

Choose a reason for hiding this comment

sander-m commented Sep 20, 2018

lavoiesl commented Sep 14, 2018 •

edited

Loading

lugray commented Sep 19, 2018 •

edited

Loading