Stacked Alignment - Use Class Constant Rather than Literal (#1716)

* Stacked Alignment - Use Class Constant Rather than Literal PR #1580 defined constants for "stacked" alignment in cells. Using those constants outside of Style/Alignment was beyond the scope of the original PR, but I said I would get to it. This PR replaces all uses of literal -165, and appropriate uses of literal 255, with the named constants, and adds tests to make sure that the changed code is covered in the test suite.
PHPOffice · Feb 3, 2021 · 2fac9ee · 2fac9ee
1 parent 30717fd
commit 2fac9ee
Show file tree

Hide file tree

Showing 8 changed files with 95 additions and 52 deletions.
diff --git a/samples/Basic/30_Templatebiff5.php b/samples/Basic/30_Templatebiff5.php
@@ -0,0 +1,43 @@
+<?php
+
+use PhpOffice\PhpSpreadsheet\IOFactory;
+use PhpOffice\PhpSpreadsheet\Shared\Date;
+
+require __DIR__ . '/../Header.php';
+
+$helper->log('Load from Xls template');
+$reader = IOFactory::createReader('Xls');
+$spreadsheet = $reader->load(__DIR__ . '/../templates/30templatebiff5.xls');
+
+$helper->log('Add new data to the template');
+$data = [['title' => 'Excel for dummies',
+    'price' => 17.99,
+    'quantity' => 2,
+],
+    ['title' => 'PHP for dummies',
+        'price' => 15.99,
+        'quantity' => 1,
+    ],
+    ['title' => 'Inside OOP',
+        'price' => 12.95,
+        'quantity' => 1,
+    ],
+];
+
+$spreadsheet->getActiveSheet()->setCellValue('D1', Date::PHPToExcel(time()));
+
+$baseRow = 5;
+foreach ($data as $r => $dataRow) {
+    $row = $baseRow + $r;
+    $spreadsheet->getActiveSheet()->insertNewRowBefore($row, 1);
+
+    $spreadsheet->getActiveSheet()->setCellValue('A' . $row, $r + 1)
+        ->setCellValue('B' . $row, $dataRow['title'])
+        ->setCellValue('C' . $row, $dataRow['price'])
+        ->setCellValue('D' . $row, $dataRow['quantity'])
+        ->setCellValue('E' . $row, '=C' . $row . '*D' . $row);
+}
+$spreadsheet->getActiveSheet()->removeRow($baseRow - 1, 1);
+
+// Save
+$helper->write($spreadsheet, __FILE__);
diff --git a/samples/templates/30template.xls b/samples/templates/30template.xls
diff --git a/samples/templates/30templatebiff5.xls b/samples/templates/30templatebiff5.xls
diff --git a/src/PhpSpreadsheet/Reader/Xls.php b/src/PhpSpreadsheet/Reader/Xls.php
@@ -2286,8 +2286,8 @@ private function readXf(): void
                     $rotation = $angle;
                 } elseif ($angle <= 180) {
                     $rotation = 90 - $angle;
-                } elseif ($angle == 255) {
-                    $rotation = -165;
+                } elseif ($angle == Alignment::TEXTROTATION_STACK_EXCEL) {
+                    $rotation = Alignment::TEXTROTATION_STACK_PHPSPREADSHEET;
                 }
                 $objStyle->getAlignment()->setTextRotation($rotation);
 
@@ -2389,7 +2389,7 @@ private function readXf(): void
 
                         break;
                     case 1:
-                        $objStyle->getAlignment()->setTextRotation(-165);
+                        $objStyle->getAlignment()->setTextRotation(Alignment::TEXTROTATION_STACK_PHPSPREADSHEET);
 
                         break;
                     case 2:

diff --git a/src/PhpSpreadsheet/Shared/Font.php b/src/PhpSpreadsheet/Shared/Font.php
@@ -4,6 +4,7 @@
 
 use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
 use PhpOffice\PhpSpreadsheet\RichText\RichText;
