Skip to content

Commit

Permalink
Merge pull request #4250 from oleibman/issue4248
Browse files Browse the repository at this point in the history
Fill Patterns/Colors When Xml Attributes are Missing
  • Loading branch information
oleibman authored Dec 6, 2024
2 parents e78ab77 + eeef6dc commit 9dc82e6
Show file tree
Hide file tree
Showing 9 changed files with 170 additions and 19 deletions.
30 changes: 27 additions & 3 deletions src/PhpSpreadsheet/Reader/Xlsx/Styles.php
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,22 @@ public function readFillStyle(Fill $fillStyle, SimpleXMLElement $fillStyleXml):
$fillStyle->getStartColor()->setARGB($this->readColor(self::getArrayItem($gradientFill->xpath('sml:stop[@position=0]'))->color)); //* @phpstan-ignore-line
$fillStyle->getEndColor()->setARGB($this->readColor(self::getArrayItem($gradientFill->xpath('sml:stop[@position=1]'))->color)); //* @phpstan-ignore-line
} elseif ($fillStyleXml->patternFill) {
$defaultFillStyle = Fill::FILL_NONE;
$defaultFillStyle = ($fillStyle->getFillType() !== null) ? Fill::FILL_NONE : '';
$fgFound = false;
$bgFound = false;
if ($fillStyleXml->patternFill->fgColor) {
$fillStyle->getStartColor()->setARGB($this->readColor($fillStyleXml->patternFill->fgColor, true));
$defaultFillStyle = Fill::FILL_SOLID;
if ($fillStyle->getFillType() !== null) {
$defaultFillStyle = Fill::FILL_SOLID;
}
$fgFound = true;
}
if ($fillStyleXml->patternFill->bgColor) {
$fillStyle->getEndColor()->setARGB($this->readColor($fillStyleXml->patternFill->bgColor, true));
$defaultFillStyle = Fill::FILL_SOLID;
if ($fillStyle->getFillType() !== null) {
$defaultFillStyle = Fill::FILL_SOLID;
}
$bgFound = true;
}

$type = '';
Expand All @@ -169,6 +177,22 @@ public function readFillStyle(Fill $fillStyle, SimpleXMLElement $fillStyleXml):
$patternType = ($type === '') ? $defaultFillStyle : $type;

$fillStyle->setFillType($patternType);
if (
!$fgFound // no foreground color specified
&& !in_array($patternType, [Fill::FILL_NONE, Fill::FILL_SOLID], true) // these patterns aren't relevant
&& $fillStyle->getStartColor()->getARGB() // not conditional
) {
$fillStyle->getStartColor()
->setARGB('', true);
}
if (
!$bgFound // no background color specified
&& !in_array($patternType, [Fill::FILL_NONE, Fill::FILL_SOLID], true) // these patterns aren't relevant
&& $fillStyle->getEndColor()->getARGB() // not conditional
) {
$fillStyle->getEndColor()
->setARGB('', true);
}
}
}

