From 011f9cd1a4285e8e951d7ec86a63efbfc4745ab6 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Sat, 18 Mar 2023 11:50:12 -0700 Subject: [PATCH] Xlsx Writer Honor Alignment in Default Font (#3459) * Xlsx Writer Honor Alignment in Default Font Fix #3443. A mysterious implementation by Excel. The style tags have an attribute applyAlignment, which nominally says whether or not the style should use its own Alignment. Except ... Excel ignores that attribute and uses the alignment tag if it is supplied ... and, another mystery, uses not the default style for the spreadsheet if not supplied, but rather uses the default alignment style for all spreadsheets even if the spreadsheet's default style uses a non-default alignment. I am changing Xlsx Writer to generate alignment tag unless the alignment matches both the default alignment for the spreadsheet and the default alignment for all spreadsheets (which I expect to happen most of the time). * Improve Performance Also don't change horizontal/vertical on Xlsx Read if they aren't explicitly set. --- src/PhpSpreadsheet/Reader/Xlsx/Styles.php | 12 ++- src/PhpSpreadsheet/Writer/Xlsx/Style.php | 15 ++- .../Writer/Xlsx/Issue3443Test.php | 97 +++++++++++++++++++ 3 files changed, 117 insertions(+), 7 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Writer/Xlsx/Issue3443Test.php diff --git a/src/PhpSpreadsheet/Reader/Xlsx/Styles.php b/src/PhpSpreadsheet/Reader/Xlsx/Styles.php index 705b319e75..9648058d9b 100644 --- a/src/PhpSpreadsheet/Reader/Xlsx/Styles.php +++ b/src/PhpSpreadsheet/Reader/Xlsx/Styles.php @@ -253,10 +253,14 @@ private function readBorder(Border $border, SimpleXMLElement $borderXml): void public function readAlignmentStyle(Alignment $alignment, SimpleXMLElement $alignmentXml): void { - $horizontal = $this->getAttribute($alignmentXml, 'horizontal'); - $alignment->setHorizontal($horizontal); - $vertical = $this->getAttribute($alignmentXml, 'vertical'); - $alignment->setVertical((string) $vertical); + $horizontal = (string) $this->getAttribute($alignmentXml, 'horizontal'); + if ($horizontal !== '') { + $alignment->setHorizontal($horizontal); + } + $vertical = (string) $this->getAttribute($alignmentXml, 'vertical'); + if ($vertical !== '') { + $alignment->setVertical($vertical); + } $textRotation = (int) $this->getAttribute($alignmentXml, 'textRotation'); if ($textRotation > 90) { diff --git a/src/PhpSpreadsheet/Writer/Xlsx/Style.php b/src/PhpSpreadsheet/Writer/Xlsx/Style.php index 0261f22e56..97d39e5b27 100644 --- a/src/PhpSpreadsheet/Writer/Xlsx/Style.php +++ b/src/PhpSpreadsheet/Writer/Xlsx/Style.php @@ -112,8 +112,13 @@ public function writeStyles(Spreadsheet $spreadsheet) $objWriter->writeAttribute('count', (string) count($spreadsheet->getCellXfCollection())); // xf + $alignment = new Alignment(); + $defaultAlignHash = $alignment->getHashCode(); + if ($defaultAlignHash !== $spreadsheet->getDefaultStyle()->getAlignment()->getHashCode()) { + $defaultAlignHash = ''; + } foreach ($spreadsheet->getCellXfCollection() as $cellXf) { - $this->writeCellStyleXf($objWriter, $cellXf, $spreadsheet); + $this->writeCellStyleXf($objWriter, $cellXf, $spreadsheet, $defaultAlignHash); } $objWriter->endElement(); @@ -400,7 +405,7 @@ private function writeBorder(XMLWriter $objWriter, Borders $borders): void /** * Write Cell Style Xf. */ - private function writeCellStyleXf(XMLWriter $objWriter, \PhpOffice\PhpSpreadsheet\Style\Style $style, Spreadsheet $spreadsheet): void + private function writeCellStyleXf(XMLWriter $objWriter, \PhpOffice\PhpSpreadsheet\Style\Style $style, Spreadsheet $spreadsheet, string $defaultAlignHash): void { // xf $objWriter->startElement('xf'); @@ -424,7 +429,11 @@ private function writeCellStyleXf(XMLWriter $objWriter, \PhpOffice\PhpSpreadshee $objWriter->writeAttribute('applyNumberFormat', ($spreadsheet->getDefaultStyle()->getNumberFormat()->getHashCode() != $style->getNumberFormat()->getHashCode()) ? '1' : '0'); $objWriter->writeAttribute('applyFill', ($spreadsheet->getDefaultStyle()->getFill()->getHashCode() != $style->getFill()->getHashCode()) ? '1' : '0'); $objWriter->writeAttribute('applyBorder', ($spreadsheet->getDefaultStyle()->getBorders()->getHashCode() != $style->getBorders()->getHashCode()) ? '1' : '0'); - $applyAlignment = ($spreadsheet->getDefaultStyle()->getAlignment()->getHashCode() != $style->getAlignment()->getHashCode()) ? '1' : '0'; + if ($defaultAlignHash !== '' && $defaultAlignHash === $style->getAlignment()->getHashCode()) { + $applyAlignment = '0'; + } else { + $applyAlignment = '1'; + } $objWriter->writeAttribute('applyAlignment', $applyAlignment); if ($style->getProtection()->getLocked() != Protection::PROTECTION_INHERIT || $style->getProtection()->getHidden() != Protection::PROTECTION_INHERIT) { $objWriter->writeAttribute('applyProtection', 'true'); diff --git a/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue3443Test.php b/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue3443Test.php new file mode 100644 index 0000000000..a54cb46ce5 --- /dev/null +++ b/tests/PhpSpreadsheetTests/Writer/Xlsx/Issue3443Test.php @@ -0,0 +1,97 @@ + [ + 'horizontal' => Alignment::HORIZONTAL_CENTER, + 'vertical' => Alignment::VERTICAL_CENTER, + 'wrapText' => true, + ], + 'font' => [ + 'name' => 'Courier New', + ], + ]; + $spreadsheet->getDefaultStyle()->applyFromArray($styleArray); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->getCell('A1')->setValue(1); + $sheet->getCell('A2')->setValue(2); + $sheet->getCell('A3')->setValue(3); + $sheet->getStyle('A2')->getAlignment()->setHorizontal(Alignment::HORIZONTAL_LEFT); + $sheet->getStyle('A3')->getFont()->setName('Tahoma'); + + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx'); + $spreadsheet->disconnectWorksheets(); + $rsheet = $reloadedSpreadsheet->getActiveSheet(); + $expected1 = [ + 'horizontal' => 'center', + 'indent' => 0, + 'readOrder' => 0, + 'shrinkToFit' => false, + 'textRotation' => 0, + 'vertical' => 'center', + 'wrapText' => true, + ]; + self::assertSame($expected1, $rsheet->getStyle('A1')->getAlignment()->exportArray()); + $expected2 = $expected1; + $expected2['horizontal'] = 'left'; + self::assertSame($expected2, $rsheet->getStyle('A2')->getAlignment()->exportArray()); + self::assertSame($expected1, $rsheet->getStyle('A3')->getAlignment()->exportArray()); + self::assertSame('Courier New', $rsheet->getStyle('A1')->getFont()->getName()); + self::assertSame('Courier New', $rsheet->getStyle('A2')->getFont()->getName()); + self::assertSame('Tahoma', $rsheet->getStyle('A3')->getFont()->getName()); + + $reloadedSpreadsheet->disconnectWorksheets(); + } + + public function testDefaultAlign(): void + { + // Issue 3443 - default alignment not honored. + $spreadsheet = new Spreadsheet(); + $styleArray = [ + 'font' => [ + 'name' => 'Courier New', + ], + ]; + $spreadsheet->getDefaultStyle()->applyFromArray($styleArray); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->getCell('A1')->setValue(1); + $sheet->getCell('A2')->setValue(2); + $sheet->getCell('A3')->setValue(3); + $sheet->getStyle('A2')->getAlignment()->setHorizontal(Alignment::HORIZONTAL_LEFT); + $sheet->getStyle('A3')->getFont()->setName('Tahoma'); + + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Xlsx'); + $spreadsheet->disconnectWorksheets(); + $rsheet = $reloadedSpreadsheet->getActiveSheet(); + $expected1 = [ + 'horizontal' => 'general', + 'indent' => 0, + 'readOrder' => 0, + 'shrinkToFit' => false, + 'textRotation' => 0, + 'vertical' => 'bottom', + 'wrapText' => false, + ]; + self::assertSame($expected1, $rsheet->getStyle('A1')->getAlignment()->exportArray()); + $expected2 = $expected1; + $expected2['horizontal'] = 'left'; + self::assertSame($expected2, $rsheet->getStyle('A2')->getAlignment()->exportArray()); + self::assertSame($expected1, $rsheet->getStyle('A3')->getAlignment()->exportArray()); + self::assertSame('Courier New', $rsheet->getStyle('A1')->getFont()->getName()); + self::assertSame('Courier New', $rsheet->getStyle('A2')->getFont()->getName()); + self::assertSame('Tahoma', $rsheet->getStyle('A3')->getFont()->getName()); + + $reloadedSpreadsheet->disconnectWorksheets(); + } +}