Add support for submenu #25
Conversation
Using the proposed change, we can add a submenu by making a multi-dimensional array; ``` $options = [ 'House' => [ 123 => 'Task 1', 124 => 'Task 2', 125 => 'Task 3', 126 => 'Task 4', ], 'Car' => [ 200 => 'Task 5', 201 => 'Task 6', 202 => 'Task 7', 203 => 'Task 8', ] ]; $taskId = $this->menu('Select task from category', $options)->open(); ``` 
| The idea is a good, but I think we do better on the public api you provided. cc @owenvoke |
| Go for it! :) |
| @nunomaduro @owenvoke do you have any guidelines on how to do better on the public api? Any suggestions? |
| @nunomaduro @owenvoke maybe it's an idea to merge this as long there is no better idea for a public api? |
| Hi, sorry I'm not sure how I completely missed this until now. 😬 I can't personally think of a better API at the moment, what sort of thing were you thinking of @nunomaduro? |
| * @return \NunoMaduro\LaravelConsoleMenu\Menu | ||
| */ | ||
| public function addOption($value, string $label): Menu | ||
| public function addOption($value, string $label, ?CliMenuBuilder $subMenu = null): Menu |
There was a problem hiding this comment.
Hey buddy, why we need the , ?CliMenuBuilder $subMenu = null?
There was a problem hiding this comment.
In the callback on line 92, I pass the Menu object so we can add options to the menu recursively.
There was a problem hiding this comment.
I assigned the values to new variables to make it a bit clearer what's happening.
There was a problem hiding this comment.
@nunomaduro @owenvoke any further thoughts on this? Trying to get a decision on this before I release my book (June 1st) so I can scratch it or leave it in or change it (because I mention this in my chapter on the menu add-on).
There was a problem hiding this comment.
Hey, sorry I seem to have a huge amount of notifications recently so think I missed this again. 🤦 Personally I think this seems fine.
There was a problem hiding this comment.
@nunomaduro i think your styleci might be misconfigured, it wants double spaces here;
Anyway, passes now. Weird.
There was a problem hiding this comment.
Double spaces is the Laravel standard. Tbh, we should probably migrate to Pint here.
Apparently it wants double spaces? Wut
3 participants
Using the proposed change, we can add a submenu by making a multi-dimensional array;