Apply custom style to empty cells if needed (#307)

Fixes #295 If a row should be written with a custom style, the handling of empty cells should change. Instead of being skipped entirely, empty cells will be applied the custom style, if this style has custom background color or borders. If not, then the cell definition can still be skipped.
box · Sep 8, 2016 · 3e0afd8 · 3e0afd8
1 parent d4e57b1
commit 3e0afd8
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 15 deletions.
diff --git a/src/Spout/Writer/Style/Style.php b/src/Spout/Writer/Style/Style.php
@@ -288,7 +288,7 @@ public function shouldApplyFont()
 
     /**
      * Sets the background color
-     * @param $color ARGB color (@see Color)
+     * @param string $color ARGB color (@see Color)
      * @return Style
      */
     public function setBackgroundColor($color)

diff --git a/src/Spout/Writer/XLSX/Helper/StyleHelper.php b/src/Spout/Writer/XLSX/Helper/StyleHelper.php
@@ -71,7 +71,7 @@ protected function registerFill($style)
 
         if ($backgroundColor) {
             $isBackgroundColorRegistered = isset($this->registeredFills[$backgroundColor]);
-            
+
             // We need to track the already registered background definitions
             if ($isBackgroundColorRegistered) {
                 $registeredStyleId = $this->registeredFills[$backgroundColor];
@@ -113,13 +113,32 @@ protected function registerBorder($style)
                 $this->styleIdToBorderMappingTable[$styleId] = count($this->registeredBorders);
             }
 
-        } else { 
+        } else {
             // If no border should be applied - the mapping is the default border: 0
             $this->styleIdToBorderMappingTable[$styleId] = 0;
         }
     }
 
 
+    /**
+     * For empty cells, we can specify a style or not. If no style are specified,
+     * then the software default will be applied. But sometimes, it may be useful
+     * to override this default style, for instance if the cell should have a
+     * background color different than the default one or some borders
+     * (fonts property don't really matter here).
+     *
+     * @param int $styleId
+     * @return bool Whether the cell should define a custom style
+     */
+    public function shouldApplyStyleOnEmptyCell($styleId)
+    {
+        $hasStyleCustomFill = (isset($this->styleIdToFillMappingTable[$styleId]) && $this->styleIdToFillMappingTable[$styleId] !== 0);
+        $hasStyleCustomBorders = (isset($this->styleIdToBorderMappingTable[$styleId]) && $this->styleIdToBorderMappingTable[$styleId] !== 0);
+
+        return ($hasStyleCustomFill || $hasStyleCustomBorders);
+    }
+
+
     /**
      * Returns the content of the "styles.xml" file, given a list of styles.
      *

diff --git a/src/Spout/Writer/XLSX/Internal/Workbook.php b/src/Spout/Writer/XLSX/Internal/Workbook.php
@@ -86,7 +86,7 @@ public function addNewSheet()
         $sheet = new Sheet($newSheetIndex);
 
         $worksheetFilesFolder = $this->fileSystemHelper->getXlWorksheetsFolder();
-        $worksheet = new Worksheet($sheet, $worksheetFilesFolder, $this->sharedStringsHelper, $this->shouldUseInlineStrings);
+        $worksheet = new Worksheet($sheet, $worksheetFilesFolder, $this->sharedStringsHelper, $this->styleHelper, $this->shouldUseInlineStrings);
         $this->worksheets[] = $worksheet;
 
         return $worksheet;

diff --git a/src/Spout/Writer/XLSX/Internal/Worksheet.php b/src/Spout/Writer/XLSX/Internal/Worksheet.php
@@ -30,6 +30,9 @@ class Worksheet implements WorksheetInterface
     /** @var \Box\Spout\Writer\XLSX\Helper\SharedStringsHelper Helper to write shared strings */
     protected $sharedStringsHelper;
 
+    /** @var \Box\Spout\Writer\XLSX\Helper\StyleHelper Helper to work with styles */
+    protected $styleHelper;
+
     /** @var bool Whether inline or shared strings should be used */
     protected $shouldUseInlineStrings;
 
