Skip to content

Checkbox + Radio item-level styling #205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 10 commits into from
Closed

Checkbox + Radio item-level styling #205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e58bb70
Item styling for Checkbox and Radio items
jtreminio Dec 19, 2019
34ea4b3
Updates README
jtreminio Dec 19, 2019
6b72992
Removes extra code
jtreminio Dec 19, 2019
0c48311
Drop comments
AydinHassan Dec 19, 2019
8179b35
CR changes
jtreminio Dec 19, 2019
d3a76ba
Merge pull request #206 from jtreminio/feature/item-style-2
AydinHassan Dec 19, 2019
7a8a963
Updates README radioStyle -> modifyRadioStyle
jtreminio Dec 19, 2019
da43609
Merge pull request #207 from jtreminio/feature/item-style-2
AydinHassan Dec 19, 2019
0f1db5f
SplitItemBuilder should reference menu styles
jtreminio Dec 19, 2019
4ea4242
Merge pull request #208 from jtreminio/feature/item-style-2
AydinHassan Dec 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -817,10 +817,13 @@ You may also change the marker for `\PhpSchool\CliMenu\MenuItem\CheckboxItem`:
<?php

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

$menu = (new CliMenuBuilder)
->setUncheckedMarker('[○] ')
->setCheckedMarker('[●] ')
->modifyCheckboxStyle(function (CheckboxStyle $style) {
$style->setMarkerOff('[○] ')
->setMarkerOn('[●] ');
})
->build();
```

Expand All @@ -830,10 +833,13 @@ and for `\PhpSchool\CliMenu\MenuItem\RadioItem`:
<?php

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

$menu = (new CliMenuBuilder)
->setUnradioMarker('[ ] ')
->setRadioMarker('[✔] ')
->modifyRadioStyle(function (RadioStyle $style) {
$style->setMarkerOff('[ ] ')
->setMarkerOn('[✔] ');
})
->build();
```

Expand Down
7 changes: 5 additions & 2 deletions examples/checkable-item.php → examples/checkbox-item.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

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

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

