From 3e0afd858f362e62eab5235e80608100fc867281 Mon Sep 17 00:00:00 2001 From: Adrien Loison Date: Wed, 7 Sep 2016 17:04:31 -0700 Subject: [PATCH] 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. --- src/Spout/Writer/Style/Style.php | 2 +- src/Spout/Writer/XLSX/Helper/StyleHelper.php | 23 +++++++- src/Spout/Writer/XLSX/Internal/Workbook.php | 2 +- src/Spout/Writer/XLSX/Internal/Worksheet.php | 16 ++++- .../Writer/XLSX/Helper/StyleHelperTest.php | 24 ++++++++ .../Spout/Writer/XLSX/WriterWithStyleTest.php | 58 ++++++++++++++++--- 6 files changed, 110 insertions(+), 15 deletions(-) diff --git a/src/Spout/Writer/Style/Style.php b/src/Spout/Writer/Style/Style.php index a8813fb6..45057802 100644 --- 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 index 60dcd8d5..4a13c956 100644 --- 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 index 5208d4f3..bdf027fd 100644 --- 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 index 67e913c8..72aa4190 100644 --- 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 .= '>' . $cellValue . ''; } 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 index 333c1c2d..bcf9344c 100644 --- 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 index e18ac98e..e4d17497 100644 --- 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');