DocBlock Changes for Chart/Title
This is a leftover Scrutinizer change, but it needed more attention than most others. Chart/Title DocBlocks define caption as `null|string`. However, in the wild, Excel usually presents the caption as an array, and not an array of strings but rather of RichText items. I am not sure why an array is needed since a RichText item can contain many text runs, but things are what they are. Reader/Xlsx/ChartTitleTest reads a spreadsheet with the captions stored as a RichText array. Since it performs array operations on something the DocBlock says cannot be an array, Scrutinizer objects, although not seriously enough to fail the module. Phpstan also objects; its objection is silenced with an annotation. Aside from this test, there are other tests which do set the caption to a string, and Excel seems to handle that without a problem. So, I have changed the DocBlock to specify `array|RichText|String`. I have dropped null as a possibility; nullstring will do equally well. Because getCaption can now return multiple datatypes, I think a new function which can return the text portion of the entire caption as a single string is needed. I have added it. This simplifies the test named above, and some code in Writer/Html. The latter is not part of unit testing because the version of JpGraph found in Composer is too antiquated. I verified the Html change manually by running samples/Chart/32_Chart_read_write_HTML.php using a recent version of JpGraph. It was as a result of this test that I uncovered issue #2203. I did not see anything about Charts in docs, so did not add a description of the new function there. Phpstan is happy with the changes. We'll see how Scrutinizer feels when I push it.
This commit is contained in:
Owen Leibman
parent
eb0cda1d63
commit
ecb4a7fe27
|
|
@ -6210,16 +6210,6 @@ parameters:
|
|||
count: 45
|
||||
path: src/PhpSpreadsheet/Writer/Xlsx/Chart.php
|
||||
|
||||
-
|
||||
message: "#^Call to function is_array\\(\\) with string will always evaluate to false\\.$#"
|
||||
count: 3
|
||||
path: src/PhpSpreadsheet/Writer/Xlsx/Chart.php
|
||||
|
||||
-
|
||||
message: "#^Result of && is always false\\.$#"
|
||||
count: 1
|
||||
path: src/PhpSpreadsheet/Writer/Xlsx/Chart.php
|
||||
|
||||
-
|
||||
message: "#^Strict comparison using \\=\\=\\= between PhpOffice\\\\PhpSpreadsheet\\\\Chart\\\\PlotArea and null will always evaluate to false\\.$#"
|
||||
count: 1
|
||||
|
|
|
|||
|
|
@ -2,14 +2,16 @@
|
|||
|
||||
namespace PhpOffice\PhpSpreadsheet\Chart;
|
||||
|
||||
use PhpOffice\PhpSpreadsheet\RichText\RichText;
|
||||
|
||||
class Title
|
||||
{
|
||||
/**
|
||||
* Title Caption.
|
||||
*
|
||||
* @var string
|
||||
* @var array|RichText|string
|
||||
*/
|
||||
private $caption;
|
||||
private $caption = '';
|
||||
|
||||
/**
|
||||
* Title Layout.
|
||||
|
|
@ -21,9 +23,9 @@ class Title
|
|||
/**
|
||||
* Create a new Title.
|
||||
*
|
||||
* @param null|mixed $caption
|
||||
* @param array|RichText|string $caption
|
||||
*/
|
||||
public function __construct($caption = null, ?Layout $layout = null)
|
||||
public function __construct($caption = '', ?Layout $layout = null)
|
||||
{
|
||||
$this->caption = $caption;
|
||||
$this->layout = $layout;
|
||||
|
|
@ -32,17 +34,40 @@ class Title
|
|||
/**
|
||||
* Get caption.
|
||||
*
|
||||
* @return string
|
||||
* @return array|RichText|string
|
||||
*/
|
||||
public function getCaption()
|
||||
{
|
||||
return $this->caption;
|
||||
}
|
||||
|
||||
public function getCaptionText(): string
|
||||
{
|
||||
$caption = $this->caption;
|
||||
if (is_string($caption)) {
|
||||
return $caption;
|
||||
}
|
||||
if ($caption instanceof RichText) {
|
||||
return $caption->getPlainText();
|
||||
}
|
||||
$retVal = '';
|
||||
foreach ($caption as $textx) {
|
||||
/** @var RichText|string */
|
||||
$text = $textx;
|
||||
if ($text instanceof RichText) {
|
||||
$retVal .= $text->getPlainText();
|
||||
} else {
|
||||
$retVal .= $text;
|
||||
}
|
||||
}
|
||||
|
||||
return $retVal;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set caption.
|
||||
*
|
||||
* @param string $caption
|
||||
* @param array|RichText|string $caption
|
||||
*
|
||||
* @return $this
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -749,7 +749,7 @@ class Html extends BaseWriter
|
|||
$html .= PHP_EOL;
|
||||
$imageDetails = getimagesize($chartFileName);
|
||||
$filedesc = $chart->getTitle();
|
||||
$filedesc = $filedesc ? self::getChartCaption($filedesc->getCaption()) : '';
|
||||
$filedesc = $filedesc ? $filedesc->getCaptionText() : '';
|
||||
$filedesc = $filedesc ? htmlspecialchars($filedesc, ENT_QUOTES) : 'Embedded chart';
|
||||
if ($fp = fopen($chartFileName, 'rb', 0)) {
|
||||
$picture = fread($fp, filesize($chartFileName));
|
||||
|
|
@ -770,27 +770,6 @@ class Html extends BaseWriter
|
|||
return $html;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extend Row if chart is placed after nominal end of row.
|
||||
* This code should be exercised by sample:
|
||||
* Chart/32_Chart_read_write_PDF.php.
|
||||
* However, that test is suppressed due to out-of-date
|
||||
* Jpgraph code issuing warnings. So, don't measure
|
||||
* code coverage for this function till that is fixed.
|
||||
* Caption is described in documentation as fixed,
|
||||
* but in 32_Chart it is somehow an array of RichText.
|
||||
*
|
||||
* @param mixed $cap
|
||||
*
|
||||
* @return string
|
||||
*
|
||||
* @codeCoverageIgnore
|
||||
*/
|
||||
private static function getChartCaption($cap)
|
||||
{
|
||||
return is_array($cap) ? implode(' ', $cap) : $cap;
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate CSS styles.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -0,0 +1,58 @@
|
|||
<?php
|
||||
|
||||
namespace PhpOffice\PhpSpreadsheetTests\Chart;
|
||||
|
||||
use PhpOffice\PhpSpreadsheet\Chart\Title;
|
||||
use PhpOffice\PhpSpreadsheet\RichText\RichText;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
class TitleTest extends TestCase
|
||||
{
|
||||
public function testString(): void
|
||||
{
|
||||
$title = new Title('hello');
|
||||
self::assertSame('hello', $title->getCaption());
|
||||
self::assertSame('hello', $title->getCaptionText());
|
||||
}
|
||||
|
||||
public function testStringArray(): void
|
||||
{
|
||||
$title = new Title();
|
||||
$title->setCaption(['Hello', ', ', 'world.']);
|
||||
self::assertSame('Hello, world.', $title->getCaptionText());
|
||||
}
|
||||
|
||||
public function testRichText(): void
|
||||
{
|
||||
$title = new Title();
|
||||
$richText = new RichText();
|
||||
$part = $richText->createTextRun('Hello');
|
||||
$font = $part->getFont();
|
||||
if ($font === null) {
|
||||
self::fail('Unable to retrieve font');
|
||||
} else {
|
||||
$font->setBold(true);
|
||||
$title->setCaption($richText);
|
||||
self::assertSame('Hello', $title->getCaptionText());
|
||||
}
|
||||
}
|
||||
|
||||
public function testMixedArray(): void
|
||||
{
|
||||
$title = new Title();
|
||||
$richText1 = new RichText();
|
||||
$part1 = $richText1->createTextRun('Hello');
|
||||
$font1 = $part1->getFont();
|
||||
$richText2 = new RichText();
|
||||
$part2 = $richText2->createTextRun('world');
|
||||
$font2 = $part2->getFont();
|
||||
if ($font1 === null || $font2 === null) {
|
||||
self::fail('Unable to retrieve font');
|
||||
} else {
|
||||
$font1->setBold(true);
|
||||
$font2->setItalic(true);
|
||||
$title->setCaption([$richText1, ', ', $richText2, '.']);
|
||||
self::assertSame('Hello, world.', $title->getCaptionText());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -8,15 +8,9 @@ use PHPUnit\Framework\TestCase;
|
|||
|
||||
class ChartsTitleTest extends TestCase
|
||||
{
|
||||
private static function getTitleText(?Title $title): ?string
|
||||
private static function getTitleText(?Title $title): string
|
||||
{
|
||||
if (null === $title || empty($title->getCaption())) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return implode("\n", array_map(function ($rt) {
|
||||
return $rt->getPlainText();
|
||||
}, $title->getCaption())); // @phpstan-ignore-line
|
||||
return ($title === null) ? '' : $title->getCaptionText();
|
||||
}
|
||||
|
||||
public function testChartTitles(): void
|
||||
|
|
|
|||
Loading…
Reference in New Issue