Error with toggling item extra

[polothy]

Use case: I was writing up a menu where I want the user to select multiple menu items. I was using the ExtraItem to indicate which menu items were selected.

Here is an example to reproduce the problem:

<?php

use PhpSchool\CliMenu\CliMenu;
use PhpSchool\CliMenu\CliMenuBuilder;
use PhpSchool\CliMenu\MenuItem\MenuItemInterface;

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

$itemCallable = function (CliMenu $menu) use ($selected) {
    if ($menu->getSelectedItem()->showsItemExtra()) {
        $menu->getSelectedItem()->hideItemExtra();
    } else {
        $menu->getSelectedItem()->showItemExtra();
    }
    $menu->redraw();
};

$menu = (new CliMenuBuilder)
    ->setTitle('Basic CLI Menu Custom Item Extra')
    ->addItem('First Item', $itemCallable)
    ->addItem('Second Item', $itemCallable)
    ->addItem('Third Item', $itemCallable)
    ->addLineBreak('-')
    ->setItemExtra('**')
    ->build();

$menu->open();

The primary take aways in the above example are:

• Initially, every item has ItemExtra false
• In item callback, we call showItemExtra and do a redraw.

The result is a PHP warning: Warning: str_repeat(): Second argument has to be greater than or equal to 0 in /path/to/cli-menu/src/CliMenu.php on line 319 and 319 is the following line of code in CliMenu::drawMenuItem:

str_repeat(' ', $this->style->getRightHandPadding(mb_strlen(s::stripAnsiEscapeSequence($row)))),

In addition, the menu renders incorrectly, the ItemExtra renders outside of the menu box.

Hope that's enough to go on, cheers! Great menu system, thanks!

[mikeymike]

hey thanks for the detailed report 😄 I'll take a look at this asap (probably on my lunch break today 👍 )

[mikeymike]

hey @mrmark narrowed this down a little more but not got a solution atm. Essentially we're building the MenuStyle with the item extra as not being displayed. This is then used in \PhpSchool\CliMenu\MenuItem\SelectableTrait::getRows when calculating the content width. Because the MenuStyle isn't being updated to say we're now displaying the item extra it's incorrectly calculating the content width by + the length of the item extra.

Will take another look asap to see if there's an easy fix for this.

[mikeymike]

@mrmark a quick way to get this working would be to add $menu->getStyle()->setDisplaysExtra(true); in the callable when you toggle a row to show.

I don't think this is the right solution and the real fix would be to have the logic for checking if the item extra is shown being taken from the row itself rather than the MenuStyle.

[polothy]

That's fine by me, thanks. No need to rush a fix, guessing this doesn't happen very much in practice. I also had another workaround in place that works as well, though yours sounds much better.

[AydinHassan]

Finally fixed with #187!