@@ -46,13 +49,15 @@ class Worksheet implements WorksheetInterface
      * @param \Box\Spout\Writer\Common\Sheet $externalSheet The associated "external" sheet
      * @param string $worksheetFilesFolder Temporary folder where the files to create the XLSX will be stored
      * @param \Box\Spout\Writer\XLSX\Helper\SharedStringsHelper $sharedStringsHelper Helper for shared strings
+     * @param \Box\Spout\Writer\XLSX\Helper\StyleHelper Helper to work with styles
      * @param bool $shouldUseInlineStrings Whether inline or shared strings should be used
      * @throws \Box\Spout\Common\Exception\IOException If the sheet data file cannot be opened for writing
      */
-    public function __construct($externalSheet, $worksheetFilesFolder, $sharedStringsHelper, $shouldUseInlineStrings)
+    public function __construct($externalSheet, $worksheetFilesFolder, $sharedStringsHelper, $styleHelper, $shouldUseInlineStrings)
     {
         $this->externalSheet = $externalSheet;
         $this->sharedStringsHelper = $sharedStringsHelper;
+        $this->styleHelper = $styleHelper;
         $this->shouldUseInlineStrings = $shouldUseInlineStrings;
 
         /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
@@ -177,8 +182,13 @@ private function getCellXML($rowIndex, $cellNumber, $cellValue, $styleId)
         } else if (CellHelper::isNumeric($cellValue)) {
             $cellXML .= '><v>' . $cellValue . '</v></c>';
         } else if (empty($cellValue)) {
-            // don't write empty cells (not appending to $cellXML is the right behavior!)
-            $cellXML = '';
+            if ($this->styleHelper->shouldApplyStyleOnEmptyCell($styleId)) {
+                $cellXML .= '/>';
+            } else {
+                // don't write empty cells that do no need styling
+                // NOTE: not appending to $cellXML is the right behavior!!
+                $cellXML = '';
+            }
         } else {
             throw new InvalidArgumentException('Trying to add a value with an unsupported type: ' . gettype($cellValue));
         }

diff --git a/tests/Spout/Writer/XLSX/Helper/StyleHelperTest.php b/tests/Spout/Writer/XLSX/Helper/StyleHelperTest.php
@@ -2,6 +2,9 @@
 
 namespace Box\Spout\Writer\XLSX\Helper;
 
+use Box\Spout\Writer\Style\Border;
+use Box\Spout\Writer\Style\BorderBuilder;
+use Box\Spout\Writer\Style\Color;
 use Box\Spout\Writer\Style\StyleBuilder;
 
 /**
@@ -57,6 +60,27 @@ public function testRegisterStyleShouldReuseAlreadyRegisteredStyles()
         $this->assertEquals(1, $registeredStyle2->getId());
     }
 
+    /**
+     * @return void
+     */
+    public function testShouldApplyStyleOnEmptyCell()
+    {
+        $styleWithFont = (new StyleBuilder())->setFontBold()->build();
+        $styleWithBackground = (new StyleBuilder())->setBackgroundColor(Color::BLUE)->build();
+
+        $border = (new BorderBuilder())->setBorderBottom(Color::GREEN)->build();
+        $styleWithBorder = (new StyleBuilder())->setBorder($border)->build();
+
+        $styleHelper = new StyleHelper($this->defaultStyle);
+        $styleHelper->registerStyle($styleWithFont);
+        $styleHelper->registerStyle($styleWithBackground);
+        $styleHelper->registerStyle($styleWithBorder);
+
+        $this->assertFalse($styleHelper->shouldApplyStyleOnEmptyCell($styleWithFont->getId()));
+        $this->assertTrue($styleHelper->shouldApplyStyleOnEmptyCell($styleWithBackground->getId()));
+        $this->assertTrue($styleHelper->shouldApplyStyleOnEmptyCell($styleWithBorder->getId()));
+    }
+
     /**
      * @return void
      */

