php-school / cli-menu

🖥 Build beautiful PHP CLI menus. Simple yet Powerful. Expressive DSL.
http://www.phpschool.io
MIT License
1.94k stars 106 forks source link

Allow the use of callables to determine whether a menu item is disabled or not #237

Closed JackWH closed 4 years ago

JackWH commented 4 years ago

It's easy enough to set a menu item as disabled, but what if you want to re-enable it following a user action within the menu? I couldn't see a clean way to do this (hopefully I'm not missing something...!), so this PR should fix it.

It simply changes the method definitions for addItem, addCheckboxItem, and addRadioItem to allow the $disabled parameter to be a boolean or a callable. When an item is selected that toggles the state of another item, a call to $menu->redraw() refreshes the screen and updates the status of the item.

Here's an example of how it works. I put another simple example in the README file too.

public bool $isChecked = false;

public function showMenu(): void
{
    $itemSelected     = function (CliMenu $menu) {
        $menu->redraw();
        $menu->flash('Item selected!')->display();
    };
    $checkboxSelected = function (CliMenu $menu) use ($itemSelected) {
        $this->isChecked = true;
        $itemSelected($menu);
    };

    (new CliMenuBuilder)
        ->setTitle('Callable Functions')
        ->addCheckboxItem(
            'Check this box...',
            $checkboxSelected,
            false,
            fn() => $this->isChecked
        )->addItem(
            'This item is enabled if the checkbox is not ticked',
            $itemSelected,
            false,
            fn() => $this->isChecked
        )->addRadioItem(
            'These radio buttons are enabled if the checkbox is selected',
            $itemSelected,
            false,
            fn() => ! $this->isChecked
        )->addRadioItem(
            'These radio buttons are enabled if the checkbox is selected',
            $itemSelected,
            false,
            fn() => ! $this->isChecked
        )
        ->build()
        ->open();
}
AydinHassan commented 4 years ago

I'm not entirely sold on this proposal as I think it makes the builder even more complicated and I already think there are too many parameters there, mixing the types of one one is even more confusing.

I think my approach to this problem would be to make the items' enabled/disabled status toggle-able. Then if you wanted to have some items depend on others you could you just keep a reference to those items inside your item and toggle their status there:

For example:

$itemSelected = function (CliMenu $menu) {
    $menu->redraw();
    $menu->flash('Item selected!')->display();
};

$item = new SelectableItem('This item is enabled if the checkbox is not ticked', $itemSelected);
$radio1 = new RadioItem('These radio buttons are enabled if the checkbox is selected', $itemSelected, false, true);
$radio2 = new RadioItem('These radio buttons are enabled if the checkbox is selected', $itemSelected, false, true);

$checkboxSelected = function (CliMenu $menu) use ($itemSelected, $item, $radio1, $radio2) {
    //could even just have a `toggleDisabled()` on all items with no need to check the current items status
    //needs a better name though
    if ($menu->getSelectedItem()->getChecked()) {
        $item->disable();
        $radio1->enable();
        $radio2->enable();
    }

    $itemSelected($menu);
};

$checkBoxItem = new CheckboxItem('Check this box...', $checkboxSelected);

(new CliMenuBuilder)
    ->setTitle('Callable Functions')
    ->addMenuItem($checkBoxItem)
    ->addMenuItem($item)
    ->addMenuItem($radio1)
    ->addMenuItem($radio2)
    ->build()
    ->open();

It's not quite as slick as your proposal, but I think it keeps the builder simpler, albeit offloading some complexity to the consumer, which I am fine with, since this is probably an edge case.

What do you think @JackWH ?

\cc @mikeymike for your thoughts

AydinHassan commented 4 years ago

Feel free to rework if you find the time @JackWH. Closing for now.