Merge pull request #4 from Finesse/font-display-swap

Add a GET parameter to change font-display style
Finesse · Mar 1, 2020 · a9abcc0 · a9abcc0
2 parents 5369f8c + f1e6f1b
commit a9abcc0
Show file tree

Hide file tree

Showing 7 changed files with 126 additions and 19 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -25,7 +25,7 @@ jobs:
           composer run-script post-create-project-cmd
       - name: Test and publish code coverage
         if: matrix.report-coverage
-        uses: paambaati/codeclimate-action@v2.3.0
+        uses: paambaati/codeclimate-action@v2.5.3
         env:
           # Get it on https://codeclimate.com/repos/{repo id}/settings/test_reporter
           CC_TEST_REPORTER_ID: 02c359b52f053b695269f876e5a55d46a7d02525a9a4e19d473c9a0b29f5a499

diff --git a/LICENSE b/LICENSE
@@ -1,4 +1,4 @@
-Copyright (c) 2017-2019 Surgie Finesse
+Copyright (c) 2017-2020 Surgie Finesse
 
 Permission is hereby granted, free of charge, to any person obtaining
 a copy of this software and associated documentation files (the

diff --git a/README.md b/README.md
@@ -145,6 +145,13 @@ You may omit the styles list. In this case the regular style (`400`) is used.
 <link rel="stylesheet" href="http://web-fonts-repository.local/css?family=Open+Sans" />
 ```
 
+You can specify a value for the [font-display](https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face/font-display)
+style property using `display` parameter. Example:
+
+```html
+<link rel="stylesheet" href="http://web-fonts-repository.local/css?family=Open+Sans&display=swap" />
+```
+
 Then embed a font in a CSS code:
 
 ```css
@@ -153,7 +160,6 @@ body {
 }
 ```
 
-
 ## Versions compatibility
 
 The project follows the [Semantic Versioning](http://semver.org).

diff --git a/src/Controllers/CSSGeneratorController.php b/src/Controllers/CSSGeneratorController.php
@@ -45,8 +45,10 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res
         if (!isset($requestParams['family'])) {
             return $this->createErrorResponse('The `family` query parameter is not set');
         }
+
         try {
             $requestedFonts = $this->parseFamilyParameter($requestParams['family']);
+            $fontDisplay = $this->parseDisplayParameter($requestParams['display'] ?? null);
         } catch (\InvalidArgumentException $error) {
             return $this->createErrorResponse($error->getMessage());
         }
@@ -60,7 +62,7 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res
             return $this->createErrorResponse('The app settings are invalid: '.$error->getMessage(), 500);
         }
         try {
-            $cssCode = $webfontCSSGenerator->makeCSS($requestedFonts);
+            $cssCode = $webfontCSSGenerator->makeCSS($requestedFonts, $fontDisplay);
         } catch (\InvalidArgumentException $error) {
             return $this->createErrorResponse($error->getMessage());
         }
@@ -87,7 +89,7 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res
      *  ]
      * </pre>
      * @throws \InvalidArgumentException If the parameter value is formatted badly. The message may be sent back to the
-     *     client.
+     *  client.
      */
     protected function parseFamilyParameter(string $value): array
     {
@@ -114,6 +116,25 @@ protected function parseFamilyParameter(string $value): array
         return $result;
     }
 
+    /**
+     * Checks `display` request query value.
+     *
+     * @param mixed $fontDisplay Parameter value
+     * @return string Valid font-display css value, or empty string, if $fontDisplay is null or empty.
+     * @throws \InvalidArgumentException If the parameter set, but has not valid value. The message may be sent back to
+     *  the client.
+     */
+    protected function parseDisplayParameter($fontDisplay): string
+    {
+        if ($fontDisplay === null) {
+            return '';
+        }
+        if (!is_string($fontDisplay)) {
+            throw new \InvalidArgumentException('Invalid font display value');
+        }
+        return $fontDisplay;
+    }
+
     /**
      * Creates a response with the client side error message.
      *

diff --git a/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php b/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php
@@ -90,15 +90,16 @@ public static function createFromSettings(array $settings, string $rootURL = '')
      *      'Roboto'    => ['100', '100i', '400', '400i']
      *  ]
      * </pre>
+     * @param string $fontDisplay Font-display css property
      * @return string
      * @throws \InvalidArgumentException
      */
-    public function makeCSS(array $requestedFamilies): string
+    public function makeCSS(array $requestedFamilies, string $fontDisplay = ''): string
     {
         $cssCode = '';
 
         foreach ($requestedFamilies as $fontName => $styles) {
-            $cssCode .= $this->makeFontFamilyCSS($fontName, $styles);
+            $cssCode .= $this->makeFontFamilyCSS($fontName, $styles, $fontDisplay);
         }
 
         return $cssCode;
@@ -120,10 +121,11 @@ protected function getFontFamily(string $name)
      *
      * @param string $name Family name
      * @param string[] $styles Font styles. The styles must have format `[0-9]+i?`.
+     * @param string $fontDisplay Font-display css property value
      * @return string
      * @throws \InvalidArgumentException
      */
-    protected function makeFontFamilyCSS(string $name, array $styles = ['400']): string
+    protected function makeFontFamilyCSS(string $name, array $styles = ['400'], string $fontDisplay = ''): string
     {
         $cssCode = '';
         $readyStyles = [];
@@ -133,7 +135,7 @@ protected function makeFontFamilyCSS(string $name, array $styles = ['400']): str
                 continue;
             }
 
-            $styleCssCode = $this->makeFontStyleCSS($name, $style);
+            $styleCssCode = $this->makeFontStyleCSS($name, $style, $fontDisplay);
             if ($styleCssCode !== '') {
                 $cssCode .= $styleCssCode."\n";
             }
@@ -149,10 +151,11 @@ protected function makeFontFamilyCSS(string $name, array $styles = ['400']): str
      *
      * @param string $familyName Font family name
      * @param string $styleName Font style. The styles must have format `[0-9]+i?`.
+     * @param string $fontDisplay Font-display css property value
      * @return string
      * @throws \InvalidArgumentException
      */
-    protected function makeFontStyleCSS(string $familyName, string $styleName): string
+    protected function makeFontStyleCSS(string $familyName, string $styleName, string $fontDisplay = ''): string
     {
         // Does the given family exist?
         $family = $this->getFontFamily($familyName);
@@ -202,6 +205,7 @@ protected function makeFontStyleCSS(string $familyName, string $styleName): stri
             . "\tfont-family: ".CSSHelpers::formatString($family->name).";\n"
             . "\tfont-weight: $style->weight;\n"
             . "\tfont-style: ".($style->isItalic ? 'italic' : 'normal').";\n"
+            . ($fontDisplay !== '' ? "\tfont-display: $fontDisplay;\n" : '')
             . (isset($files['eot']) ? "\tsrc: url(".CSSHelpers::formatString($files['eot']).");\n" : '')
             . "\tsrc: ".implode(', ', $sources).";\n"
             . "}";

diff --git a/tests/Functional/CssGeneratorTest.php b/tests/Functional/CssGeneratorTest.php
@@ -13,26 +13,81 @@ class CssGeneratorTest extends FunctionalTestCase
      */
     public function testHeaders()
     {
-        $response = $this->runApp('GET', '/css?family=Open+Sans:400,700');
+        $response = $this->runApp('GET', '/css?family=Open+Sans');
 
         $this->assertEquals(200, $response->getStatusCode());
         $this->assertTrue((bool)preg_match('~^text/css(;|$)~', $response->getHeaderLine('Content-Type')));
         $this->assertTrue($response->hasHeader('Cache-Control'));
         $this->assertTrue($response->hasHeader('Pragma'));
+    }
+
+    /**
+     * Test the controller `family` parameter parsing
+     */
+    public function testFamilyParsing()
+    {
+        $app = $this->makeApp();
+        $container = $app->getContainer();
+        $container['webfontCSSGenerator'] = function () {
+            $generator = \Mockery::mock(WebfontCSSGenerator::class);
+            $generator->shouldReceive('makeCSS')
+                ->once()
+                ->with(['Roboto' => ['400']], '')
+                ->andReturn('');
+            $generator->shouldReceive('makeCSS')
+                ->once()
+                ->with([
+                    'Open Sans' => ['400'],
+                    'Roboto' => ['400', '400i', '700i'],
+                ], '')
+                ->andReturn('');
+            return $generator;
+        };
+
+        $this->assertEquals(200, $this->runSpecificApp($app, 'GET', '/css?family=Roboto')->getStatusCode());
+        $this->assertEquals(200, $this->runSpecificApp($app, 'GET', '/css?family=Open+Sans|Roboto:400,400i,700i')->getStatusCode());
 
-        $this->assertEquals(200, $this->runApp('GET', '/css?family=Open+Sans')->getStatusCode());
+        $this->assertEquals(422, $this->runSpecificApp($app, 'GET', '/css?family=')->getStatusCode());
+        $this->assertEquals(422, $this->runSpecificApp($app, 'GET', '/css?family=|Open+Sans:400,700')->getStatusCode());
+        $this->assertEquals(422, $this->runSpecificApp($app, 'GET', '/css?family=:400,700')->getStatusCode());
+        $this->assertEquals(422, $this->runSpecificApp($app, 'GET', '/css')->getStatusCode());
     }
 
     /**
-     * Tests that the route returns an client error status with bad requests
+     * Test the controller `display` parameter parsing
      */
-    public function testBadRequests()
+    public function testDisplayParsing()
     {
-        $this->assertEquals(422, $this->runApp('GET', '/css?family=')->getStatusCode());
-        $this->assertEquals(422, $this->runApp('GET', '/css?family=|Open+Sans:400,700')->getStatusCode());
-        $this->assertEquals(422, $this->runApp('GET', '/css?family=:400,700')->getStatusCode());
-        $this->assertEquals(422, $this->runApp('GET', '/css')->getStatusCode());
+        $app = $this->makeApp();
+        $container = $app->getContainer();
+        $container['webfontCSSGenerator'] = function () {
+            $generator = \Mockery::mock(WebfontCSSGenerator::class);
+            $generator->shouldReceive('makeCSS')
+                ->once()
+                ->with(['Open Sans' => ['400']], '')
+                ->andReturn('');
+            $generator->shouldReceive('makeCSS')
+                ->once()
+                ->with(['Open Sans' => ['400']], '')
+                ->andReturn('');
+            $generator->shouldReceive('makeCSS')
+                ->once()
+                ->with(['Open Sans' => ['400']], 'swap')
+                ->andReturn('');
+            return $generator;
+        };
 
+        $this->assertEquals(200, $this->runSpecificApp($app, 'GET', '/css?family=Open+Sans')->getStatusCode());
+        $this->assertEquals(200, $this->runSpecificApp($app, 'GET', '/css?family=Open+Sans&display=')->getStatusCode());
+        $this->assertEquals(200, $this->runSpecificApp($app, 'GET', '/css?family=Open+Sans&display=swap')->getStatusCode());
+        $this->assertEquals(422, $this->runSpecificApp($app, 'GET', '/css?family=Open+Sans&display[]=bad')->getStatusCode());
+    }
+
+    /**
+     * Test handling errors from webfontCSSGenerator
+     */
+    public function testGeneratorInputError()
+    {
         $app = $this->makeApp();
         $container = $app->getContainer();
         $container['webfontCSSGenerator'] = function () {
@@ -41,7 +96,7 @@ public function testBadRequests()
             return $generator;
         };
 
-        $this->assertEquals(422, $this->runSpecificApp($app, 'GET', '/css?family=Open+Sans:400,700')->getStatusCode());
+        $this->assertEquals(422, $this->runSpecificApp($app, 'GET', '/css?family=Open+Sans')->getStatusCode());
     }
 
     /**

diff --git a/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php b/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php
@@ -145,5 +145,26 @@ public function testMakeCSS()
             'Open Sans' => ['400i', '500'],
             'Roboto' => ['400', '400', '900'] // Regular style two times
         ]));
+
+        $this->assertCSSEquals("
+@font-face {
+	font-family: 'Open Sans';
+	font-weight: 400;
+	font-style: italic;
+	font-display: swap;
+	src: url('/generator/fonts/OpenSans/opensans_italic.eot');
+	src: local('Open Sans Italic'), local('OpenSans-Italic'), url('/generator/fonts/OpenSans/opensans_italic.eot?#iefix') format('embedded-opentype'), url('/generator/fonts/OpenSans/opensans_italic.woff2') format('woff2'), url('/generator/fonts/OpenSans/opensans_italic.woff') format('woff'), url('/generator/fonts/OpenSans/opensans_italic.ttf') format('truetype'), url('/generator/fonts/OpenSans/opensans_italic.otf') format('opentype'), url('/generator/fonts/OpenSans/opensans_italic.svg#webfontregular') format('svg');
+}
+@font-face {
+	font-family: 'Open Sans';
+	font-weight: 500;
+	font-style: normal;
+	font-display: swap;
+	src: url('/generator/fonts/OpenSans/opensans_demi.eot');
+	src: local('Open Sans DemiBold'), local('OpenSans-DemiBold'), url('/generator/fonts/OpenSans/opensans_demi.eot?#iefix') format('embedded-opentype'), url('/generator/fonts/OpenSans/opensans_demi.woff2') format('woff2'), url('/generator/fonts/OpenSans/opensans_demi.woff') format('woff'), url('/generator/fonts/OpenSans/opensans_demi.ttf') format('truetype'), url('/generator/fonts/OpenSans/opensans_demi.otf') format('opentype'), url('/generator/fonts/OpenSans/opensans_demi.svg#webfontregular') format('svg');
+}
+        ", $generator->makeCSS([
+            'Open Sans' => ['400i', '500']
+        ], 'swap'));
     }
 }