Skip to content

Fix issues with updating Conditional Formatting when inserting/deleting rows/columns #2689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).

### Fixed

- Update Conditional Formatting ranges and rule conditions when inserting/deleting rows/columns [Issue #2678](https://github.com/PHPOffice/PhpSpreadsheet/issues/2678) [PR #2689](https://github.com/PHPOffice/PhpSpreadsheet/pull/2689)
- Allow `INDIRECT()` to accept row/column ranges as well as cell ranges [PR #2687](https://github.com/PHPOffice/PhpSpreadsheet/pull/2687)
- Fix bug when deleting cells with hyperlinks, where the hyperlink was then being "inherited" by whatever cell moved to that cell address.
- Fix bug in Conditional Formatting in the Xls Writer that resulted in a broken file when there were multiple conditional ranges in a worksheet.
Expand Down
5 changes: 5 additions & 0 deletions src/PhpSpreadsheet/CellReferenceHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ public function __construct(string $beforeCellAddress = 'A1', int $numberOfColum
$this->beforeRow = (int) $beforeRow;
}

public function beforeCellAddress(): string
{
return $this->beforeCellAddress;
}

public function refreshRequired(string $beforeCellAddress, int $numberOfColumns, int $numberOfRows): bool
{
return $this->beforeCellAddress !== $beforeCellAddress ||
Expand Down
99 changes: 65 additions & 34 deletions src/PhpSpreadsheet/ReferenceHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
use PhpOffice\PhpSpreadsheet\Cell\DataType;
use PhpOffice\PhpSpreadsheet\Style\Conditional;
use PhpOffice\PhpSpreadsheet\Worksheet\AutoFilter;
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;

Expand Down Expand Up @@ -198,6 +199,45 @@ protected function adjustHyperlinks($worksheet, $numberOfColumns, $numberOfRows)
}
}

/**
* Update conditional formatting styles when inserting/deleting rows/columns.
*
* @param Worksheet $worksheet The worksheet that we're editing
* @param int $numberOfColumns Number of columns to insert/delete (negative values indicate deletion)
* @param int $numberOfRows Number of rows to insert/delete (negative values indicate deletion)
*/
protected function adjustConditionalFormatting($worksheet, $numberOfColumns, $numberOfRows): void
{
$aStyles = $worksheet->getConditionalStylesCollection();
($numberOfColumns > 0 || $numberOfRows > 0)
? uksort($aStyles, [self::class, 'cellReverseSort'])
: uksort($aStyles, [self::class, 'cellSort']);

foreach ($aStyles as $cellAddress => $cfRules) {
$worksheet->removeConditionalStyles($cellAddress);
$newReference = $this->updateCellReference($cellAddress);

foreach ($cfRules as &$cfRule) {
/** @var Conditional $cfRule */
$conditions = $cfRule->getConditions();
foreach ($conditions as &$condition) {
if (is_string($condition)) {
$condition = $this->updateFormulaReferences(
$condition,
$this->cellReferenceHelper->beforeCellAddress(),
$numberOfColumns,
$numberOfRows,
$worksheet->getTitle(),
true
);
}
}
$cfRule->setConditions($conditions);
}
$worksheet->setConditionalStyles($newReference, $cfRules);
}
}

/**
* Update data validations when inserting/deleting rows/columns.
*
Expand Down Expand Up @@ -442,6 +482,9 @@ function ($coordinate) use ($allCoordinates) {
// Update worksheet: hyperlinks
$this->adjustHyperlinks($worksheet, $numberOfColumns, $numberOfRows);

// Update worksheet: conditional formatting styles
$this->adjustConditionalFormatting($worksheet, $numberOfColumns, $numberOfRows);

// Update worksheet: data validations
$this->adjustDataValidations($worksheet, $numberOfColumns, $numberOfRows);

Expand Down Expand Up @@ -505,8 +548,14 @@ function ($coordinate) use ($allCoordinates) {
*
* @return string Updated formula
*/
public function updateFormulaReferences($formula = '', $beforeCellAddress = 'A1', $numberOfColumns = 0, $numberOfRows = 0, $worksheetName = '')
{
public function updateFormulaReferences(
$formula = '',
$beforeCellAddress = 'A1',
$numberOfColumns = 0,
$numberOfRows = 0,
$worksheetName = '',
bool $includeAbsoluteReferences = false
) {
if (
$this->cellReferenceHelper === null ||
$this->cellReferenceHelper->refreshRequired($beforeCellAddress, $numberOfColumns, $numberOfRows)
Expand All @@ -528,8 +577,8 @@ public function updateFormulaReferences($formula = '', $beforeCellAddress = 'A1'
foreach ($matches as $match) {
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3] . ':' . $match[4];
$modified3 = substr($this->updateCellReference('$A' . $match[3]), 2);
$modified4 = substr($this->updateCellReference('$A' . $match[4]), 2);
$modified3 = substr($this->updateCellReference('$A' . $match[3], $includeAbsoluteReferences), 2);
$modified4 = substr($this->updateCellReference('$A' . $match[4], $includeAbsoluteReferences), 2);

if ($match[3] . ':' . $match[4] !== $modified3 . ':' . $modified4) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
Expand All @@ -553,8 +602,8 @@ public function updateFormulaReferences($formula = '', $beforeCellAddress = 'A1'
foreach ($matches as $match) {
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3] . ':' . $match[4];
$modified3 = substr($this->updateCellReference($match[3] . '$1'), 0, -2);
$modified4 = substr($this->updateCellReference($match[4] . '$1'), 0, -2);
$modified3 = substr($this->updateCellReference($match[3] . '$1', $includeAbsoluteReferences), 0, -2);
$modified4 = substr($this->updateCellReference($match[4] . '$1', $includeAbsoluteReferences), 0, -2);

if ($match[3] . ':' . $match[4] !== $modified3 . ':' . $modified4) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
Expand All @@ -578,8 +627,8 @@ public function updateFormulaReferences($formula = '', $beforeCellAddress = 'A1'
foreach ($matches as $match) {
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3] . ':' . $match[4];
$modified3 = $this->updateCellReference($match[3]);
$modified4 = $this->updateCellReference($match[4]);
$modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences);
$modified4 = $this->updateCellReference($match[4], $includeAbsoluteReferences);

if ($match[3] . $match[4] !== $modified3 . $modified4) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
Expand All @@ -606,7 +655,7 @@ public function updateFormulaReferences($formula = '', $beforeCellAddress = 'A1'
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3];

$modified3 = $this->updateCellReference($match[3]);
$modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences);
if ($match[3] !== $modified3) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
$toString = ($match[2] > '') ? $match[2] . '!' : '';
Expand Down Expand Up @@ -786,18 +835,18 @@ private function updateRowRangesAllWorksheets(string $formula, int $numberOfRows
*
* @return string Updated cell range
*/
private function updateCellReference($cellReference = 'A1')
private function updateCellReference($cellReference = 'A1', bool $includeAbsoluteReferences = false)
{
// Is it in another worksheet? Will not have to update anything.
if (strpos($cellReference, '!') !== false) {
return $cellReference;
// Is it a range or a single cell?
} elseif (!Coordinate::coordinateIsRange($cellReference)) {
// Single cell
return $this->cellReferenceHelper->updateCellReference($cellReference);
return $this->cellReferenceHelper->updateCellReference($cellReference, $includeAbsoluteReferences);
} elseif (Coordinate::coordinateIsRange($cellReference)) {
// Range
return $this->updateCellRange($cellReference);
return $this->updateCellRange($cellReference, $includeAbsoluteReferences);
}

// Return original
Expand Down Expand Up @@ -839,7 +888,7 @@ public function updateNamedFormulas(Spreadsheet $spreadsheet, $oldName = '', $ne
*
* @return string Updated cell range
*/
private function updateCellRange(string $cellRange = 'A1:A1'): string
private function updateCellRange(string $cellRange = 'A1:A1', bool $includeAbsoluteReferences = false): string
{
if (!Coordinate::coordinateIsRange($cellRange)) {
throw new Exception('Only cell ranges may be passed to this method.');
Expand All @@ -853,14 +902,14 @@ private function updateCellRange(string $cellRange = 'A1:A1'): string
for ($j = 0; $j < $jc; ++$j) {
if (ctype_alpha($range[$i][$j])) {
$range[$i][$j] = Coordinate::coordinateFromString(
$this->cellReferenceHelper->updateCellReference($range[$i][$j] . '1')
$this->cellReferenceHelper->updateCellReference($range[$i][$j] . '1', $includeAbsoluteReferences)
)[0];
} elseif (ctype_digit($range[$i][$j])) {
$range[$i][$j] = Coordinate::coordinateFromString(
$this->cellReferenceHelper->updateCellReference('A' . $range[$i][$j])
$this->cellReferenceHelper->updateCellReference('A' . $range[$i][$j], $includeAbsoluteReferences)
)[1];
} else {
$range[$i][$j] = $this->cellReferenceHelper->updateCellReference($range[$i][$j]);
$range[$i][$j] = $this->cellReferenceHelper->updateCellReference($range[$i][$j], $includeAbsoluteReferences);
}
}
}
Expand Down Expand Up @@ -985,17 +1034,8 @@ private function duplicateStylesByColumn(Worksheet $worksheet, int $beforeColumn
$coordinate = $beforeColumnName . $i;
if ($worksheet->cellExists($coordinate)) {
$xfIndex = $worksheet->getCell($coordinate)->getXfIndex();
$conditionalStyles = $worksheet->conditionalStylesExists($coordinate) ?
$worksheet->getConditionalStyles($coordinate) : false;
for ($j = $beforeColumn; $j <= $beforeColumn - 1 + $numberOfColumns; ++$j) {
$worksheet->getCellByColumnAndRow($j, $i)->setXfIndex($xfIndex);
if ($conditionalStyles) {
$cloned = [];
foreach ($conditionalStyles as $conditionalStyle) {
$cloned[] = clone $conditionalStyle;
}
$worksheet->setConditionalStyles(Coordinate::stringFromColumnIndex($j) . $i, $cloned);
}
}
}
}
Expand All @@ -1009,17 +1049,8 @@ private function duplicateStylesByRow(Worksheet $worksheet, int $beforeColumn, i
$coordinate = Coordinate::stringFromColumnIndex($i) . ($beforeRow - 1);
if ($worksheet->cellExists($coordinate)) {
$xfIndex = $worksheet->getCell($coordinate)->getXfIndex();
$conditionalStyles = $worksheet->conditionalStylesExists($coordinate) ?
$worksheet->getConditionalStyles($coordinate) : false;
for ($j = $beforeRow; $j <= $beforeRow - 1 + $numberOfRows; ++$j) {
$worksheet->getCell(Coordinate::stringFromColumnIndex($i) . $j)->setXfIndex($xfIndex);
if ($conditionalStyles) {
$cloned = [];
foreach ($conditionalStyles as $conditionalStyle) {
$cloned[] = clone $conditionalStyle;
}
$worksheet->setConditionalStyles(Coordinate::stringFromColumnIndex($i) . $j, $cloned);
}
}
}
}
Expand Down
39 changes: 39 additions & 0 deletions tests/PhpSpreadsheetTests/ReferenceHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use PhpOffice\PhpSpreadsheet\Comment;
use PhpOffice\PhpSpreadsheet\ReferenceHelper;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Style\ConditionalFormatting\Wizard;
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
use PHPUnit\Framework\TestCase;

Expand Down Expand Up @@ -296,4 +297,42 @@ function (Hyperlink $value) {

self::assertSame(['A3' => 'https://phpspreadsheet.readthedocs.io/en/latest/'], $hyperlinks);
}

public function testInsertRowsWithConditionalFormatting(): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->fromArray([[1, 2, 3, 4], [3, 4, 5, 6], [5, 6, 7, 8], [7, 8, 9, 10], [9, 10, 11, 12]], null, 'C3', true);
$sheet->getCell('H5')->setValue(5);

$cellRange = 'C3:F7';
$conditionalStyles = [];
$wizardFactory = new Wizard($cellRange);
/** @var Wizard\CellValue $cellWizard */
$cellWizard = $wizardFactory->newRule(Wizard::CELL_VALUE);

$cellWizard->equals('$H$5', Wizard::VALUE_TYPE_CELL);
$conditionalStyles[] = $cellWizard->getConditional();

$cellWizard->greaterThan('$H$5', Wizard::VALUE_TYPE_CELL);
$conditionalStyles[] = $cellWizard->getConditional();

$cellWizard->lessThan('$H$5', Wizard::VALUE_TYPE_CELL);
$conditionalStyles[] = $cellWizard->getConditional();

$spreadsheet->getActiveSheet()
->getStyle($cellWizard->getCellRange())
->setConditionalStyles($conditionalStyles);
$sheet->insertNewRowBefore(4, 2);

$styles = $sheet->getConditionalStylesCollection();
// verify that the conditional range has been updated
self::assertSame('C3:F9', array_keys($styles)[0]);
// verify that the conditions have been updated
foreach ($styles as $style) {
foreach ($style as $conditions) {
self::assertSame('$H$7', $conditions->getConditions()[0]);
}
}
}
}