Skip to content

Commit 0c93bba

Browse files
tiagomalheiroPowerKiKi
tiagomalheiro
authored and
PowerKiKi
committed
Don't corrupt file when using chart with fill color
When the fill color property of `DataSeries.plotLabel` using a DataSeriesValues on a line chart is set, the XLSX file written is corrupted, and MSExcel2016 removes the drawing1.xml if forced open. This problem was already documented on issue #589 along with a possible solution. So all credits go to @madrussa. I am only submitting the PR. Fixes #589 Closes #1930
1 parent b674042 commit 0c93bba

File tree

2 files changed

+172
-1
lines changed

2 files changed

+172
-1
lines changed

‎src/PhpSpreadsheet/Writer/Xlsx/Chart.php‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1058,7 +1058,7 @@ private function writePlotGroup(?DataSeries $plotGroup, $groupType, XMLWriter $o
10581058
$objWriter->startElement('c:ser');
10591059

10601060
$plotLabel = $plotGroup->getPlotLabelByIndex($plotSeriesIdx);
1061-
if ($plotLabel) {
1061+
if ($plotLabel && $groupType !== DataSeries::TYPE_LINECHART) {
10621062
$fillColor = $plotLabel->getFillColor();
10631063
if ($fillColor !== null && !is_array($fillColor)) {
10641064
$objWriter->startElement('c:spPr');
@@ -1116,6 +1116,15 @@ private function writePlotGroup(?DataSeries $plotGroup, $groupType, XMLWriter $o
11161116
if ($groupType == DataSeries::TYPE_STOCKCHART) {
11171117
$objWriter->startElement('a:noFill');
11181118
$objWriter->endElement();
1119+
} elseif ($plotLabel) {
1120+
$fillColor = $plotLabel->getFillColor();
1121+
if (is_string($fillColor)) {
1122+
$objWriter->startElement('a:solidFill');
1123+
$objWriter->startElement('a:srgbClr');
1124+
$objWriter->writeAttribute('val', $fillColor);
1125+
$objWriter->endElement();
1126+
$objWriter->endElement();
1127+
}
11191128
}
11201129
$objWriter->endElement();
11211130
$objWriter->endElement();

‎tests/PhpSpreadsheetTests/Writer/Xlsx/Issue589Test.php‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;
4+
5+
use DOMDocument;
6+
use DOMNode;
7+
use PhpOffice\PhpSpreadsheet\Chart\Chart;
8+
use PhpOffice\PhpSpreadsheet\Chart\DataSeries;
9+
use PhpOffice\PhpSpreadsheet\Chart\DataSeriesValues;
10+
use PhpOffice\PhpSpreadsheet\Chart\PlotArea;
11+
use PhpOffice\PhpSpreadsheet\Shared\File;
12+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
13+
use PhpOffice\PhpSpreadsheet\Writer\Xlsx as Writer;
14+
use PHPUnit\Framework\TestCase;
15+
use ZipArchive;
16+
17+
class Issue589Test extends TestCase
18+
{
19+
/**
20+
* Build a testable chart in a spreadsheet and set fill color for series.
21+
*
22+
* @param string|string[] $color HEX color or array with HEX colors
23+
*/
24+
private function buildChartSpreadsheet($color): Spreadsheet
25+
{
26+
// Problem occurs when setting plot line color
27+
// The output chart xml file is missing the a:ln tag
28+
$spreadsheet = new Spreadsheet();
29+
$worksheet = $spreadsheet->getActiveSheet();
30+
$worksheet->fromArray(
31+
[
32+
[2010],
33+
[12],
34+
[56],
35+
]
36+
);
37+
38+
$dataSeriesLabels = [
39+
new DataSeriesValues(DataSeriesValues::DATASERIES_TYPE_STRING, 'Worksheet!$A$1', null, 1),
40+
];
41+
$dataSeriesLabels[0]->setFillColor($color);
42+
$dataSeriesValues = [
43+
new DataSeriesValues(DataSeriesValues::DATASERIES_TYPE_NUMBER, 'Worksheet!$A$2:$A$3', null, 2),
44+
];
45+
46+
// Build the dataseries
47+
$series = new DataSeries(
48+
DataSeries::TYPE_LINECHART,
49+
DataSeries::GROUPING_STACKED,
50+
range(0, count($dataSeriesValues) - 1),
51+
$dataSeriesLabels,
52+
[],
53+
$dataSeriesValues
54+
);
55+
56+
// Set the series in the plot area
57+
$plotArea = new PlotArea(null, [$series]);
58+
59+
// Create the chart
60+
$chart = new Chart(
61+
'chart1',
62+
null,
63+
null,
64+
$plotArea
65+
);
66+
67+
// Add the chart to the worksheet
68+
$worksheet->addChart($chart);
69+
70+
return $spreadsheet;
71+
}
72+
73+
public function testLineChartFill(): void
74+
{
75+
$outputFilename = File::temporaryFilename();
76+
$spreadsheet = $this->buildChartSpreadsheet('98B954');
77+
$writer = new Writer($spreadsheet);
78+
$writer->setIncludeCharts(true);
79+
$writer->save($outputFilename);
80+
81+
$zip = new ZipArchive();
82+
$zip->open($outputFilename);
83+
$resultChart1Raw = $zip->getFromName('xl/charts/chart1.xml');
84+
$zip->close();
85+
unlink($outputFilename);
86+
87+
$dom = new DOMDocument();
88+
if ($resultChart1Raw === false) {
89+
self::fail('Unable to open the chart file');
90+
} else {
91+
$loaded = $dom->loadXML($resultChart1Raw);
92+
if (!$loaded) {
93+
self::fail('Unable to load the chart xml');
94+
} else {
95+
$series = $dom->getElementsByTagName('ser');
96+
$firstSeries = $series->item(0);
97+
if ($firstSeries === null) {
98+
self::fail('The chart XML does not contain a \'ser\' tag!');
99+
} else {
100+
$spPrList = $firstSeries->getElementsByTagName('spPr');
101+
102+
// expect to see only one element with name 'c:spPr'
103+
self::assertCount(1, $spPrList);
104+
105+
/** @var DOMNode $node */
106+
$node = $spPrList->item(0); // Get the spPr element
107+
$actualXml = $dom->saveXML($node);
108+
if ($actualXml === false) {
109+
self::fail('Failure saving the spPr element as xml string!');
110+
} else {
111+
self::assertXmlStringEqualsXmlString('<c:spPr><a:ln w="12700"><a:solidFill><a:srgbClr val="98B954"/></a:solidFill></a:ln></c:spPr>', $actualXml);
112+
}
113+
}
114+
}
115+
}
116+
}
117+
118+
public function testLineChartFillIgnoresColorArray(): void
119+
{
120+
$outputFilename = File::temporaryFilename();
121+
$spreadsheet = $this->buildChartSpreadsheet(['98B954']);
122+
$writer = new Writer($spreadsheet);
123+
$writer->setIncludeCharts(true);
124+
$writer->save($outputFilename);
125+
126+
$zip = new ZipArchive();
127+
$zip->open($outputFilename);
128+
$resultChart1Raw = $zip->getFromName('xl/charts/chart1.xml');
129+
$zip->close();
130+
unlink($outputFilename);
131+
132+
$dom = new DOMDocument();
133+
if ($resultChart1Raw === false) {
134+
self::fail('Unable to open the chart file');
135+
} else {
136+
$loaded = $dom->loadXML($resultChart1Raw);
137+
if (!$loaded) {
138+
self::fail('Unable to load the chart xml');
139+
} else {
140+
$series = $dom->getElementsByTagName('ser');
141+
$firstSeries = $series->item(0);
142+
if ($firstSeries === null) {
143+
self::fail('The chart XML does not contain a \'ser\' tag!');
144+
} else {
145+
$spPrList = $firstSeries->getElementsByTagName('spPr');
146+
147+
// expect to see only one element with name 'c:spPr'
148+
self::assertCount(1, $spPrList);
149+
150+
/** @var DOMNode $node */
151+
$node = $spPrList->item(0); // Get the spPr element
152+
$actualXml = $dom->saveXML($node);
153+
if ($actualXml === false) {
154+
self::fail('Failure saving the spPr element as xml string!');
155+
} else {
156+
self::assertXmlStringEqualsXmlString('<c:spPr><a:ln w="12700"></a:ln></c:spPr>', $actualXml);
157+
}
158+
}
159+
}
160+
}
161+
}
162+
}

0 commit comments

Comments
 (0)