Skip to content

Commit 1509097

Browse files
oleibmanoleibman
authored
Recalibrate Row/Column Dimensions After removeRow/Col (#2486)
Fix #2442. Although data and styles are handled correctly after removing row(s) or column(s), the dimensions of the removed rows and columns remain behind to afflict their replacements. This PR will take care of removing the dimensions as well. Dimensions has a _clone method for a deep clone, but all of its properties, as well as the properties of RowDimensions and ColumnDimensions, are scalars, and do not require a deep clone. The method is deleted.
1 parent 8ab8345 commit 1509097

File tree

4 files changed

+153
-18
lines changed

4 files changed

+153
-18
lines changed

‎src/PhpSpreadsheet/Worksheet/ColumnDimension.php

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PhpOffice\PhpSpreadsheet\Worksheet;
44

5+
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
56
use PhpOffice\PhpSpreadsheet\Helper\Dimension as CssDimension;
67

78
class ColumnDimension extends Dimension
@@ -53,16 +54,32 @@ public function getColumnIndex(): string
5354

5455
/**
5556
* Set column index as string eg: 'A'.
56-
*
57-
* @return $this
5857
*/
59-
public function setColumnIndex(string $index)
58+
public function setColumnIndex(string $index): self
6059
{
6160
$this->columnIndex = $index;
6261

6362
return $this;
6463
}
6564

65+
/**
66+
* Get column index as numeric.
67+
*/
68+
public function getColumnNumeric(): int
69+
{
70+
return Coordinate::columnIndexFromString($this->columnIndex);
71+
}
72+
73+
/**
74+
* Set column index as numeric.
75+
*/
76+
public function setColumnNumeric(int $index): self
77+
{
78+
$this->columnIndex = Coordinate::stringFromColumnIndex($index);
79+
80+
return $this;
81+
}
82+
6683
/**
6784
* Get Width.
6885
*

‎src/PhpSpreadsheet/Worksheet/Dimension.php

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -131,19 +131,4 @@ public function setXfIndex(int $XfIndex)
131131

132132
return $this;
133133
}
134-
135-
/**
136-
* Implement PHP __clone to create a deep clone, not just a shallow copy.
137-
*/
138-
public function __clone()
139-
{
140-
$vars = get_object_vars($this);
141-
foreach ($vars as $key => $value) {
142-
if (is_object($value)) {
143-
$this->$key = clone $value;
144-
} else {
145-
$this->$key = $value;
146-
}
147-
}
148-
}
149134
}

‎src/PhpSpreadsheet/Worksheet/Worksheet.php

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2101,6 +2101,7 @@ public function removeRow($row, $numberOfRows = 1)
21012101
throw new Exception('Rows to be deleted should at least start from row 1.');
21022102
}
21032103

2104+
$holdRowDimensions = $this->removeRowDimensions($row, $numberOfRows);
21042105
$highestRow = $this->getHighestDataRow();
21052106
$removedRowsCounter = 0;
21062107

@@ -2118,9 +2119,30 @@ public function removeRow($row, $numberOfRows = 1)
21182119
--$highestRow;
21192120
}
21202121

2122+
$this->rowDimensions = $holdRowDimensions;
2123+
21212124
return $this;
21222125
}
21232126

