getSelectedItem() makes it impossible to group split items / Radio items

[jtreminio]

https://github.com/php-school/cli-menu/blob/74dca3eb0a73fb816a446f8ba3bee552e2656f56/src/CliMenu.php#L384

If you have a group of MenuItem in a SplitItem calling CliMenu::getSelectedItem() returns the actual MenuItem, not the SplitItem group. This is fine.

However, there is currently no way of fetching the SplitItem group from within a callback:

<?php

use PhpSchool\CliMenu\Builder\SplitItemBuilder;
use PhpSchool\CliMenu\CliMenu;
use PhpSchool\CliMenu\Builder\CliMenuBuilder;
use PhpSchool\CliMenu\MenuItem\CheckableItem;

require_once(__DIR__ . '/../vendor/autoload.php');

$itemCallable = function (CliMenu $menu) {
    /** @var CheckableItem $item */
    $item = $menu->getSelectedItem();

    // Unable to access SplitItem from here
};

$menu = (new CliMenuBuilder)
    ->setTitle('Select a Language')
    ->addSplitItem(function (SplitItemBuilder $b) use ($itemCallable) {
        $b->setGutter(5)
            ->addCheckableItem('Rust', $itemCallable)
            ->addCheckableItem('C++', $itemCallable)
            ->addCheckableItem('Go', $itemCallable)
            ->addCheckableItem('Java', $itemCallable)
            ->addCheckableItem('C', $itemCallable)
        ;
    })
    ->build();

$menu->open();

If we add the following to CliMenu we can work around this problem:

    public function getSelectedSplitItem() : MenuItemInterface
    {
        if (null === $this->selectedItem) {
            throw new \RuntimeException('No selected item');
        }

        return $this->items[$this->selectedItem];
    }

and changing our code to reflect this:

<?php

use PhpSchool\CliMenu\Builder\SplitItemBuilder;
use PhpSchool\CliMenu\CliMenu;
use PhpSchool\CliMenu\Builder\CliMenuBuilder;
use PhpSchool\CliMenu\MenuItem\CheckableItem;
use PhpSchool\CliMenu\MenuItem\SplitItem;

require_once(__DIR__ . '/../vendor/autoload.php');

$itemCallable = function (CliMenu $menu) {
    /** @var CheckableItem $selected */
    $selected = $menu->getSelectedItem();

    /** @var SplitItem $splitGroup */
    $splitGroup = $menu->getSelectedSplitItem();

    $selected->toggle();

    /** @var CheckableItem $item */
    foreach ($splitGroup->getItems() as $item) {
        if (!is_a($item, CheckableItem::class)) {
            continue;
        }

        if ($item === $selected) {
            continue;
        }

        $item->setUnchecked();
    }

    $menu->redraw();
};

$menu = (new CliMenuBuilder)
    ->setTitle('Select a Language')
    ->addSplitItem(function (SplitItemBuilder $b) use ($itemCallable) {
        $b->setGutter(5)
            ->addCheckableItem('Rust', $itemCallable)
            ->addCheckableItem('C++', $itemCallable)
            ->addCheckableItem('Go', $itemCallable)
            ->addCheckableItem('Java', $itemCallable)
            ->addCheckableItem('C', $itemCallable)
        ;
    })
    ->build();

$menu->open();

results in radio-style elements:

Thoughts?

edit: Adding CheckableItem via SplitItemBuilder is part of PR #189.

This issue that you are reading now deals with being able to group items together by fetching SplitItem from CliMenu. It can also be applied to normal SelectableItem.

[AydinHassan]

Maybe something like

public function getSelectedItemIndex() : int
{
    if (null === $this->selectedItem) {
        throw new \RuntimeException('No selected item');
    }

    return $this->selectedItem;
}

public function getItemByIndex(int $index) : MenuItemInterface
{
    if (!isset($this->items[$index])) {
        throw new \RuntimeException('Item with index does not exist');
    }

    return $this->items[$index];
}

So we don't have to tie it to the split item in particular?

[AydinHassan]

Also, thinking about this. You could implement this as separate item type and do the unselecting as part of the items action and keep the logic nicely separated to the item type like so:

<?php

namespace PhpSchool\CliMenu\MenuItem;

use PhpSchool\CliMenu\CliMenu;
use PhpSchool\CliMenu\MenuItem;
use PhpSchool\CliMenu\MenuStyle;
use PhpSchool\CliMenu\Util\StringUtil;

class RadioItem extends CheckableItem
{
    /**
     * Execute the items callable if required
     */
    public function getSelectAction() : ?callable
    {
        return function (CliMenu $cliMenu) {
            $parentItem = $cliMenu->getItemByIndex($cliMenu->getSelectedItemIndex());
            array_walk(
                array_filter(
                    $parentItem->getItems(),
                    function (MenuItem\MenuItemInterface $item) {
                        return $item instanceof self;
                    }
                ),
                function (CheckableItem $checkableItem) {
                    $checkableItem->setUnchecked();
                }
            );

            $this->setChecked();
            $cliMenu->redraw();
            parent::getSelectAction()($cliMenu);
        };
    }
}

[jtreminio]

Good idea on auto-managing state.

[jtreminio]

Should CheckableItem and RadioItem share the same MenuStyle::checkedMarker and MenuStyle::uncheckedMarker or be separate?

I'm leaning towards separate.

[AydinHassan]

I'm fine with either :)

[jtreminio]

Merged in #193