cli-menu icon indicating copy to clipboard operation
cli-menu copied to clipboard

Add option to change the newlines before and after the menu is drawn

Open Zhigalin opened this issue 2 years ago • 8 comments

This feature is useful because it allows you to either reduce or set to zero the vertical margins in cases of a small terminal window (my case for which I made this feature) or a very large menu and you have to use every inch to fit it into the terminal, or you can increment it to move or center the menu vertically.

Zhigalin avatar Dec 15 '23 14:12 Zhigalin

What is the difference between this and the top/bottom padding?

AydinHassan avatar Dec 15 '23 15:12 AydinHassan

What is the difference between this and the top/bottom padding?

If you want to decrement the number of lines the difference is that these 4 lines are not part of the padding number and as such cannot be removed and the space occupied by these lines cannot be used even if we set the padding to 0.

If you want to increment it the difference is in the color. In the screenshot to the left we use the padding and to the right we use the new setting to align the menu vertically Screenshot_term

Zhigalin avatar Dec 18 '23 09:12 Zhigalin

@AydinHassan could you take another look at this PR? Thank you

Zhigalin avatar Jun 11 '24 14:06 Zhigalin

@Zhigalin sorry i took so long to look. I think it would make a bit more sense to just use the margin value here?

AydinHassan avatar Oct 26 '24 11:10 AydinHassan

@Zhigalin sorry i took so long to look. I think it would make a bit more sense to just use the margin value here?

@AydinHassan margins are horisontal, the proposed fearure is abount vertical spacing. Maybe I should rename it to verticalMargins or something like that for clarity?

Zhigalin avatar Oct 26 '24 11:10 Zhigalin

yes but we have the concept of top/bottom padding and left/right padding so we could do the same with margin, and then use the top/bottom margin instead of the top/bottom frame count. Then we could also take in to consideration the margin auto option and center the menu vertically when it's set.

So yeah first lets add setMarginTopBottom and use that instead of the top/bottom frame lines stuff.

AydinHassan avatar Oct 26 '24 11:10 AydinHassan

I was thinking something like:

[$marginTop, $marginBottom] = $this->style->getMarginTopBottom($frame->count());
      
$frame->prependNewLine($marginTop);
$frame->newLine($marginBottom);

in CliMenu::draw

and

/**
     * @return array{0: int, 1: int}
     */
    public function getMarginTopBottom(int $contentHeight): array
    {
        if ($this->isMarginAuto()) {
            $availableHeight = $this->terminal->getHeight();

            if ($contentHeight < $availableHeight) {
                return [
                    (int) floor(($availableHeight - $contentHeight) / 2),
                    (int) ceil(($availableHeight - $contentHeight) / 2)
                ];
            }

            return [0, 0];
        }

        return [$this->marginTop, $this->marginBottom];
    }

in MenuStyle of course with all the setters and props etc

AydinHassan avatar Oct 26 '24 12:10 AydinHassan

Ok, will do

Zhigalin avatar Oct 26 '24 12:10 Zhigalin