2127+
private function removeRowDimensions(int $row, int $numberOfRows): array
2128+
{
2129+
$highRow = $row + $numberOfRows - 1;
2130+
$holdRowDimensions = [];
2131+
foreach ($this->rowDimensions as $rowDimension) {
2132+
$num = $rowDimension->getRowIndex();
2133+
if ($num < $row) {
2134+
$holdRowDimensions[$num] = $rowDimension;
2135+
} elseif ($num > $highRow) {
2136+
$num -= $numberOfRows;
2137+
$cloneDimension = clone $rowDimension;
2138+
$cloneDimension->setRowIndex($num);
2139+
$holdRowDimensions[$num] = $cloneDimension;
2140+
}
2141+
}
2142+
2143+
return $holdRowDimensions;
2144+
}
2145+
21242146
/**
21252147
* Remove a column, updating all possible related data.
21262148
*
@@ -2143,6 +2165,8 @@ public function removeColumn($column, $numberOfColumns = 1)
21432165
return $this;
21442166
}
21452167

2168+
$holdColumnDimensions = $this->removeColumnDimensions($pColumnIndex, $numberOfColumns);
2169+
21462170
$column = Coordinate::stringFromColumnIndex($pColumnIndex + $numberOfColumns);
21472171
$objReferenceHelper = ReferenceHelper::getInstance();
21482172
$objReferenceHelper->insertNewBefore($column . '1', -$numberOfColumns, 0, $this);
@@ -2154,11 +2178,33 @@ public function removeColumn($column, $numberOfColumns = 1)
21542178
$highestColumn = Coordinate::stringFromColumnIndex(Coordinate::columnIndexFromString($highestColumn) - 1);
21552179
}
21562180

2181+
$this->columnDimensions = $holdColumnDimensions;
2182+
21572183
$this->garbageCollect();
21582184

21592185
return $this;
21602186
}
21612187

2188+
private function removeColumnDimensions(int $pColumnIndex, int $numberOfColumns): array
2189+
{
2190+
$highCol = $pColumnIndex + $numberOfColumns - 1;
2191+
$holdColumnDimensions = [];
2192+
foreach ($this->columnDimensions as $columnDimension) {
2193+
$num = $columnDimension->getColumnNumeric();
2194+
if ($num < $pColumnIndex) {
2195+
$str = $columnDimension->getColumnIndex();
2196+
$holdColumnDimensions[$str] = $columnDimension;
2197+
} elseif ($num > $highCol) {
2198+
$cloneDimension = clone $columnDimension;
2199+
$cloneDimension->setColumnNumeric($num - $numberOfColumns);
2200+
$str = $cloneDimension->getColumnIndex();
2201+
$holdColumnDimensions[$str] = $cloneDimension;
2202+
}
2203+
}
2204+
2205+
return $holdColumnDimensions;
2206+
}
2207+
21622208
/**
21632209
* Remove a column, updating all possible related data.
21642210
*

‎tests/PhpSpreadsheetTests/Worksheet/RemoveTest.php

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
4+
5+
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
6+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
7+
use PhpOffice\PhpSpreadsheet\Style\Color;
8+
use PhpOffice\PhpSpreadsheet\Style\Fill;
9+
use PHPUnit\Framework\TestCase;
10+
11+
class RemoveTest extends TestCase
12+
{
13+
public function testRemoveRow(): void
14+
{
15+
$spreadsheet = new Spreadsheet();
16+
$sheet = $spreadsheet->getActiveSheet();
17+
$fillColors = [
18+
'FFFF0000',
19+
'FF00FF00',
20+
'FF0000FF',
21+
];
22+
$rowHeights = [-1, -1, 1.2, 1.3, 1.4, 1.5, -1, -1, -1];
23+
for ($row = 1; $row < 10; ++$row) {
24+
$sheet->getCell("B$row")
25+
->getStyle()
26+
->getFill()
27+
->setFillType(Fill::FILL_SOLID)
28+
->setStartColor(new Color($fillColors[$row % 3]));
29+
$sheet->getCell("B$row")->setValue("X$row");
30+
$height = $rowHeights[$row - 1];
31+
if ($height > 0) {
32+
$sheet->getRowDimension($row)->setRowHeight($height);
33+
}
34+
}
35+
//$mapRow = [1, 2, 3, 4, 5, 6, 7, 8, 9];
36+
$sheet->removeRow(4, 2);
37+
$mapRow = [1, 2, 3, 6, 7, 8, 9];
38+
$rowCount = count($mapRow);
39+
for ($row = 1; $row <= $rowCount; ++$row) {
40+
$mappedRow = $mapRow[$row - 1];
41+
self::assertSame("X$mappedRow", $sheet->getCell("B$row")->getValue(), "Row value $row mapped to $mappedRow");
42+
self::assertSame($fillColors[$mappedRow % 3], $sheet->getCell("B$row")->getStyle()->getFill()->getStartColor()->getArgb(), "Row fill color $row mapped to $mappedRow");
43+
self::assertSame($rowHeights[$mappedRow - 1], $sheet->getRowDimension($row)->getRowHeight(), "Row height $row mapped to $mappedRow");
44+
}
45+
46+
$spreadsheet->disconnectWorksheets();
47+
}
48+
49+
public function testRemoveColumn(): void
50+
{
51+
$spreadsheet = new Spreadsheet();
52+
$sheet = $spreadsheet->getActiveSheet();
53+
$fillColors = [
54+
'FFFF0000',
55+
'FF00FF00',
56+
'FF0000FF',
57+
];
58+
$colWidths = [-1, -1, 1.2, 1.3, 1.4, 1.5, -1, -1, -1];
59+
for ($colNumber = 1; $colNumber < 10; ++$colNumber) {
60+
$col = Coordinate::stringFromColumnIndex($colNumber);
61+
$sheet->getCell("{$col}1")
62+
->getStyle()
63+
->getFill()
64+
->setFillType(Fill::FILL_SOLID)
65+
->setStartColor(new Color($fillColors[$colNumber % 3]));
66+
$sheet->getCell("{$col}1")->setValue("100$col");
67+
$width = $colWidths[$colNumber - 1];
68+
if ($width > 0) {
69+
$sheet->getColumnDimension($col)->setWidth($width);
70+
}
71+
}
72+
//$mapCol = [1, 2, 3, 4, 5, 6, 7, 8, 9];
73+
$sheet->removeColumn('D', 2);
74+
$mapCol = [1, 2, 3, 6, 7, 8, 9];
75+
$colCount = count($mapCol);
76+
for ($colNumber = 1; $colNumber < $colCount; ++$colNumber) {
77+
$col = Coordinate::stringFromColumnIndex($colNumber);
78+
$mappedCol = $mapCol[$colNumber - 1];
79+
$mappedColString = Coordinate::stringFromColumnIndex($mappedCol);
80+
self::assertSame("100$mappedColString", $sheet->getCell("{$col}1")->getValue(), "Column value $colNumber mapped to $mappedCol");
81+
self::assertSame($fillColors[$mappedCol % 3], $sheet->getCell("{$col}1")->getStyle()->getFill()->getStartColor()->getArgb(), "Col fill color $col mapped to $mappedColString");
82+
self::assertEquals($colWidths[$mappedCol - 1], $sheet->getColumnDimension($col)->getWidth(), "Col width $col mapped to $mappedColString");
83+
}
84+
85+
$spreadsheet->disconnectWorksheets();
86+
}
87+
}

0 commit comments

Comments
 (0)