+use PhpOffice\PhpSpreadsheet\Style\Alignment;
 
 class Font
 {
@@ -342,7 +343,7 @@ public static function getTextWidthPixelsApprox($columnText, \PhpOffice\PhpSprea
 
         // Calculate approximate rotated column width
         if ($rotation !== 0) {
-            if ($rotation == -165) {
+            if ($rotation == Alignment::TEXTROTATION_STACK_PHPSPREADSHEET) {
                 // stacked text
                 $columnWidth = 4; // approximation
             } else {

diff --git a/src/PhpSpreadsheet/Writer/Xls/Xf.php b/src/PhpSpreadsheet/Writer/Xls/Xf.php
@@ -165,9 +165,9 @@ public function writeXf()
                         self::mapBorderStyle($this->_style->getBorders()->getTop()->getBorderStyle()) ||
                         self::mapBorderStyle($this->_style->getBorders()->getLeft()->getBorderStyle()) ||
                         self::mapBorderStyle($this->_style->getBorders()->getRight()->getBorderStyle())) ? 1 : 0;
-        $atr_pat = (($this->foregroundColor != 0x40) ||
-                        ($this->backgroundColor != 0x41) ||
-                        self::mapFillType($this->_style->getFill()->getFillType())) ? 1 : 0;
+        $atr_pat = ($this->foregroundColor != 0x40) ? 1 : 0;
+        $atr_pat = ($this->backgroundColor != 0x41) ? 1 : $atr_pat;
+        $atr_pat = self::mapFillType($this->_style->getFill()->getFillType()) ? 1 : $atr_pat;
         $atr_prot = self::mapLocked($this->_style->getProtection()->getLocked())
                         | self::mapHidden($this->_style->getProtection()->getHidden());
 
@@ -375,11 +375,7 @@ public function setFontIndex($value): void
      */
     private static function mapBorderStyle($borderStyle)
     {
-        if (isset(self::$mapBorderStyles[$borderStyle])) {
-            return self::$mapBorderStyles[$borderStyle];
-        }
-
-        return 0x00;
+        return self::$mapBorderStyles[$borderStyle] ?? 0;
     }
 
     /**
@@ -420,11 +416,7 @@ private static function mapBorderStyle($borderStyle)
      */
     private static function mapFillType($fillType)
     {
-        if (isset(self::$mapFillTypes[$fillType])) {
-            return self::$mapFillTypes[$fillType];
-        }
-
-        return 0x00;
+        return self::$mapFillTypes[$fillType] ?? 0;
     }
 
     /**
@@ -451,11 +443,7 @@ private static function mapFillType($fillType)
      */
     private function mapHAlign($hAlign)
     {
-        if (isset(self::$mapHAlignments[$hAlign])) {
-            return self::$mapHAlignments[$hAlign];
-        }
-
-        return 0;
+        return self::$mapHAlignments[$hAlign] ?? 0;
     }
 
     /**
@@ -479,11 +467,7 @@ private function mapHAlign($hAlign)
      */
     private static function mapVAlign($vAlign)
     {
-        if (isset(self::$mapVAlignments[$vAlign])) {
-            return self::$mapVAlignments[$vAlign];
-        }
-
-        return 2;
+        return self::$mapVAlignments[$vAlign] ?? 2;
     }
 
     /**
@@ -497,34 +481,38 @@ private static function mapTextRotation($textRotation)
     {
         if ($textRotation >= 0) {
             return $textRotation;
-        } elseif ($textRotation == -165) {
-            return 255;
-        } elseif ($textRotation < 0) {
-            return 90 - $textRotation;
         }
+        if ($textRotation == Alignment::TEXTROTATION_STACK_PHPSPREADSHEET) {
+            return Alignment::TEXTROTATION_STACK_EXCEL;
+        }
+
+        return 90 - $textRotation;
     }
 
+    private const LOCK_ARRAY = [
+        Protection::PROTECTION_INHERIT => 1,
+        Protection::PROTECTION_PROTECTED => 1,
+        Protection::PROTECTION_UNPROTECTED => 0,
+    ];
+
     /**
-     * Map locked.
+     * Map locked values.
      *
      * @param string $locked
      *
      * @return int
      */
     private static function mapLocked($locked)
     {
-        switch ($locked) {
-            case Protection::PROTECTION_INHERIT:
-                return 1;
-            case Protection::PROTECTION_PROTECTED:
-                return 1;
-            case Protection::PROTECTION_UNPROTECTED:
-                return 0;
-            default:
-                return 1;
-        }
+        return array_key_exists($locked, self::LOCK_ARRAY) ? self::LOCK_ARRAY[$locked] : 1;
     }
 
+    private const HIDDEN_ARRAY = [
+        Protection::PROTECTION_INHERIT => 0,
+        Protection::PROTECTION_PROTECTED => 1,
+        Protection::PROTECTION_UNPROTECTED => 0,
+    ];
+
     /**
      * Map hidden.
      *
@@ -534,15 +522,6 @@ private static function mapLocked($locked)
      */
     private static function mapHidden($hidden)
     {
-        switch ($hidden) {
-            case Protection::PROTECTION_INHERIT:
-                return 0;
-            case Protection::PROTECTION_PROTECTED:
-                return 1;
-            case Protection::PROTECTION_UNPROTECTED:
-                return 0;
-            default:
-                return 0;
-        }
+        return array_key_exists($hidden, self::HIDDEN_ARRAY) ? self::HIDDEN_ARRAY[$hidden] : 0;
     }
 }
diff --git a/tests/PhpSpreadsheetTests/Functional/ActiveSheetTest.php b/tests/PhpSpreadsheetTests/Functional/ActiveSheetTest.php
@@ -3,6 +3,7 @@
 namespace PhpOffice\PhpSpreadsheetTests\Functional;
 
 use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Style\Protection;
 
 class ActiveSheetTest extends AbstractFunctional
 {
@@ -46,6 +47,12 @@ public function testActiveSheet($format): void
 
         $spreadsheet->createSheet(2);
 
+        $spreadsheet->setActiveSheetIndex(2)
+            ->setCellValue('F1', 2)
+            ->getCell('F1')
+            ->getStyle()
+            ->getProtection()
+            ->setHidden(Protection::PROTECTION_PROTECTED);
         $spreadsheet->setActiveSheetIndex(2)
             ->setTitle('Test3')
             ->setCellValue('A1', 4)

diff --git a/tests/PhpSpreadsheetTests/Shared/FontTest.php b/tests/PhpSpreadsheetTests/Shared/FontTest.php
@@ -3,6 +3,7 @@
 namespace PhpOffice\PhpSpreadsheetTests\Shared;
 
 use PhpOffice\PhpSpreadsheet\Shared\Font;
+use PhpOffice\PhpSpreadsheet\Style\Font as StyleFont;
 use PHPUnit\Framework\TestCase;
 
 class FontTest extends TestCase
@@ -83,4 +84,16 @@ public function providerCentimeterSizeToPixels()
     {
         return require 'tests/data/Shared/CentimeterSizeToPixels.php';
     }
+
+    public function testVerdanaRotation(): void
+    {
+        $font = new StyleFont();
+        $font->setName('Verdana')->setSize(10);
+        $width = Font::getTextWidthPixelsApprox('n', $font, 0);
+        self::assertEquals(8, $width);
+        $width = Font::getTextWidthPixelsApprox('n', $font, 45);
+        self::assertEquals(7, $width);
+        $width = Font::getTextWidthPixelsApprox('n', $font, -165);
+        self::assertEquals(4, $width);
+    }
 }