Ascii art refactor #105
Ascii art refactor #105
Conversation
src/MenuItem/AsciiArtItem.php
Outdated
| @@ -64,19 +66,16 @@ public function getRows(MenuStyle $style, bool $selected = false) : array | |||
|
|
|||
| switch ($this->position) { | |||
| case self::POSITION_LEFT: | |||
| return $row; | |||
There was a problem hiding this comment.
return is done below the switch anyway.
src/MenuItem/AsciiArtItem.php
Outdated
| $row = sprintf('%s%s%s', str_repeat(' ', $left), $row, str_repeat(' ', $right)); | ||
| $padding -= ($this->artLength - $length); | ||
| $left = ceil($padding / 2); | ||
| $row = sprintf('%s%s', str_repeat(' ', $left), $row); |
There was a problem hiding this comment.
Not really sure why we added the white space on the end
test/MenuItem/AsciiArtItemTest.php
Outdated
| ); | ||
| } | ||
|
|
||
| public function testWithRealAsciiArtCenteredWithWhiteSpaceAtTheEndOfEachLine() : void |
There was a problem hiding this comment.
This is the test for your end whitespace fix
There was a problem hiding this comment.
Much sexier than the one I made eh :)
Codecov Report
@@ Coverage Diff @@
## master #105 +/- ##
============================================
- Coverage 96.68% 96.66% -0.02%
Complexity 293 293
============================================
Files 22 22
Lines 904 900 -4
============================================
- Hits 874 870 -4
Misses 30 30
Continue to review full report at Codecov.
|
src/MenuItem/AsciiArtItem.php
Outdated
| break; | ||
| case self::POSITION_RIGHT: | ||
| $row = rtrim($row); | ||
| $padding = $padding - ($this->artLength - mb_strlen($row)); | ||
| $padding -= ($this->artLength - $length); |
There was a problem hiding this comment.
Those can be removed. The padding is simply equal to
$padding = $style->getContentWidth() - $this->artLength;and can be moved out of the array_map (line 65).
Moreover the $length doesn't need to be calculated (you made everything so it's based on the longest row) so you can remove line 63 too (can't comment on it).
src/MenuItem/AsciiArtItem.php
Outdated
| $left = ceil($padding/2); | ||
| $right = $padding - $left; | ||
| $row = sprintf('%s%s%s', str_repeat(' ', $left), $row, str_repeat(' ', $right)); | ||
| $padding -= ($this->artLength - $length); |
|
That's a good idea ;) |
|
@Lynesth requested changes made! |
trimandmb_strlencalls@Lynesth could you take a look at this please