Skip to content

Commit 33edddf

Browse files
committed
Split Conditional Ranges
Fix #4039. Excel sometimes stores the location for a Conditional as, say, `B1:B10 C1:C10` rather than `B1:C10`. PhpSpreadsheet does not have a problem with this, but internally stores it as separate Conditionals, one for each range. (There may be more than 2.) User would like it stored as a single Conditional. Since the range is used as the index of an array which holds the conditionals, there is no technical reason why this can't be done. And it does seem like being able to change multiple ranges all at once has some advantages, whether you're doing it in PhpSpreadsheet or in Excel itself. Making such a change in Xlsx Reader showed no problem in Xlsx Reader and Writer tests. A number of problems did show up in CellMatcherTest, and one in WizardFactoryTest. This was good news, because it meant some of our test spreadsheets used the same type of construction as the test spreadsheet supplied with the issue. So, if additional fixes make the test problems go away, I think no new tests are required. And a smattering of source changes did indeed result in a clean test suite once more. This is a change, but I don't really think it should break anyone. So I don't think it's necessary to add an option to opt in to the old or new behavior. I could be wrong. I'll leave this ticket open for at least a couple of weeks to see if anyone thinks otherwise.
1 parent 2ed696f commit 33edddf

File tree

4 files changed

+44
-9
lines changed

4 files changed

+44
-9
lines changed

src/PhpSpreadsheet/Reader/Xlsx/ConditionalStyles.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,8 @@ private function setConditionalStyles(Worksheet $worksheet, array $conditionals,
188188
$conditionalStyles = $this->readStyleRules($cfRules, $xmlExtLst);
189189

190190
// Extract all cell references in $cellRangeReference
191-
$cellBlocks = explode(' ', str_replace('$', '', strtoupper($cellRangeReference)));
192-
foreach ($cellBlocks as $cellBlock) {
193-
$worksheet->getStyle($cellBlock)->setConditionalStyles($conditionalStyles);
194-
}
191+
$cellRangeReference = str_replace('$', '', strtoupper($cellRangeReference));
192+
$worksheet->getStyle($cellRangeReference)->setConditionalStyles($conditionalStyles);
195193
}
196194
}
197195

src/PhpSpreadsheet/Style/ConditionalFormatting/CellMatcher.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public function __construct(Cell $cell, string $conditionalRange)
6060

6161
protected function setReferenceCellForExpressions(string $conditionalRange): void
6262
{
63+
$conditionalRange = str_replace(' ', ',', $conditionalRange);
6364
$conditionalRange = Coordinate::splitRange(str_replace('$', '', strtoupper($conditionalRange)));
6465
[$this->referenceCell] = $conditionalRange[0];
6566

src/PhpSpreadsheet/Worksheet/Worksheet.php

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,8 +1422,11 @@ public function getConditionalStyles(string $coordinate): array
14221422

14231423
$cell = $this->getCell($coordinate);
14241424
foreach (array_keys($this->conditionalStylesCollection) as $conditionalRange) {
1425-
if ($cell->isInRange($conditionalRange)) {
1426-
return $this->conditionalStylesCollection[$conditionalRange];
1425+
$cellBlocks = explode(' ', $conditionalRange);
1426+
foreach ($cellBlocks as $cellBlock) {
1427+
if ($cell->isInRange($cellBlock)) {
1428+
return $this->conditionalStylesCollection[$conditionalRange];
1429+
}
14271430
}
14281431
}
14291432

@@ -1435,8 +1438,11 @@ public function getConditionalRange(string $coordinate): ?string
14351438
$coordinate = strtoupper($coordinate);
14361439
$cell = $this->getCell($coordinate);
14371440
foreach (array_keys($this->conditionalStylesCollection) as $conditionalRange) {
1438-
if ($cell->isInRange($conditionalRange)) {
1439-
return $conditionalRange;
1441+
$cellBlocks = explode(' ', $conditionalRange);
1442+
foreach ($cellBlocks as $cellBlock) {
1443+
if ($cell->isInRange($cellBlock)) {
1444+
return $conditionalRange;
1445+
}
14401446
}
14411447
}
14421448

@@ -1501,7 +1507,7 @@ public function getConditionalStylesCollection(): array
15011507
*/
15021508
public function setConditionalStyles(string $coordinate, array $styles): static
15031509
{
1504-
$this->conditionalStylesCollection[strtoupper($coordinate)] = $styles;
1510+
$this->conditionalStylesCollection[trim(strtoupper($coordinate))] = $styles;
15051511

15061512
return $this;
15071513
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
6+
7+
use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
8+
9+
class Issue4039Test extends \PHPUnit\Framework\TestCase
10+
{
11+
private static string $testbook = 'tests/data/Style/ConditionalFormatting/CellMatcher.xlsx';
12+
13+
public function testSplitRange(): void
14+
{
15+
$reader = new Xlsx();
16+
$spreadsheet = $reader->load(self::$testbook);
17+
$sheet = $spreadsheet->getSheetByNameOrThrow('cellIs Expression');
18+
$expected = [
19+
'A12:D17 A20', // split range
20+
'A22:D27',
21+
'A2:E6',
22+
];
23+
self::assertSame($expected, array_keys($sheet->getConditionalStylesCollection()));
24+
self::assertSame($expected[0], $sheet->getConditionalRange('A20'));
25+
self::assertSame($expected[0], $sheet->getConditionalRange('C15'));
26+
self::assertNull($sheet->getConditionalRange('A19'));
27+
self::assertSame($expected[1], $sheet->getConditionalRange('D25'));
28+
$spreadsheet->disconnectWorksheets();
29+
}
30+
}

0 commit comments

Comments
 (0)