From 4f30158722b83d541fadcd262431e801be466fbf Mon Sep 17 00:00:00 2001
From: Vlad <vlad@mailpoet.com>
Date: Thu, 31 Mar 2016 15:23:30 -0400
Subject: [PATCH] - Updates button height calculation - Removes background
 color when it's set to "transparent" - Removes second line break from
 paragraphs - Introduces other changes based on Becks's testing - Updates unit
 tests

---
 lib/Newsletter/Renderer/Blocks/Button.php    | 12 ++----------
 lib/Newsletter/Renderer/Blocks/Footer.php    |  7 ++++++-
 lib/Newsletter/Renderer/Blocks/Header.php    |  7 ++++++-
 lib/Newsletter/Renderer/Blocks/Renderer.php  |  5 ++++-
 lib/Newsletter/Renderer/Blocks/Text.php      |  2 +-
 lib/Newsletter/Renderer/Columns/Renderer.php | 16 +++++++++++++---
 lib/Newsletter/Renderer/StylesHelper.php     | 20 +++++++++++++++++---
 tests/unit/Newsletter/RendererTest.php       | 13 ++-----------
 8 files changed, 51 insertions(+), 31 deletions(-)

diff --git a/lib/Newsletter/Renderer/Blocks/Button.php b/lib/Newsletter/Renderer/Blocks/Button.php
index 3d9a764a65..2d0bca3615 100644
--- a/lib/Newsletter/Renderer/Blocks/Button.php
+++ b/lib/Newsletter/Renderer/Blocks/Button.php
@@ -7,7 +7,6 @@ use MailPoet\Newsletter\Renderer\StylesHelper;
 class Button {
   static function render($element, $column_count) {
     $element['styles']['block']['width'] = self::calculateWidth($element, $column_count);
-    $element['styles']['block']['height'] = self::calculateHeight($element);
     $template = '
       <tr>
         <td class="mailpoet_padded_bottom mailpoet_padded_side" valign="top">
@@ -17,10 +16,10 @@ class Button {
                 <td class="mailpoet_button-container" style="padding:8px 20px;text-align:' . $element['styles']['block']['textAlign'] . ';"><!--[if mso]>
                   <v:roundrect xmlns:v="urn:schemas-microsoft-com:vml" xmlns:w="urn:schemas-microsoft-com:office:word"
                     href="' . $element['url'] . '"
-                    style="height:' . $element['styles']['block']['height'] . ';
+                    style="height:' . $element['styles']['block']['lineHeight'] . ';
                            width:' . $element['styles']['block']['width'] . ';
                            v-text-anchor:middle;"
-                    arcsize="' . round($element['styles']['block']['borderRadius'] / $element['styles']['block']['height'] * 100) . '%"
+                    arcsize="' . round($element['styles']['block']['borderRadius'] / $element['styles']['block']['lineHeight'] * 100) . '%"
                     strokeweight="1px"
                     strokecolor="' . $element['styles']['block']['borderColor'] . '"
                     fillcolor="' . $element['styles']['block']['backgroundColor'] . '">
@@ -52,11 +51,4 @@ class Button {
     $button_width = $button_width - (2 * $border_width) . 'px';
     return $button_width;
   }
-
-  static function calculateHeight($element) {
-    $border_width = (int) $element['styles']['block']['borderWidth'];
-    $button_height = (int) $element['styles']['block']['lineHeight'];
-    $button_height = $button_height - (2 * $border_width) . 'px';
-    return $button_height;
-  }
 }
\ No newline at end of file
diff --git a/lib/Newsletter/Renderer/Blocks/Footer.php b/lib/Newsletter/Renderer/Blocks/Footer.php
index 4412ffa375..fb29ee2546 100644
--- a/lib/Newsletter/Renderer/Blocks/Footer.php
+++ b/lib/Newsletter/Renderer/Blocks/Footer.php
@@ -17,9 +17,14 @@ class Footer {
         }
       }
     }
+    $background_color = $element['styles']['block']['backgroundColor'];
+    $background_color = ($background_color !== 'transparent') ?
+      'bgcolor="' . $background_color . '"' :
+      false;
+    if(!$background_color) unset($element['styles']['block']['backgroundColor']);
     $template = '
       <tr>
-        <td class="mailpoet_header_footer_padded mailpoet_footer" bgcolor="' . $element['styles']['block']['backgroundColor'] . '"
+        <td class="mailpoet_header_footer_padded mailpoet_footer" ' . $background_color . '
         style="line-height: ' . StylesHelper::$line_height . ';' . StylesHelper::getBlockStyles($element) . StylesHelper::getStyles($element['styles'], 'text') . '">
           ' . $DOM->html() . '
         </td>
