diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index 9bd4c477b..5c26e2dd8 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -569,14 +569,14 @@ class Worksheet /** * Get a chart by its index position. * - * @param ?string $index Chart index position + * @param null|int|string $index Chart index position * * @return Chart|false */ - public function getChartByIndex(?string $index) + public function getChartByIndex(null|int|string $index) { $chartCount = count($this->chartCollection); - if ($chartCount == 0) { + if ($chartCount === 0 || (is_string($index) && $index !== (string) (int) $index)) { return false; } if ($index === null) { diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Drawing.php b/src/PhpSpreadsheet/Writer/Xlsx/Drawing.php index 3f9bdb648..874982cb6 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Drawing.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Drawing.php @@ -60,15 +60,8 @@ class Drawing extends WriterPart } if ($includeCharts) { - $chartCount = $worksheet->getChartCount(); - // Loop through charts and write the chart position - if ($chartCount > 0) { - for ($c = 0; $c < $chartCount; ++$c) { - $chart = $worksheet->getChartByIndex((string) $c); - if ($chart !== false) { - $this->writeChart($objWriter, $chart, $c + $i); - } - } + foreach ($worksheet->getChartCollection() as $c => $chart) { + $this->writeChart($objWriter, $chart, $c + $i); } } diff --git a/tests/PhpSpreadsheetTests/Chart/ChartsByNameTest.php b/tests/PhpSpreadsheetTests/Chart/ChartsByNameTest.php index a3583672f..e6d792378 100644 --- a/tests/PhpSpreadsheetTests/Chart/ChartsByNameTest.php +++ b/tests/PhpSpreadsheetTests/Chart/ChartsByNameTest.php @@ -19,6 +19,7 @@ class ChartsByNameTest extends TestCase $spreadsheet = new Spreadsheet(); $sheet = $spreadsheet->getActiveSheet(); $sheet->setTitle('Only Sheet'); + self::assertFalse($sheet->getChartByIndex(null)); $sheet->fromArray( [ ['Some Title'], @@ -80,6 +81,13 @@ class ChartsByNameTest extends TestCase $sheet->setSelectedCells('D1'); self::assertSame($chart, $sheet->getChartByName('namedchart1')); self::assertSame($chart, $sheet->getChartByNameOrThrow('namedchart1')); + self::assertSame($chart, $sheet->getChartByIndex(0)); + self::assertSame($chart, $sheet->getChartByIndex('0')); + self::assertSame($chart, $sheet->getChartByIndex(null)); + self::assertFalse($sheet->getChartByIndex(' 0')); + self::assertFalse($sheet->getChartByIndex(1)); + self::assertFalse($sheet->getChartByIndex('2')); + self::assertFalse($sheet->getChartByIndex('x')); self::assertFalse($sheet->getChartByName('namedchart2')); try { diff --git a/tests/PhpSpreadsheetTests/Worksheet/CloneTest.php b/tests/PhpSpreadsheetTests/Worksheet/CloneTest.php index c1d9edf5e..5aa515f57 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/CloneTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/CloneTest.php @@ -7,16 +7,23 @@ namespace PhpOffice\PhpSpreadsheetTests\Worksheet; use PhpOffice\PhpSpreadsheet\Exception as SpreadsheetException; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; class CloneTest extends TestCase { - public function testUnattachedIndex(): void + #[DataProvider('providerCopyClone')] + public function testUnattachedIndex(string $type): void { $spreadsheet = new Spreadsheet(); $sheet1 = $spreadsheet->getActiveSheet(); $sheet1->getCell('A1')->setValue(10); - $sheet2 = clone $sheet1; + $sheet1->getCell('A2')->setValue(30); + if ($type === 'copy') { + $sheet2 = $sheet1->copy(); + } else { + $sheet2 = clone $sheet1; + } $sheet2->getCell('A1')->setValue(20); self::assertSame(0, $spreadsheet->getIndex($sheet1)); $idx = $spreadsheet->getIndex($sheet2, true); @@ -27,9 +34,19 @@ class CloneTest extends TestCase self::assertSame(1, $idx); self::assertSame(10, $spreadsheet->getSheet(0)->getCell('A1')->getValue()); self::assertSame(20, $spreadsheet->getSheet(1)->getCell('A1')->getValue()); + self::assertSame(30, $spreadsheet->getSheet(0)->getCell('A2')->getValue()); + self::assertSame(30, $spreadsheet->getSheet(1)->getCell('A2')->getValue()); $spreadsheet->disconnectWorksheets(); } + public static function providerCopyClone(): array + { + return [ + ['copy'], + ['clone'], + ]; + } + public function testGetCloneIndex(): void { $this->expectException(SpreadsheetException::class); diff --git a/tests/PhpSpreadsheetTests/Worksheet/ShrinkRangeToFitTest.php b/tests/PhpSpreadsheetTests/Worksheet/ShrinkRangeToFitTest.php new file mode 100644 index 000000000..a803e8337 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Worksheet/ShrinkRangeToFitTest.php @@ -0,0 +1,41 @@ +getActiveSheet(); + $sheet->getCell('B2')->setValue(1); + $sheet->getCell('E10')->setValue(2); + $testArray = [ + 'C3:F12' => 'C3:E10', + 'C3:D12' => 'C3:D10', + 'C3:D9' => 'C3:D9', + 'A1:B2 C3:D4' => 'A1:B2 C3:D4', + 'A1:B11 C3:D4' => 'A1:B10 C3:D4', + 'A1:B2 C3:F12' => 'A1:B2 C3:E10', + 'A1:B11 C3:F12' => 'A1:B10 C3:E10', + // In both of the following, the range + // isn't merely shrunk - it has moved. + // This doesn't seem right, although I am + // hard-pressed to come up with an alternative, + // and not willing to make a breaking change, + // even to code which probably isn't used much. + 'A11:B12' => 'A10:B10', + 'G1:H4' => 'E1:E4', + ]; + foreach ($testArray as $input => $expectedOutput) { + $output = $sheet->shrinkRangeToFit($input); + self::assertSame($expectedOutput, $output, $input); + } + $spreadsheet->disconnectWorksheets(); + } +}