Skip to content

Commit 6b61da0

Browse files
oleibmanoleibman
committed
Column Widths Not Preserved When Using Read Filter
Fix #4416. A peculiar problem indeed. PhpSpreadsheet has been considering a column to be filtered if any cell in the column is filtered and does not preserve the column width if that is the case. It should consider the column not filtered if any cell in the column is not filtered, and consider it filtered only if there are no cells to which that applies. At least, that's how I think it should work, and this change doesn't break any existing tests, and solves this issue.
1 parent 74dca30 commit 6b61da0

File tree

4 files changed

+117
-14
lines changed

4 files changed

+117
-14
lines changed

‎src/PhpSpreadsheet/Reader/Xlsx/ColumnAndRowAttributes.php‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ public function load(?IReadFilter $readFilter = null, bool $readDataOnly = false
7777
if ($this->worksheetXml === null) {
7878
return;
7979
}
80+
if ($readFilter !== null && $readFilter::class === DefaultReadFilter::class) {
81+
$readFilter = null;
82+
}
8083

8184
$columnsAttributes = [];
8285
$rowsAttributes = [];
@@ -85,11 +88,7 @@ public function load(?IReadFilter $readFilter = null, bool $readDataOnly = false
8588
}
8689

8790
if ($this->worksheetXml->sheetData && $this->worksheetXml->sheetData->row) {
88-
$rowsAttributes = $this->readRowAttributes($this->worksheetXml->sheetData->row, $readDataOnly, $ignoreRowsWithNoCells);
89-
}
90-
91-
if ($readFilter !== null && $readFilter::class === DefaultReadFilter::class) {
92-
$readFilter = null;
91+
$rowsAttributes = $this->readRowAttributes($this->worksheetXml->sheetData->row, $readDataOnly, $ignoreRowsWithNoCells, $readFilter !== null);
9392
}
9493

9594
// set columns/rows attributes
@@ -123,12 +122,12 @@ public function load(?IReadFilter $readFilter = null, bool $readDataOnly = false
123122
private function isFilteredColumn(IReadFilter $readFilter, string $columnCoordinate, array $rowsAttributes): bool
124123
{
125124
foreach ($rowsAttributes as $rowCoordinate => $rowAttributes) {
126-
if (!$readFilter->readCell($columnCoordinate, $rowCoordinate, $this->worksheet->getTitle())) {
127-
return true;
125+
if ($readFilter->readCell($columnCoordinate, $rowCoordinate, $this->worksheet->getTitle())) {
126+
return false;
128127
}
129128
}
130129

131-
return false;
130+
return true;
132131
}
133132

134133
private function readColumnAttributes(SimpleXMLElement $worksheetCols, bool $readDataOnly): array
@@ -189,27 +188,31 @@ private function isFilteredRow(IReadFilter $readFilter, int $rowCoordinate, arra
189188
return false;
190189
}
191190

192-
private function readRowAttributes(SimpleXMLElement $worksheetRow, bool $readDataOnly, bool $ignoreRowsWithNoCells): array
191+
private function readRowAttributes(SimpleXMLElement $worksheetRow, bool $readDataOnly, bool $ignoreRowsWithNoCells, bool $readFilterIsNotNull): array
193192
{
194193
$rowAttributes = [];
195194

196195
foreach ($worksheetRow as $rowx) {
197196
$row = $rowx->attributes();
198197
if ($row !== null && (!$ignoreRowsWithNoCells || isset($rowx->c))) {
198+
$rowIndex = (int) $row['r'];
199199
if (isset($row['ht']) && !$readDataOnly) {
200-
$rowAttributes[(int) $row['r']]['rowHeight'] = (float) $row['ht'];
200+
$rowAttributes[$rowIndex]['rowHeight'] = (float) $row['ht'];
201201
}
202202
if (isset($row['hidden']) && self::boolean($row['hidden'])) {
203-
$rowAttributes[(int) $row['r']]['visible'] = false;
203+
$rowAttributes[$rowIndex]['visible'] = false;
204204
}
205205
if (isset($row['collapsed']) && self::boolean($row['collapsed'])) {
206-
$rowAttributes[(int) $row['r']]['collapsed'] = true;
206+
$rowAttributes[$rowIndex]['collapsed'] = true;
207207
}
208208
if (isset($row['outlineLevel']) && (int) $row['outlineLevel'] > 0) {
209-
$rowAttributes[(int) $row['r']]['outlineLevel'] = (int) $row['outlineLevel'];
209+
$rowAttributes[$rowIndex]['outlineLevel'] = (int) $row['outlineLevel'];
210210
}
211211
if (isset($row['s']) && !$readDataOnly) {
212-
$rowAttributes[(int) $row['r']]['xfIndex'] = (int) $row['s'];
212+
$rowAttributes[$rowIndex]['xfIndex'] = (int) $row['s'];
213+
}
214+
if ($readFilterIsNotNull && empty($rowAttributes[$rowIndex])) {
215+
$rowAttributes[$rowIndex]['exists'] = true;
213216
}
214217
}
215218
}

‎tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4416Filter.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
6+
7+
use PhpOffice\PhpSpreadsheet\Reader\IReadFilter;
8+
9+
class Issue4416Filter implements IReadFilter
10+
{
11+
public function readCell(string $columnAddress, int $row, string $worksheetName = ''): bool
12+
{
13+
$allowedColumns = ['A', 'B', 'C', 'D'];
14+
$allowedRows = range(1, 5);
15+
16+
return in_array($columnAddress, $allowedColumns, true) && in_array($row, $allowedRows, true);
17+
}
18+
}

‎tests/PhpSpreadsheetTests/Reader/Xlsx/Issue4416Test.php‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
6+
7+
use PhpOffice\PhpSpreadsheet\Reader\Xlsx as XlsxReader;
8+
use PHPUnit\Framework\TestCase;
9+
10+
class Issue4416Test extends TestCase
11+
{
12+
private static string $file = 'tests/data/Reader/XLSX/issue.4416.smallauto.xlsx';
13+
14+
public function testNoFilter(): void
15+
{
16+
$file = self::$file;
17+
$reader = new XlsxReader();
18+
$spreadsheet = $reader->load($file);
19+
$sheet = $spreadsheet->getActiveSheet();
20+
self::assertEqualsWithDelta(
21+
16.5430,
22+
$sheet->getColumnDimension('A')->getWidth(),
23+
1E-4
24+
);
25+
self::assertEqualsWithDelta(
26+
6.0,
27+
$sheet->getColumnDimension('B')->getWidth(),
28+
1E-4
29+
);
30+
self::assertEqualsWithDelta(
31+
11.3633,
32+
$sheet->getColumnDimension('C')->getWidth(),
33+
1E-4
34+
);
35+
self::assertEqualsWithDelta(
36+
41.0898,
37+
$sheet->getColumnDimension('D')->getWidth(),
38+
1E-4
39+
);
40+
self::assertEqualsWithDelta(
41+
28.5,
42+
$sheet->getRowDimension(6)->getRowHeight(),
43+
1E-4
44+
);
45+
$spreadsheet->disconnectWorksheets();
46+
}
47+
48+
public function testWithFilter(): void
49+
{
50+
$file = self::$file;
51+
$reader = new XlsxReader();
52+
$reader->setReadFilter(new Issue4416Filter());
53+
$spreadsheet = $reader->load($file);
54+
$sheet = $spreadsheet->getActiveSheet();
55+
self::assertEqualsWithDelta(
56+
16.5430,
57+
$sheet->getColumnDimension('A')->getWidth(),
58+
1E-4
59+
);
60+
self::assertEqualsWithDelta(
61+
6.0,
62+
$sheet->getColumnDimension('B')->getWidth(),
63+
1E-4
64+
);
65+
self::assertEqualsWithDelta(
66+
11.3633,
67+
$sheet->getColumnDimension('C')->getWidth(),
68+
1E-4
69+
);
70+
self::assertEqualsWithDelta(
71+
41.0898,
72+
$sheet->getColumnDimension('D')->getWidth(),
73+
1E-4
74+
);
75+
self::assertEquals(
76+
-1,
77+
$sheet->getRowDimension(6)->getRowHeight(),
78+
'row has been filtered away'
79+
);
80+
$spreadsheet->disconnectWorksheets();
81+
}
82+
}

‎tests/data/Reader/XLSX/issue.4416.smallauto.xlsx‎

9.39 KB
Binary file not shown.

0 commit comments

Comments
 (0)