diff --git a/lib/Newsletter/Renderer/Blocks/Header.php b/lib/Newsletter/Renderer/Blocks/Header.php
index 95fee5fed8..b1beb887ee 100644
--- a/lib/Newsletter/Renderer/Blocks/Header.php
+++ b/lib/Newsletter/Renderer/Blocks/Header.php
@@ -17,9 +17,14 @@ class Header {
         }
       }
     }
+    $background_color = $element['styles']['block']['backgroundColor'];
+    $background_color = ($background_color !== 'transparent') ?
+      'bgcolor="' . $background_color . '"' :
+      false;
+    if(!$background_color) unset($element['styles']['block']['backgroundColor']);
     $template = '
       <tr>
-        <td class="mailpoet_header_footer_padded mailpoet_header" bgcolor="' . $element['styles']['block']['backgroundColor'] . '"
+        <td class="mailpoet_header_footer_padded mailpoet_header" ' . $background_color . '
         style="' . StylesHelper::getBlockStyles($element) . StylesHelper::getStyles($element['styles'], 'text') . '">
         ' . $DOM->html() . '
         </td>
diff --git a/lib/Newsletter/Renderer/Blocks/Renderer.php b/lib/Newsletter/Renderer/Blocks/Renderer.php
index cd418c70df..6c95b3de95 100644
--- a/lib/Newsletter/Renderer/Blocks/Renderer.php
+++ b/lib/Newsletter/Renderer/Blocks/Renderer.php
@@ -1,10 +1,12 @@
 <?php
 namespace MailPoet\Newsletter\Renderer\Blocks;
 
