margin out of contentWidth + add menu centering#103
margin out of contentWidth + add menu centering#103AydinHassan merged 13 commits intophp-school:masterfrom
Conversation
|
Of course, this messed up some tests. If this is accepted, I will take care of fixing those. |
Codecov Report
@@ Coverage Diff @@
## master #103 +/- ##
===========================================
+ Coverage 96.68% 96.7% +0.02%
- Complexity 293 297 +4
===========================================
Files 22 22
Lines 905 911 +6
===========================================
+ Hits 875 881 +6
Misses 30 30
Continue to review full report at Codecov.
|
src/MenuStyle.php
Outdated
| } | ||
|
|
||
| $this->width = $width; | ||
| if ($this->margin === -1) { |
There was a problem hiding this comment.
If we want to center the menu we force recalculation of margin by setting it again each time the width is modified.
src/MenuStyle.php
Outdated
| $this->margin = $margin; | ||
|
|
||
| $this->calculateContentWidth(); | ||
| if ($this->margin === -1) { |
There was a problem hiding this comment.
I think this is meant to be $margin === -1
Margin auto fix
|
Sadly this makes "responsive" layouts really complicated as the content doesn't auto fit any more if you add margins around the border. |
|
You can just do something like: $menu = ($builder = new CliMenuBuilder)
->setTitle('Basic CLI Menu')
->addItem('First Item', $itemCallable)
->addItem('Second Item', $itemCallable)
->addItem('Third Item', $itemCallable)
->addLineBreak('-')
->setBorder(1, 'green')
->setWidth($builder->getTerminal()->getWidth() - 2 * 2)
->setMargin(2)
->build();For auto width with a margin of 2. I agree we could probably make it easier or mention in the docs. |
|
Thank you very much!
that would be great. It's not obvious what kind of box model you have 😄 |
4 participants
Ok so this makes the whole "width" thing more consistent. It now works like border-box in CSS (it includes the padding and borders but not the margin):
This prevents PHP notices and improves rendered output when values are off the charts.
The following was made with width = 120, padding = 10 and margin = 120 on a 237 column terminal.
Before PR (this also throws notices because negative values are passed to str_repeat):
After PR:
It also adds the possibility to automatically center the whole menu by setting margins to
-1. I wanted to set it to 'auto' instead but since it's already typehinted to int and all, I thought -1 was good enough. This can be changed though.Anyway let me know what you think.