Expand Down
10 changes: 6 additions & 4 deletions src/PhpSpreadsheet/Style/Color.php
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,14 @@ public function getARGB(): ?string
*
* @return $this
*/
public function setARGB(?string $colorValue = self::COLOR_BLACK): static
public function setARGB(?string $colorValue = self::COLOR_BLACK, bool $nullStringOkay = false): static
{
$this->hasChanged = true;
$colorValue = $this->validateColor($colorValue);
if ($colorValue === '') {
return $this;
if (!$nullStringOkay || $colorValue !== '') {
$colorValue = $this->validateColor($colorValue);
if ($colorValue === '') {
return $this;
}
}

if ($this->isSupervisor) {
Expand Down
13 changes: 10 additions & 3 deletions src/PhpSpreadsheet/Writer/Html.php
Original file line number Diff line number Diff line change
Expand Up @@ -1139,9 +1139,16 @@ private function createCSSStyleFill(Fill $fill): array

// Create CSS
if ($fill->getFillType() !== Fill::FILL_NONE) {
$value = $fill->getFillType() == Fill::FILL_NONE
? 'white' : '#' . $fill->getStartColor()->getRGB();
$css['background-color'] = $value;
if (
(in_array($fill->getFillType(), ['', Fill::FILL_SOLID], true) || !$fill->getEndColor()->getRGB())
&& $fill->getStartColor()->getRGB()
) {
$value = '#' . $fill->getStartColor()->getRGB();
$css['background-color'] = $value;
} elseif ($fill->getEndColor()->getRGB()) {
$value = '#' . $fill->getEndColor()->getRGB();
$css['background-color'] = $value;
}
}

return $css;
Expand Down
16 changes: 14 additions & 2 deletions src/PhpSpreadsheet/Writer/Xls/Workbook.php
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,20 @@ public function addXfWriter(Style $style, bool $isStyleXf = false): int
$xfWriter->setFontIndex($fontIndex);

// Background colors, best to treat these after the font so black will come after white in custom palette
$xfWriter->setFgColor($this->addColor($style->getFill()->getStartColor()->getRGB()));
$xfWriter->setBgColor($this->addColor($style->getFill()->getEndColor()->getRGB()));
if ($style->getFill()->getStartColor()->getRGB()) {
$xfWriter->setFgColor(
$this->addColor(
$style->getFill()->getStartColor()->getRGB()
)
);
}
if ($style->getFill()->getEndColor()->getRGB()) {
$xfWriter->setBgColor(
$this->addColor(
$style->getFill()->getEndColor()->getRGB()
)
);
}
$xfWriter->setBottomColor($this->addColor($style->getBorders()->getBottom()->getColor()->getRGB()));
$xfWriter->setTopColor($this->addColor($style->getBorders()->getTop()->getColor()->getRGB()));
$xfWriter->setRightColor($this->addColor($style->getBorders()->getRight()->getColor()->getRGB()));
Expand Down
6 changes: 3 additions & 3 deletions src/PhpSpreadsheet/Writer/Xls/Worksheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -2875,9 +2875,9 @@ private function writeCFRule(
$bFormatBorder = 0;
}
// Pattern
$bFillStyle = ($conditional->getStyle()->getFill()->getFillType() === null ? 0 : 1);
$bFillColor = ($conditional->getStyle()->getFill()->getStartColor()->getARGB() === null ? 0 : 1);
$bFillColorBg = ($conditional->getStyle()->getFill()->getEndColor()->getARGB() === null ? 0 : 1);
$bFillStyle = $conditional->getStyle()->getFill()->getFillType() ? 1 : 0;
$bFillColor = $conditional->getStyle()->getFill()->getStartColor()->getARGB() ? 1 : 0;
$bFillColorBg = $conditional->getStyle()->getFill()->getEndColor()->getARGB() ? 1 : 0;
if ($bFillStyle == 1 || $bFillColor == 1 || $bFillColorBg == 1) {
$bFormatFill = 1;
} else {
Expand Down
8 changes: 5 additions & 3 deletions src/PhpSpreadsheet/Writer/Xlsx/Style.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ private function writeGradientFill(XMLWriter $objWriter, Fill $fill): void
$objWriter->writeAttribute('position', '0');

// color
if ($fill->getStartColor()->getARGB() !== null) {
if (!empty($fill->getStartColor()->getARGB())) {
$objWriter->startElement('color');
$objWriter->writeAttribute('rgb', $fill->getStartColor()->getARGB());
$objWriter->endElement();
Expand All @@ -212,7 +212,7 @@ private function writeGradientFill(XMLWriter $objWriter, Fill $fill): void
$objWriter->writeAttribute('position', '1');

// color
if ($fill->getEndColor()->getARGB() !== null) {
if (!empty($fill->getEndColor()->getARGB())) {
$objWriter->startElement('color');
$objWriter->writeAttribute('rgb', $fill->getEndColor()->getARGB());
$objWriter->endElement();
Expand Down Expand Up @@ -244,7 +244,9 @@ private function writePatternFill(XMLWriter $objWriter, Fill $fill): void

// patternFill
$objWriter->startElement('patternFill');
$objWriter->writeAttribute('patternType', (string) $fill->getFillType());
if ($fill->getFillType()) {
$objWriter->writeAttribute('patternType', (string) $fill->getFillType());
}

if (self::writePatternColors($fill)) {
// fgColor
Expand Down
2 changes: 1 addition & 1 deletion tests/PhpSpreadsheetTests/Reader/Xlsx/DefaultFillTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ public function testDefaultConditionalFill(): void
$spreadsheet = $reader->load($filename);

$style = $spreadsheet->getActiveSheet()->getConditionalStyles('A1')[0]->getStyle();
self::assertSame('solid', $style->getFill()->getFillType());
self::assertSame('', $style->getFill()->getFillType());
}
}
104 changes: 104 additions & 0 deletions tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4248Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
<?php

declare(strict_types=1);

namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;

use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
use PhpOffice\PhpSpreadsheet\Shared\File;
use PhpOffice\PhpSpreadsheet\Writer\Html as HtmlWriter;
use PhpOffice\PhpSpreadsheet\Writer\Xlsx as XlsxWriter;
use PHPUnit\Framework\TestCase;

class Issue4248Test extends TestCase
{
private string $outfile = '';

protected function tearDown(): void
{
if ($this->outfile !== '') {
unlink($this->outfile);
$this->outfile = '';
}
}

public function testStyles(): void
{
$file = 'tests/data/Reader/XLSX/issue.4248.xlsx';
$reader = new XlsxReader();
$spreadsheet = $reader->load($file);
$writer = new XlsxWriter($spreadsheet);
$this->outfile = File::temporaryFilename();
$writer->save($this->outfile);
$spreadsheet->disconnectWorksheets();

$file = 'zip://';
$file .= $this->outfile;
$file .= '#xl/styles.xml';
$data = file_get_contents($file) ?: '';
$expected = '<fill>'
. '<patternFill patternType="darkDown"/>'
. '</fill>';
self::assertStringContainsString($expected, $data, 'neither fgColor nor bgColor');
$expected = '<fill>'
. '<patternFill patternType="darkDown">'
. '<bgColor rgb="FFBDD7EE"/>'
. '</patternFill></fill>';
self::assertStringContainsString($expected, $data, 'bgColor but no fgColor');
$expected = '<dxfs count="15">'
. '<dxf>' // dxfId 1 - fill color for Oui
. '<fill>'
. '<patternFill><bgColor rgb="FF00B050"/></patternFill>'
. '</fill>'
. '<border/>'
. '</dxf>'
. '<dxf>' // dxfId 2 - fill color for Non
. '<font><color rgb="FF9C0006"/></font>'
. '<fill>'
. '<patternFill><bgColor rgb="FFFFC7CE"/></patternFill>'
. '</fill>'
. '<border/>'
. '</dxf>';
self::assertStringContainsString($expected, $data, 'conditional fill styles');

$file = 'zip://';
$file .= $this->outfile;
$file .= '#xl/worksheets/sheet1.xml';
$data = file_get_contents($file) ?: '';
$expected = '<conditionalFormatting sqref="C16:C38 E17:H18 I17:J37 D18 J23:J38 E38 I38">'
. '<cfRule type="containsText" dxfId="0" priority="1" operator="containsText" text="Oui">'
. '<formula>NOT(ISERROR(SEARCH(&quot;Oui&quot;,C16)))</formula>'
. '</cfRule>'
. '</conditionalFormatting>';
self::assertStringContainsString($expected, $data, 'first condition for D18');
$expected = '<conditionalFormatting sqref="C16:C38 I17:J37 E17:H18 D18 J23:J38 E38 I38">'
. '<cfRule type="containsText" dxfId="1" priority="2" operator="containsText" text="Non">'
. '<formula>NOT(ISERROR(SEARCH(&quot;Non&quot;,C16)))</formula>'
. '</cfRule>'
. '</conditionalFormatting>';
self::assertStringContainsString($expected, $data, 'second condition for D18');
}

public function testHtml(): void
{
$file = 'tests/data/Reader/XLSX/issue.4248.xlsx';
$reader = new XlsxReader();
$spreadsheet = $reader->load($file);
$writer = new HtmlWriter($spreadsheet);

$file = 'zip://';
$file .= $this->outfile;
$file .= '#xl/styles.xml';
$data = str_replace(["\r", "\n"], '', $writer->generateHtmlAll());
$expected = ' <tr class="row17">' // Cell D18
. ' <td class="column0 style0">&nbsp;</td>'
. ' <td class="column1 style28 null"></td>'
. ' <td class="column2 style35 s">Eligible </td>'
. ' <td class="column3 style70 f">Non</td>';
self::assertStringContainsString($expected, $data, 'Cell D18 style');
$expected = ' td.style70, th.style70 { vertical-align:middle; text-align:center; border-bottom:1px solid #000000 !important; border-top:2px solid #000000 !important; border-left:2px solid #000000 !important; border-right:1px solid #000000 !important; font-weight:bold; color:#000000; font-family:\'Calibri\'; font-size:16pt; background-color:#BDD7EE }';
self::assertStringContainsString($expected, $data, 'background color');

$spreadsheet->disconnectWorksheets();
}
}
Binary file added tests/data/Reader/XLSX/issue.4248.xlsx
Binary file not shown.

0 comments on commit 9dc82e6

Please sign in to comment.