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

Item-level style example; fixes disappearing styles for nested #203

Closed jtreminio closed 4 years ago

jtreminio commented 4 years ago

Not ready for merge. Needs feedback, unit tests and general cleanup.

jtreminio commented 4 years ago

Basically what I've changed is created three new style objects:

Each CliMenu keeps a copy of each of these styles to use as its base for all current-level and children items (checkable, radio and selectable items).

So an example CheckableItem would be:

<?php

use PhpSchool\CliMenu\CliMenu;
use PhpSchool\CliMenu\Builder\CliMenuBuilder;
use PhpSchool\CliMenu\Style\CheckableStyle;

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

$itemCallable = function (CliMenu $menu) {
    echo $menu->getSelectedItem()->getText();
};

$menu = (new CliMenuBuilder)
    ->setTitle('Select a Language')
    ->addSubMenu('Compiled', function (CliMenuBuilder $b) use ($itemCallable) {
        $b->setTitle('Compiled Languages')
            ->addCheckableItem('Rust', $itemCallable)
            ->addCheckableItem('C++', $itemCallable)
            ->addCheckableItem('Go', $itemCallable)
            ->addCheckableItem('Java', $itemCallable)
            ->addCheckableItem('C', $itemCallable)
        ;
    })
    ->addSubMenu('Interpreted', function (CliMenuBuilder $b) use ($itemCallable) {
        $b->setTitle('Interpreted Languages')
            ->checkableStyle(function (CheckableStyle $style) {
                $style->setMarkerOff('[β—‹] ')
                    ->setMarkerOn('[●] ');
            })
            ->addCheckableItem('PHP', $itemCallable)
            ->addCheckableItem('Javascript', $itemCallable)
            ->addCheckableItem('Ruby', $itemCallable)
            ->addCheckableItem('Python', $itemCallable)
        ;
    })
    ->build();

$menu->open();

The style classes only care about styles related to their item type. Right now they're identical, only difference are the marker texts.

You'll be able to deprecate the following in MenuStyle:

since these are item-level styles. Note that not all item types have moved off of MenuStyle - this is just the start of the PR, pending feedback.

Since the style classes all share the same interface, we're able to get rid of setUncheckedMarker, getUnchecked and similar.


Aside from this, I was able to fix the #201 issue.

CliMenuBuilder::addSubMenu() and CliMenuBuilder::addSubMenuFromBuilder() now inject a closure to MenuMenuItem(). Then, when the menu is first opened and MenuMenuItem::showSubMenu() is called then the closure is unpackaged and the menu generated.

This helps avoid creating the menu code ahead of time, and uses style values that are set after all PHP code has been read.

codecov-io commented 4 years ago

Codecov Report

Merging #203 into master will decrease coverage by 2.05%. The diff coverage is 85.04%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #203      +/-   ##
============================================
- Coverage     93.91%   91.85%   -2.06%     
- Complexity      516      554      +38     
============================================
  Files            29       35       +6     
  Lines          1561     1744     +183     
============================================
+ Hits           1466     1602     +136     
- Misses           95      142      +47
Impacted Files Coverage Ξ” Complexity Ξ”
src/MenuItem/RadioItem.php 100% <100%> (ΓΈ) 6 <2> (-4) :arrow_down:
src/MenuItem/SelectableTrait.php 100% <100%> (ΓΈ) 11 <2> (+2) :arrow_up:
src/Style/SelectableStyle.php 100% <100%> (ΓΈ) 3 <3> (?)
src/Style/RadioStyle.php 100% <100%> (ΓΈ) 3 <3> (?)
src/CliMenu.php 95.34% <100%> (+0.29%) 115 <6> (+6) :arrow_up:
src/MenuItem/CheckableItem.php 100% <100%> (ΓΈ) 5 <4> (-4) :arrow_down:
src/Style/CheckableStyle.php 100% <100%> (ΓΈ) 3 <3> (?)
src/MenuItem/ToggableTrait.php 100% <100%> (ΓΈ) 15 <6> (+4) :arrow_up:
src/MenuStyle.php 96.61% <100%> (-0.67%) 60 <0> (-18)
src/MenuItem/SplitItem.php 100% <100%> (ΓΈ) 46 <1> (-1) :arrow_down:
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data Powered by Codecov. Last update a9e6a34...b08a081. Read the comment docs.

jtreminio commented 4 years ago

createMenuClosure() should be refactored into a trait or something else. IT's identical between CliMenuBuilder and SplitItemBuilder.

jtreminio commented 4 years ago

This PR provides item-level styles, and completely removes these same styles from MenuStyle.

To run a quick test:

<?php

use PhpSchool\CliMenu\CliMenu;
use PhpSchool\CliMenu\Builder\CliMenuBuilder;
use PhpSchool\CliMenu\MenuItem\SelectableItem;

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

$itemCallable = function (CliMenu $menu) {
    echo $menu->getSelectedItem()->getText();
};

$item1 = new SelectableItem('Item 1', function () {});
$item1->getStyle()
    ->setMarkerOff('! ')
    ->setMarkerOn('1 ');

$item2 = new SelectableItem('Item 2', function () {});
$item2->getStyle()
    ->setMarkerOff('@ ')
    ->setMarkerOn('2 ');

$item3 = new SelectableItem('Item 3', function () {});
$item3->getStyle()
    ->setMarkerOff('# ')
    ->setMarkerOn('3 ');

$menu = (new CliMenuBuilder)
    ->setTitle('Basic CLI Menu')
    ->addMenuItem($item1)
    ->addMenuItem($item2)
    ->addMenuItem($item3)
    ->addLineBreak('-')
    ->build();

$menu->open();

Peek 2019-12-18 22-20

AydinHassan commented 4 years ago

What I'd like to see here (after changing the checkableitem to checkbox item) a PR just changing checkbox item to this new individual style design. And then we can PR each item type one by one making it easier to review. I think it makes it easier to discuss the design that way. What do you think?

jtreminio commented 4 years ago

What I'd like to see here (after changing the checkableitem to checkbox item) a PR just changing checkbox item to this new individual style design. And then we can PR each item type one by one making it easier to review. I think it makes it easier to discuss the design that way. What do you think?

I'll redo the PR to slim it down in scope, but doing only Checkbox styling and leaving Radio and Selectable will be ugly as will need to do a bunch if if (... instanceof ...) checks.

I'll create the minimum for the Selectable and Radio styles and include in PR.