Expand All @@ -22,8 +23,10 @@
})
->addSubMenu('Interpreted', function (CliMenuBuilder $b) use ($itemCallable) {
$b->setTitle('Interpreted Languages')
->setUncheckedMarker('[○] ')
->setCheckedMarker('[●] ')
->modifyCheckboxStyle(function (CheckboxStyle $style) {
$style->setMarkerOff('[○] ')
->setMarkerOn('[●] ');
})
->addCheckboxItem('PHP', $itemCallable)
->addCheckboxItem('Javascript', $itemCallable)
->addCheckboxItem('Ruby', $itemCallable)
Expand Down
7 changes: 5 additions & 2 deletions examples/radio-item.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

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

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

Expand All @@ -22,8 +23,10 @@
})
->addSubMenu('Interpreted', function (CliMenuBuilder $b) use ($itemCallable) {
$b->setTitle('Interpreted Languages')
->setUnradioMarker('[ ] ')
->setRadioMarker('[✔] ')
->modifyRadioStyle(function (RadioStyle $style) {
$style->setMarkerOff('[ ] ')
->setMarkerOn('[✔] ');
})
->addRadioItem('PHP', $itemCallable)
->addRadioItem('Javascript', $itemCallable)
->addRadioItem('Ruby', $itemCallable)
Expand Down
0 examples/split-checkable-item.php → examples/split-checkbox-item.php
View file
Open in desktop
File renamed without changes.
98 changes: 67 additions & 31 deletions src/Builder/CliMenuBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
use PhpSchool\CliMenu\MenuItem\SplitItem;
use PhpSchool\CliMenu\MenuItem\StaticItem;
use PhpSchool\CliMenu\MenuStyle;
use PhpSchool\CliMenu\Style\CheckboxStyle;
use PhpSchool\CliMenu\Style\RadioStyle;
use PhpSchool\CliMenu\Terminal\TerminalFactory;
use PhpSchool\Terminal\Terminal;

Expand Down Expand Up @@ -138,7 +140,10 @@ public function addCheckboxItem(
bool $showItemExtra = false,
bool $disabled = false
) : self {
$this->addMenuItem(new CheckboxItem($text, $itemCallable, $showItemExtra, $disabled));
$item = (new CheckboxItem($text, $itemCallable, $showItemExtra, $disabled))
->setStyle($this->menu->getCheckboxStyle());

$this->addMenuItem($item);

return $this;
}
Expand All @@ -149,7 +154,10 @@ public function addRadioItem(
bool $showItemExtra = false,
bool $disabled = false
) : self {
$this->addMenuItem(new RadioItem($text, $itemCallable, $showItemExtra, $disabled));
$item = (new RadioItem($text, $itemCallable, $showItemExtra, $disabled))
->setStyle($this->menu->getRadioStyle());

$this->addMenuItem($item);

return $this;
}
Expand Down Expand Up @@ -194,6 +202,14 @@ public function addSubMenu(string $text, \Closure $callback) : self
$menu->setStyle($this->menu->getStyle());
}

if (!$menu->getCheckboxStyle()->hasChangedFromDefaults()) {
$menu->setCheckboxStyle(clone $this->menu->getCheckboxStyle());
}

if (!$menu->getRadioStyle()->hasChangedFromDefaults()) {
$menu->setRadioStyle(clone $this->menu->getRadioStyle());
}

$this->menu->addItem($item = new MenuMenuItem(
$text,
$menu,
Expand All @@ -216,6 +232,14 @@ public function addSubMenuFromBuilder(string $text, CliMenuBuilder $builder) : s
$menu->setStyle($this->menu->getStyle());
}

if (!$menu->getCheckboxStyle()->hasChangedFromDefaults()) {
$menu->setCheckboxStyle(clone $this->menu->getCheckboxStyle());
}

if (!$menu->getRadioStyle()->hasChangedFromDefaults()) {
$menu->setRadioStyle(clone $this->menu->getRadioStyle());
}
Comment on lines +235 to +241
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could extract this to a private method, since we do it below as well. But we can do that later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above even.


$this->menu->addItem($item = new MenuMenuItem(
$text,
$menu,
Expand Down Expand Up @@ -296,13 +320,15 @@ private function processIndividualShortcut(MenuItemInterface $item, callable $ca
public function addSplitItem(\Closure $callback) : self
{
$builder = new SplitItemBuilder($this->menu);
$builder->setCheckboxStyle(clone $this->menu->getCheckboxStyle())
->setRadioStyle(clone $this->menu->getRadioStyle());

if ($this->autoShortcuts) {
$builder->enableAutoShortcuts($this->autoShortcutsRegex);
}

$callback($builder);

$this->menu->addItem($splitItem = $builder->build());

$this->processSplitItemShortcuts($splitItem);
Expand Down Expand Up @@ -417,34 +443,6 @@ public function setSelectedMarker(string $marker) : self
return $this;
}

public function setUncheckedMarker(string $marker) : self
{
$this->style->setUncheckedMarker($marker);

return $this;
}

public function setCheckedMarker(string $marker) : self
{
$this->style->setCheckedMarker($marker);

return $this;
}

public function setUnradioMarker(string $marker) : self
{
$this->style->setUnradioMarker($marker);

return $this;
}

public function setRadioMarker(string $marker) : self
{
$this->style->setRadioMarker($marker);

return $this;
}

public function setItemExtra(string $extra) : self
{
$this->style->setItemExtra($extra);
Expand Down Expand Up @@ -558,4 +556,42 @@ public function build() : CliMenu

return $this->menu;
}

public function getCheckboxStyle() : CheckboxStyle
{
return $this->menu->getCheckboxStyle();
}

public function setCheckboxStyle(CheckboxStyle $style) : self
{
$this->menu->setCheckboxStyle($style);

return $this;
}

public function modifyCheckboxStyle(callable $itemCallable) : self
{
$itemCallable($this->menu->getCheckboxStyle());

return $this;
}

public function getRadioStyle() : RadioStyle
{
return $this->menu->getRadioStyle();
}

public function setRadioStyle(RadioStyle $style) : self
{
$this->menu->setRadioStyle($style);

return $this;
}

public function modifyRadioStyle(callable $itemCallable) : self
{
$itemCallable($this->menu->getRadioStyle());

return $this;
}
}
58 changes: 56 additions & 2 deletions src/Builder/SplitItemBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
use PhpSchool\CliMenu\MenuItem\SelectableItem;
use PhpSchool\CliMenu\MenuItem\SplitItem;
use PhpSchool\CliMenu\MenuItem\StaticItem;
use PhpSchool\CliMenu\Style\CheckboxStyle;
use PhpSchool\CliMenu\Style\RadioStyle;

/**
* @author Aydin Hassan <aydin@hotmail.co.uk>
Expand Down Expand Up @@ -65,7 +67,10 @@ public function addCheckboxItem(
bool $showItemExtra = false,
bool $disabled = false
) : self {
$this->splitItem->addItem(new CheckboxItem($text, $itemCallable, $showItemExtra, $disabled));
$item = (new CheckboxItem($text, $itemCallable, $showItemExtra, $disabled))
->setStyle($this->menu->getCheckboxStyle());

$this->splitItem->addItem($item);

return $this;
}
Expand All @@ -76,7 +81,10 @@ public function addRadioItem(
bool $showItemExtra = false,
bool $disabled = false
) : self {
$this->splitItem->addItem(new RadioItem($text, $itemCallable, $showItemExtra, $disabled));
$item = (new RadioItem($text, $itemCallable, $showItemExtra, $disabled))
->setStyle($this->menu->getRadioStyle());

$this->splitItem->addItem($item);

return $this;
}
Expand Down Expand Up @@ -108,6 +116,14 @@ public function addSubMenu(string $text, \Closure $callback) : self
$menu = $builder->build();
$menu->setParent($this->menu);

if (!$menu->getCheckboxStyle()->hasChangedFromDefaults()) {
$menu->setCheckboxStyle(clone $this->menu->getCheckboxStyle());
}

if (!$menu->getRadioStyle()->hasChangedFromDefaults()) {
$menu->setRadioStyle(clone $this->menu->getRadioStyle());
}

$this->splitItem->addItem(new MenuMenuItem(
$text,
$menu,
Expand Down Expand Up @@ -139,4 +155,42 @@ public function build() : SplitItem
{
return $this->splitItem;
}

public function getCheckboxStyle() : CheckboxStyle
{
return $this->menu->getCheckboxStyle();
}

public function setCheckboxStyle(CheckboxStyle $style) : self
{
$this->menu->setCheckboxStyle($style);

return $this;
}

public function modifyCheckboxStyle(callable $itemCallable) : self
Copy link
Member Author

@AydinHassan AydinHassan Dec 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think we should drop this. It's took me a while but I think I get what you are doing. If you change the styles in a splititem builder, they persist back to the parent menu. I think it makes the code very confusing. I would rather we just not allow to modify the styles in the SplitItem. Or rather if you modify the styles, it only applies to the split item and not the parent menu.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style is always limited only to the current scope and any children:

<?php

use PhpSchool\CliMenu\Builder\SplitItemBuilder;
use PhpSchool\CliMenu\CliMenu;
use PhpSchool\CliMenu\Builder\CliMenuBuilder;
use PhpSchool\CliMenu\Style\CheckboxStyle;

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

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

$menu = (new CliMenuBuilder)
    ->setTitle('Select a Language')
    ->addCheckboxItem('Rust', $itemCallable)
    ->addCheckboxItem('C++', $itemCallable)
    ->addCheckboxItem('Go', $itemCallable)
    ->addCheckboxItem('Java', $itemCallable)
    ->addCheckboxItem('C', $itemCallable)
    ->addSplitItem(function (SplitItemBuilder $b) use ($itemCallable) {
        $b->setGutter(5)
            ->modifyCheckboxStyle(function (CheckboxStyle $style) {
                $style->setMarkerOff('ooo')
                    ->setMarkerOn('xxx');
            })
            ->addCheckboxItem('Rust', $itemCallable)
            ->addCheckboxItem('C++', $itemCallable)
            ->addCheckboxItem('Go', $itemCallable)
            ->addCheckboxItem('Java', $itemCallable)
            ->addCheckboxItem('C', $itemCallable)
        ;
    })
    ->build();

$menu->open();

The parent should never be affected from the modify*Style() methods:

Peek 2019-12-19 16-26

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually you're right, there's a bug where changing in a child updates the parent:

<?php

use PhpSchool\CliMenu\Builder\SplitItemBuilder;
use PhpSchool\CliMenu\CliMenu;
use PhpSchool\CliMenu\Builder\CliMenuBuilder;
use PhpSchool\CliMenu\Style\CheckboxStyle;

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

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

$menu = (new CliMenuBuilder)
    ->setTitle('Select a Language')
    ->addCheckboxItem('1-A', $itemCallable)
    ->addCheckboxItem('1-B', $itemCallable)
    ->addSplitItem(function (SplitItemBuilder $b) use ($itemCallable) {
        $b->setGutter(5)
            ->addCheckboxItem('2-A', $itemCallable)
            ->modifyCheckboxStyle(function (CheckboxStyle $style) {
                $style->setMarkerOff('o ')
                    ->setMarkerOn('x ');
            })
            ->addCheckboxItem('2-B', $itemCallable)
            ->addSubMenu('Options', function (CliMenuBuilder $b) use ($itemCallable) {
                $b->setTitle('CLI Menu > Options')
                    ->addCheckboxItem('Rust', $itemCallable)
                    ->addCheckboxItem('C++', $itemCallable)
                    ->addCheckboxItem('Go', $itemCallable)
                    ->addCheckboxItem('Java', $itemCallable)
                    ->addCheckboxItem('C', $itemCallable)
                    ->addLineBreak('-');
            })
            ->addCheckboxItem('2-C', $itemCallable)
            ->addCheckboxItem('2-D', $itemCallable)
            ->addCheckboxItem('2-E', $itemCallable)
        ;
    })
    ->addCheckboxItem('1-C', $itemCallable)
    ->addCheckboxItem('1-D', $itemCallable)
    ->addCheckboxItem('1-E', $itemCallable)
    ->build();

$menu->open();

I'll fix and add a test.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phew 😅 Thought I was going crazy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I see, because SplitMenuBuilder::$menu is the exact same instance as CliMenuBuilder::$menu.

This is why SplitMenuBuilder had its own style instances (removed in #208).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I think the problem is that I had a fundamental misunderstanding on the SplitItem.

I was thinking this was a separate menu ala submenu, but it's simply a horizontal partitioner of the current menu.

You're completely right, SplitItemBuilder shouldn't have any of the radio/checkbox style methods, and should not have its own style instance(s).

{
$itemCallable($this->menu->getCheckboxStyle());

return $this;
}

public function getRadioStyle() : RadioStyle
{
return $this->menu->getRadioStyle();
}

public function setRadioStyle(RadioStyle $style) : self
{
$this->menu->setRadioStyle($style);

return $this;
}

public function modifyRadioStyle(callable $itemCallable) : self
{
$itemCallable($this->menu->getRadioStyle());

return $this;
}
}
Loading