Columns < 100 results in broken submenu layouts/colors

[jtreminio]

To show current size use tput cols

Here it is with 100:

I am able to reproduce across several different terminal emulators.

[jtreminio]

I've gone back several tags and seems this problem has existed for some time... but I did not composer update on them, always using the latest PhpSchool\Terminal package.

edit: I've gone to 3.0.0, ran composer update to fetch proper packages, and can reproduce.

[jtreminio]

The cause seems to be the MenuStyle::$defaultStyleValues['width'] value.

If I set it to 80, and my columns are 99, the menu displays as expected.

If I set it to 80 and set cols size to 80, menu displays as expected.

Resetting to 100 reproduces bug.

[AydinHassan]

This is correct (expected behaviour). The default size of the menu is 100. So unless you change the width to a smaller value it will overlap.

You can grab the Terminal object and set the width based on the terminal size.

https://github.com/php-school/cli-menu#width

[AydinHassan]

I guess we could treat the default the same as we do when manually setting a value larger than the terminal width.

[AydinHassan]

Actually. There is a bug, it's the default margin. Looks like it might not be taken into account when calculating the width. We are already shrinking the menu when the default width value is too large for the terminal.

[AydinHassan]

Replacing \PhpSchool\CliMenu\MenuStyle::calculateContentWidth with the below code fixes it. Breaks lots of tests though. I don't have time to fix it right now.

/**
 * Calculate the contents width
 */
protected function calculateContentWidth() : void
{
    $this->contentWidth = $this->width
        - ($this->paddingLeftRight * 2)
        - ($this->borderRightWidth + $this->borderLeftWidth)
        - $this->margin;

    if ($this->contentWidth < 0) {
        $this->contentWidth = 0;
    }
}

[jtreminio]

While that fixes the broken column sizing for submenus, it does not fix passing the parent style to child because hasChangedFromDefaults() returns true.

Running the initial test in a terminal with 95 cols hasChangedFromDefaults returns true because the parent's width is 95 but the default static width is still 100.

[jtreminio]

The following solves the issue:

diff --git a/src/MenuStyle.php b/src/MenuStyle.php
index 0bd0974..05f9b7d 100644
--- a/src/MenuStyle.php
+++ b/src/MenuStyle.php
@@ -199,6 +199,8 @@ class MenuStyle
         'conceal'    => ['set' => 8, 'unset' => 28]
     ];
 
+    private static $widthOverridden = false;
+
     /**
      * Initialise style
      */
@@ -211,6 +213,12 @@ class MenuStyle
 
         $this->generateColoursSetCode();
 
+        if (!self::$widthOverridden && $this->terminal->getWidth() < self::$defaultStyleValues['width']) {
+            self::$defaultStyleValues['width'] = $this->terminal->getWidth();
+        }
+
+        self::$widthOverridden = true;
+
         $this->setWidth(self::$defaultStyleValues['width']);
         $this->setPaddingTopBottom(self::$defaultStyleValues['paddingTopBottom']);
         $this->setPaddingLeftRight(self::$defaultStyleValues['paddingLeftRight']);
@@ -317,7 +325,8 @@ class MenuStyle
     {
         $this->contentWidth = $this->width
             - ($this->paddingLeftRight * 2)
-            - ($this->borderRightWidth + $this->borderLeftWidth);
+            - ($this->borderRightWidth + $this->borderLeftWidth)
+            - ($this->margin * 2);
 
         if ($this->contentWidth < 0) {
             $this->contentWidth = 0;

It includes your patch as well as updating the default width the first time the class is instantiated. Since it's a static the change reflects.

edit: updated patch.

[AydinHassan]

There is another bug as well with margin auto. Need to look in to that more but I think calculateContentWidth() into setMarginAuto() might help.