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-specific styling #200

Closed jtreminio closed 4 years ago

jtreminio commented 4 years ago

Currently all items are on the same vertical level:

    Example Title                                                                              
    ================================================================================================  

    Static Item #1                                                                                    
    ○ Item 1                                                                                          
    ○ Item 2                                                                                          
    ○ Item 3                                                                                          
    ○ Item 4                                                                                          
    Static Item #2                                                                                    
    ○ Item 1                                                                                          
    ○ Item 2                                                                                          
    ○ Item 3                                                                                          
    ○ Item 4                                                                                          
    Static Item #3                                                                                    
    [○] Item 1                      [○] Item 2                      [○] Item 3                        

    ------------------------------------------------------------------------------------------------  
    ● Exit                                                                                            

Using line breaks makes it a little easier to separate elements:

    Example Title                                                                              
    ================================================================================================  

    Static Item #1                                                                                    
    ○ Item 1                                                                                          
    ○ Item 2                                                                                          
    ○ Item 3                                                                                          
    ○ Item 4                                                                                          

    Static Item #2                                                                                    
    ○ Item 1                                                                                          
    ○ Item 2                                                                                          
    ○ Item 3                                                                                          
    ○ Item 4                                                                                          

    Static Item #3                                                                                    
    [○] Item 1                      [○] Item 2                      [○] Item 3                        

    ------------------------------------------------------------------------------------------------  
    ● Exit                                                                                            

This works fine but could be better. What do you think about an "itemIndent" vlaue in the MenuStyle that is applied only to SelectItem, RadioItem and CheckableItem?

    Example Title                                                                                     
    ================================================================================================  

    Static Item #1                                                                                    
       ○ Item 1                                                                                       
       ○ Item 2                                                                                       
       ○ Item 3                                                                                       
       ○ Item 4                                                                                       
    Static Item #2                                                                                    
       ○ Item 1                                                                                       
       ○ Item 2                                                                                       
       ○ Item 3                                                                                       
       ● Item 4                                                                                       
    Static Item #3                                                                                    
       [○] Item 1                   [○] Item 2                      [○] Item 3                        

    ------------------------------------------------------------------------------------------------  
    ○ Exit                                                                                            

The indentation helps visually separate what is an actionable item and what is mostly text description:

Peek 2019-12-17 21-39

Let me know if this is something that would be interesting. I've got it implemented but needs some testing for the StringUtil::wordwrap() function that calculates terminal width and breaks up lines.

jtreminio commented 4 years ago

To get this to work, SplitItemBuilder would need to have a MenuStyle of its own, so its constructor would need updated.

Could also have separate itemIndent values for CheckableItem and RadioItem to match their marker styles:

    Example Title                                                                                     
    ================================================================================================  

    Static Item #1                                                                                    
       ○ Item 1                                                                                       
       ○ Item 2                                                                                       
       ● Item 3                                                                                       
       ○ Item 4                                                                                       
    Static Item #2                                                                                    
       ○ Item 1                                                                                       
       ○ Item 2                                                                                       
       ○ Item 3                                                                                       
       ○ Item 4                                                                                       
    Static Item #3                                                                                    
      [●] Item 1                                                                                      
      [○] Item 2                                                                                      
      [○] Item 3                                                                                      

    ------------------------------------------------------------------------------------------------  
    ○ Exit                                                                                            

If a developer wants they can align everything using the SelectableItem's marker as well:

    Example Title                                                                                     
    ================================================================================================  

    Static Item #1                                                                                    
       ○  Item 1                                                                                      
       ○  Item 2                                                                                      
       ○  Item 3                                                                                      
       ○  Item 4                                                                                      
    Static Item #2                                                                                    
       ○  Item 1                                                                                      
       ○  Item 2                                                                                      
       ○  Item 3                                                                                      
       ○  Item 4                                                                                      
    Static Item #3                                                                                    
      [○] Item 1                                                                                      
      [●] Item 2                                                                                      
      [○] Item 3                                                                                      
    Static Item #4                                                                                    
      [ ] Item 1                                                                                      
      [✔] Item 2                                                                                      
      [ ] Item 3                                                                                      

    ------------------------------------------------------------------------------------------------  
    ●  Exit                                                                                           
AydinHassan commented 4 years ago

It sounds interesting but I think I would rather implement it in a different way. At this point we might want to introduce per item styling. That way we wouldn't have to special case items and introduce new styling concepts. A item could have its own styling and in this case it would be left padding.

This would leave us room to add other features such as items with different colours and text styling in the future. What do you think?

jtreminio commented 4 years ago

Do you have any plans drawn up on how you'd like to implement item-level styling?

AydinHassan commented 4 years ago

Hey, not particularly, my only thoughts were that we might want items to expose a getStyle method and have a new ItemStyle class. Items should be able to be constructed without a style object and in that case styles would propagate from the main menu style (the ones that would make sense anyway for example: fg and bg colours).

We could have the builders have an internal ItemStyle object with various setters which delegate to the item style. Each Item created via the builders would use this ItemStyle.

If you wanted to build a custom item style that should be used for all items, you should be able to add items with your already constructed style and set that on the builder or individual items.

And if you want to just modify some properties without building a theme and constructing your own item style, you could just do something like:

$item = (new MyItem(/** */))
    ->getStyle()
    ->setFg('green');

I realise this is probably quite a lot of work, and you're probably not too interested in that. But I definitely would accept something that is very bare bones and allows us to build on. You definitely don't need to build all the features. What are your thoughts?

jtreminio commented 4 years ago

Can you provide feedback on PR #203

I know it's a major API change style-wise. The CliMenuBuilder::setUnselectedMarker() and similar style-related methods are a little weird to be included in the class, but I kept them around and only apply changes to items at current menu-level and of SelectableInterface (SelectableItem, MenuMenuItem) as those were the only ones before CheckableItem and RadioItem.

Would make sense to deprecate and remove them, though, and to apply a callback to the MenuStyle from within CliMenuBuilder like I did for the other new styles:

<?php

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

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

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

$menu = (new CliMenuBuilder)
    ->setTitle('Basic CLI Menu')
    ->selectableStyle(function (SelectableStyle $style) {
        $style->setMarkerOff('- ');
    })
    ->addItem('First Item', $itemCallable)
    ->addItem('Second Item', $itemCallable)
    ->addItem('Third Item', $itemCallable)
    ->addLineBreak('-')
    ->build();

$menu->open();
jtreminio commented 4 years ago

PR #203 has been replaced with PR #204 - it does not include SelectableItem styling, only CheckboxItem and RadioItem.

jtreminio commented 4 years ago

@AydinHassan Once #205 is merged, what do you suggest doing with SelectableItem styling?

Right now you change its marker via CliMenuBuilder::setSelectedMarker() which is different from how RadioItem and CheckboxItem now do it.

My previous PR that was closed added a style class for the SelectableItem to match the other two. It also kept BC with current API (CliMenuBuilder::setSelectedMarker()).

It's a bit weird for SelectableItem to have different style entrypoints than RadioItem and CheckboxItem and I'm unsure how much BC you're open to breaking for your new 4.0 version.

AydinHassan commented 4 years ago

I'd like to introduce SelectableStyle too - I just want to keep the diffs minimal so we can concentrate on having good design.

So yeah we can either drop CliMenuBuilder::setSelectedMarker() altogether or deprecate it and have it proxy to SelectableStyle and remove in a further major, but we can discuss that later.

AydinHassan commented 4 years ago

Btw I don't mind breaking a few things for 4.0.

AydinHassan commented 4 years ago

Whats the status here @jtreminio can this be closed now?