+use MailPoet\Newsletter\Renderer\StylesHelper;
+
 class Renderer {
   function render($data, $column_count) {
     $block_content = '';
-    array_map(function ($block) use (&$block_content, &$columns, $column_count) {
+    array_map(function($block) use (&$block_content, &$columns, $column_count) {
       $block_content .= $this->createElementFromBlockType($block, $column_count);
       if(isset($block['blocks'])) {
         $block_content = $this->render($block, $column_count);
@@ -18,6 +20,7 @@ class Renderer {
   }
 
   function createElementFromBlockType($block, $column_count) {
+    $block = StylesHelper::setTextAlign($block);
     $block_class = __NAMESPACE__ . '\\' . ucfirst($block['type']);
     return (class_exists($block_class)) ? $block_class::render($block, $column_count) : '';
   }
diff --git a/lib/Newsletter/Renderer/Blocks/Text.php b/lib/Newsletter/Renderer/Blocks/Text.php
index 3baf1d9edd..9907bcd46d 100644
--- a/lib/Newsletter/Renderer/Blocks/Text.php
+++ b/lib/Newsletter/Renderer/Blocks/Text.php
@@ -81,7 +81,7 @@ class Text {
         <tr>
           <td class="mailpoet_paragraph" style="line-height:' . StylesHelper::$line_height . ';word-break:break-word;word-wrap:break-word;' . $style . '">
             ' . $contents . '
-            <br /><br />
+            <br />
           </td>
          </tr>'
       );
diff --git a/lib/Newsletter/Renderer/Columns/Renderer.php b/lib/Newsletter/Renderer/Columns/Renderer.php
index afeb37b6e7..6a396a69fa 100644
--- a/lib/Newsletter/Renderer/Columns/Renderer.php
+++ b/lib/Newsletter/Renderer/Columns/Renderer.php
@@ -22,6 +22,7 @@ class Renderer {
   }
 
   function getOneColumnTemplate($styles, $class) {
+    $background_color = $this->getBackgroundColor($styles);
     $template['content_start'] = '
       <tr>
         <td class="mailpoet_content" align="center" style="border-collapse:collapse">
@@ -29,7 +30,7 @@ class Renderer {
             <tbody>
               <tr>
                 <td style="padding-left:0;padding-right:0">
-                  <table width="100%" border="0" cellpadding="0" cellspacing="0" class="mailpoet_' . $class . '" style="border-spacing:0;mso-table-lspace:0;mso-table-rspace:0;table-layout:fixed;margin-left:auto;margin-right:auto;padding-left:0;padding-right:0;background-color:' . $styles['backgroundColor'] . '!important;" bgcolor="' . $styles['backgroundColor'] . '">
+                  <table width="100%" border="0" cellpadding="0" cellspacing="0" class="mailpoet_' . $class . '" style="border-spacing:0;mso-table-lspace:0;mso-table-rspace:0;table-layout:fixed;margin-left:auto;margin-right:auto;padding-left:0;padding-right:0;' . $background_color . '">
                     <tbody>';
     $template['content_end'] = '
                     </tbody>
@@ -44,9 +45,10 @@ class Renderer {
   }
 
   function getMultipleColumnsTemplate($styles, $width, $alignment, $class) {
+    $background_color = $this->getBackgroundColor($styles);
     $template['container_start'] = '
       <tr>
-        <td class="mailpoet_content-' . $class . '" align="left" style="border-collapse:collapse;background-color:' . $styles['backgroundColor'] . '!important;" bgcolor="' . $styles['backgroundColor'] . '">
+        <td class="mailpoet_content-' . $class . '" align="left" style="border-collapse:collapse;' . $background_color . '">
           <table width="100%" border="0" cellpadding="0" cellspacing="0" style="border-spacing:0;mso-table-lspace:0;mso-table-rspace:0">
             <tbody>
               <tr>
@@ -78,6 +80,14 @@ class Renderer {
   }
 
   function removePaddingFromLastElement($element) {
-    return preg_replace('/mailpoet_padded_bottom["|\s](?!.*mailpoet_padded_bottom["|\s])/ism', '', $element);
+    return preg_replace('/mailpoet_padded_bottom(?!.*mailpoet_padded_bottom)/ism', '', $element);
+  }
+
+  function getBackgroundColor($styles) {
+    if(!isset($styles['backgroundColor'])) return false;
+    $background_color = $styles['backgroundColor'];
+    return ($background_color !== 'transparent') ?
+      sprintf('background-color:%s!important;" bgcolor="%s', $background_color, $background_color) :
+      false;
   }
 }
\ No newline at end of file
diff --git a/lib/Newsletter/Renderer/StylesHelper.php b/lib/Newsletter/Renderer/StylesHelper.php
index ddfd422a60..6021b296ac 100644
--- a/lib/Newsletter/Renderer/StylesHelper.php
+++ b/lib/Newsletter/Renderer/StylesHelper.php
@@ -1,8 +1,6 @@
 <?php
 namespace MailPoet\Newsletter\Renderer;
 
-use MailPoet\Newsletter\Renderer\Columns\ColumnsHelper;
-
 class StylesHelper {
   static $css_attributes = array(
     'backgroundColor' => 'background-color',
@@ -67,9 +65,25 @@ class StylesHelper {
   static function setStyle($style, $selector) {
     $css = $selector . '{' . PHP_EOL;
     foreach($style as $attribute => $individual_style) {
-      $css .= self::translateCSSAttribute($attribute) . ':' . $individual_style . ';' .  PHP_EOL;
+      $css .= self::translateCSSAttribute($attribute) . ':' . $individual_style . ';' . PHP_EOL;
     }
     $css .= '}' . PHP_EOL;
     return $css;
   }
+
+  static function setTextAlign($block) {
+    $alignments = array(
+      'center',
+      'right',
+      'justify'
+    );
+    $text_alignment = isset($block['styles']['block']['textAlign']) ?
+      strtolower($block['styles']['block']['textAlign']) :
+      false;
+    if(!$text_alignment || !in_array($text_alignment, $alignments)) {
+      return $block;
+    }
+    $block['styles']['block']['textAlign'] = 'left';
+    return $block;
+  }
 }
\ No newline at end of file
diff --git a/tests/unit/Newsletter/RendererTest.php b/tests/unit/Newsletter/RendererTest.php
index 7767db3e5f..cde0ae4c46 100644
--- a/tests/unit/Newsletter/RendererTest.php
+++ b/tests/unit/Newsletter/RendererTest.php
@@ -23,7 +23,6 @@ class NewsletterRendererTest extends MailPoetTest {
     $this->DOM_parser = new \pQuery();
   }
 
-
   function testItRendersCompleteNewsletter() {
     $template = $this->renderer->render();
     expect(isset($template['html']))->true();
@@ -254,14 +253,6 @@ class NewsletterRendererTest extends MailPoetTest {
     expect($button_width)->equals('618px'); //(width - (2 * border width)
   }
 
-  function testItCalculatesButtonHeight() {
-    $newsletter = json_decode($this->newsletter['body'], true);
-    $template = $newsletter['content']['blocks'][0]['blocks'][0]['blocks'][5];
-    $template['styles']['block']['height'] = '30px';
-    $button_width = Button::calculateHeight($template, $columnCunt = 1);
-    expect($button_width)->equals('28px'); //(height - (2 * border width)
-  }
-
   function testItRendersButton() {
     $newsletter = json_decode($this->newsletter['body'], true);
     $template = $newsletter['content']['blocks'][0]['blocks'][0]['blocks'][5];
@@ -277,12 +268,12 @@ class NewsletterRendererTest extends MailPoetTest {
     )->equals(1);
     expect(
       preg_match(
-        '/arcsize="' . round(20 / 28 * 100) . '%"/',
+        '/arcsize="' . round(20 / 30 * 100) . '%"/',
         $DOM('tr > td > div > table > tr > td', 0)->text())
     )->equals(1);
     expect(
       preg_match(
-        '/style="height:28px.*?width:98px/',
+        '/style="height:30px.*?width:98px/',
         $DOM('tr > td > div > table > tr > td', 0)->text())
     )->equals(1);
     expect(