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

What's the life cycle of the menu? #182

Closed taytus closed 5 years ago

taytus commented 5 years ago

Hello!

I'm having a problem where the menu won't close or won't exit the routine and it's a bit confusing to me why that is happening.

I have a small method that creates a menu:

//$options is a one level array of strings

public function popup_menu($options){
        $itemCallable = function (CliMenu $menu) {
            $menu->close();
            $this->latest_menu_selection=$menu->getSelectedItem()->getText();
        };

        $menu = (new CliMenuBuilder)
            ->disableDefaultItems()
            ->setTitle('Basic CLI Menu');
        foreach ($options as $item){
            $menu->addItem($item, $itemCallable);
        }
        $menu->setBorder(1, 2, 'yellow')
            ->setPadding(2, 4)
            ->setMarginAuto()
            ->addItem('Exit', new ExitAction) //add an exit button
            ->build()->open();
        return $this->latest_menu_selection;
    }

In the same class, I call this method twice with different $options.

public function show_urls_menu(){

        $this->popup_menu(["Blue","Green"]);
        $this->popup_menu(['Apple',"oranges"]);
}

If I don't stop the execution, after the second menu, it loads again the first menu and the second menu 2 times.

What am I doing wrong?

Thanks!

AydinHassan commented 5 years ago

It doesn't look like you are doing anything wrong on quick glance. Could be a bug. I'l have a look now.

AydinHassan commented 5 years ago

Seem to work fine (on latest master) for me with the following. If I select Blue then Red. It prints out:

string(4) "Blue"
string(3) "Red"
<?php

use PhpSchool\CliMenu\Action\ExitAction;
use PhpSchool\CliMenu\Builder\CliMenuBuilder;
use PhpSchool\CliMenu\CliMenu;

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

class Menu {
    private $latest_menu_selection;

    public function popup_menu($options){
        $itemCallable = function (CliMenu $menu) {
            $menu->close();
            $this->latest_menu_selection=$menu->getSelectedItem()->getText();
        };

        $menu = (new CliMenuBuilder)
            ->disableDefaultItems()
            ->setTitle('Basic CLI Menu');

        foreach ($options as $item){
            $menu->addItem($item, $itemCallable);
        }

        $menu->setBorder(1, 2, 'yellow')
            ->setPadding(2, 4)
            ->setMarginAuto()
            ->addItem('Exit', new ExitAction) //add an exit button
            ->build()->open();

        return $this->latest_menu_selection;
    }
}

$menu = new Menu;

var_dump($menu->popup_menu(['Blue', 'Green']));
var_dump($menu->popup_menu(['Red', 'Orange']));

What version are you using?

taytus commented 5 years ago

I'm using 3.2. I didn't create a class as you did. I mean my method calls a method within its class. I'm going to try that approach but it still is not clear why the menu re-draws twice.

AydinHassan commented 5 years ago

If you have code that is doing something different can you post as well so I can debug. If it's redrawing like you said then it's probably a bug but what you gave me works fine on 3.2.0.

taytus commented 5 years ago

I'm going to uninstall it and install it again and will update you. Thank you @AydinHassan

taytus commented 5 years ago

I'm still having the same issue. I will modify my code to its barebones and will share it here.

One question thou:

In your example, you said this code:

$menu = new Menu;

var_dump($menu->popup_menu(['Blue', 'Green']));
var_dump($menu->popup_menu(['Red', 'Orange']));

returns:

string(4) "Blue"
string(3) "Red"

Shouldn't only return "Red" since it's the same Menu instance and you are modifying the latest_menu_selection variable?

That is what I'm getting.

AydinHassan commented 5 years ago

Not sure I'm following. $latest_menu_selection is just a string, so first time popup_menu runs I select Blue and then it sets $latest_menu_selection to Blue and prints it. Then next time popup_menu runs I select Red and it sets $latest_menu_selection to Red and prints it.

taytus commented 5 years ago

I think it has something to do with Laravel, but I'm not sure.

I created a new laravel app, here is the code: https://github.com/taytus/menu

Run the menu with php artisan menu:show

Inside app\Console\Commands\my_menu.php I have this code:

public function handle(){
        $menu = new Menu();
        var_dump($menu->popup_menu(["black","white"]));
        var_dump($menu->popup_menu(["blue","red"]));
    }

And only returns the latest selection.

AydinHassan commented 5 years ago

Ok when I run that and select black then blue the output is

string(5) "black"
string(4) "blue"

Is that not what you expect? Or is that not what you get?

taytus commented 5 years ago

That is what I expect, but I'm only getting the latest option. So weird!

https://youtu.be/vnl8D01RDeA

AydinHassan commented 5 years ago

Can you try scrolling up the terminal to check because the terminal is cleared when opening a new menu, I have to scroll up to get the result.

taytus commented 5 years ago

@AydinHassan yeah, I figured it was that 😅

So now that part is solved, I have to check again why the menu keeps looping.

I'm going to keep this ticket open and will update with my finding.

Thanks!

AydinHassan commented 5 years ago

Sure no problem :)

taytus commented 5 years ago

I'm closing this because I found out the culprit of this bug.

My code is calling twice the method in charge of painting the menu 😭

Thank you @AydinHassan for your help!

AydinHassan commented 5 years ago

It's okay. Glad you sorted!