diff --git a/tests/Spout/Writer/XLSX/WriterWithStyleTest.php b/tests/Spout/Writer/XLSX/WriterWithStyleTest.php
@@ -176,6 +176,53 @@ public function testAddRowWithStyleShouldApplyStyleToCells()
         $this->assertEquals('0', $cellDomElements[2]->getAttribute('s'));
     }
 
+    /**
+     * @return void
+     */
+    public function testAddRowWithStyleShouldApplyStyleToEmptyCellsIfNeeded()
+    {
+        $fileName = 'test_add_row_with_style_should_apply_style_to_empty_cells_if_needed.xlsx';
+        $dataRows = [
+            ['xlsx--11', '', 'xlsx--13'],
+            ['xlsx--21', '', 'xlsx--23'],
+            ['xlsx--31', '', 'xlsx--33'],
+            ['xlsx--41', '', 'xlsx--43'],
+        ];
+
+        $styleWithFont = (new StyleBuilder())->setFontBold()->build();
+        $styleWithBackground = (new StyleBuilder())->setBackgroundColor(Color::BLUE)->build();
+
+        $border = (new BorderBuilder())->setBorderBottom(Color::GREEN)->build();
+        $styleWithBorder = (new StyleBuilder())->setBorder($border)->build();
+
+        $this->writeToXLSXFileWithMultipleStyles($dataRows, $fileName, [null, $styleWithFont, $styleWithBackground, $styleWithBorder]);
+
+        $cellDomElements = $this->getCellElementsFromSheetXmlFile($fileName);
+
+        // The first and second rows should not have a reference to the empty cell
+        // The other rows should have the reference because style should be applied to them
+        // So that's: 2 + 2 + 3 + 3 = 10 cells
+        $this->assertEquals(10, count($cellDomElements));
+
+        // First row has 2 styled cells
+        $this->assertEquals('0', $cellDomElements[0]->getAttribute('s'));
+        $this->assertEquals('0', $cellDomElements[1]->getAttribute('s'));
+
+        // Second row has 2 styled cells
+        $this->assertEquals('1', $cellDomElements[2]->getAttribute('s'));
+        $this->assertEquals('1', $cellDomElements[3]->getAttribute('s'));
+
+        // Third row has 3 styled cells
+        $this->assertEquals('2', $cellDomElements[4]->getAttribute('s'));
+        $this->assertEquals('2', $cellDomElements[5]->getAttribute('s'));
+        $this->assertEquals('2', $cellDomElements[6]->getAttribute('s'));
+
+        // Third row has 3 styled cells
+        $this->assertEquals('3', $cellDomElements[7]->getAttribute('s'));
+        $this->assertEquals('3', $cellDomElements[8]->getAttribute('s'));
+        $this->assertEquals('3', $cellDomElements[9]->getAttribute('s'));
+    }
+
     /**
      * @return void
      */
@@ -403,17 +450,12 @@ public function testSetDefaultRowStyle()
      */
     public function testReUseBorders()
     {
-
         $fileName = 'test_reuse_borders.xlsx';
 
-        $borderLeft = (new BorderBuilder())
-            ->setBorderLeft()
-            ->build();
+        $borderLeft = (new BorderBuilder())->setBorderLeft()->build();
         $borderLeftStyle = (new StyleBuilder())->setBorder($borderLeft)->build();
 
-        $borderRight = (new BorderBuilder())
-            ->setBorderRight(Color::RED, Border::WIDTH_THICK)
-            ->build();
+        $borderRight = (new BorderBuilder())->setBorderRight(Color::RED, Border::WIDTH_THICK)->build();
         $borderRightStyle = (new StyleBuilder())->setBorder($borderRight)->build();
 
         $fontStyle = (new StyleBuilder())->setFontBold()->build();
@@ -436,7 +478,7 @@ public function testReUseBorders()
             $borderRightStyle,
             $borderRightFontBoldStyle
         ];
-        
+
         $this->writeToXLSXFileWithMultipleStyles($dataRows, $fileName, $styles);
         $borderElements = $this->getXmlSectionFromStylesXmlFile($fileName, 'borders');