From 5060505e83b222b8042f436541034eb0ad1a388b Mon Sep 17 00:00:00 2001 From: Iliy Date: Mon, 17 Feb 2020 08:55:10 +1000 Subject: [PATCH 01/10] Added the ability to set the css property font-display: swap, through the url parameter. --- README.md | 5 +++++ src/Controllers/CSSGeneratorController.php | 3 ++- .../WebfontCSSGenerator/WebfontCSSGenerator.php | 14 +++++++++----- .../WebfontCSSGeneratorTest.php | 7 +++++-- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index ff48ebb..1e97181 100644 --- a/README.md +++ b/README.md @@ -153,6 +153,11 @@ body { } ``` +You can set use reserve fonts before custom fonts will full download. For this, set 'display' parameter to 'swap' into url. As example: + +```html + +``` ## Versions compatibility diff --git a/src/Controllers/CSSGeneratorController.php b/src/Controllers/CSSGeneratorController.php index a1ae500..87f6399 100644 --- a/src/Controllers/CSSGeneratorController.php +++ b/src/Controllers/CSSGeneratorController.php @@ -60,7 +60,8 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res return $this->createErrorResponse('The app settings are invalid: '.$error->getMessage(), 500); } try { - $cssCode = $webfontCSSGenerator->makeCSS($requestedFonts); + $displaySwap = isset($requestParams['display']) ? $requestParams['display'] === 'swap' : false; + $cssCode = $webfontCSSGenerator->makeCSS($requestedFonts, $displaySwap); } catch (\InvalidArgumentException $error) { return $this->createErrorResponse($error->getMessage()); } diff --git a/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php b/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php index 9a4adbb..6bd0e52 100644 --- 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'] * ] * + * @param bool $displaySwap * @return string * @throws \InvalidArgumentException */ - public function makeCSS(array $requestedFamilies): string + public function makeCSS(array $requestedFamilies, bool $displaySwap): string { $cssCode = ''; foreach ($requestedFamilies as $fontName => $styles) { - $cssCode .= $this->makeFontFamilyCSS($fontName, $styles); + $cssCode .= $this->makeFontFamilyCSS($fontName, $styles, $displaySwap); } 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 bool $displaySwap Enable font-display css properties to `swap`. Default value is `false` * @return string * @throws \InvalidArgumentException */ - protected function makeFontFamilyCSS(string $name, array $styles = ['400']): string + protected function makeFontFamilyCSS(string $name, array $styles = ['400'], bool $displaySwap = false): 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, $displaySwap); 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 bool $displaySwap Enable font-display css properties to `swap` * @return string * @throws \InvalidArgumentException */ - protected function makeFontStyleCSS(string $familyName, string $styleName): string + protected function makeFontStyleCSS(string $familyName, string $styleName, bool $displaySwap): string { // Does the given family exist? $family = $this->getFontFamily($familyName); @@ -204,6 +207,7 @@ protected function makeFontStyleCSS(string $familyName, string $styleName): stri . "\tfont-style: ".($style->isItalic ? 'italic' : 'normal').";\n" . (isset($files['eot']) ? "\tsrc: url(".CSSHelpers::formatString($files['eot']).");\n" : '') . "\tsrc: ".implode(', ', $sources).";\n" + . ($displaySwap ? "\tfont-display: swap;\n" : "") . "}"; } diff --git a/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php b/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php index 1d44598..f9bc14b 100644 --- a/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php +++ b/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php @@ -118,7 +118,7 @@ public function testMakeCSS() ", $generator->makeCSS([ 'Open Sans' => ['400', '400i', '700', '700i', '100', '200', '300', '800'], 'Roboto' => ['400', '400i', '700', '700i'] - ])); + ], false)); $this->assertCSSEquals(" @font-face { @@ -127,6 +127,7 @@ public function testMakeCSS() font-style: italic; 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-display: swap; } @font-face { font-family: 'Open Sans'; @@ -134,16 +135,18 @@ public function testMakeCSS() font-style: normal; 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'); + font-display: swap; } @font-face { font-family: 'Roboto'; font-weight: 400; font-style: normal; src: local('Roboto Regular'), local('Roboto-Regular'), url('/generator/fonts/Roboto/roboto.woff2') format('woff2'); + font-display: swap; } ", $generator->makeCSS([ 'Open Sans' => ['400i', '500'], 'Roboto' => ['400', '400', '900'] // Regular style two times - ])); + ], true)); } } From 484d74957d57af5ebb38e83222d349ef01a76159 Mon Sep 17 00:00:00 2001 From: Iliy Date: Sat, 22 Feb 2020 16:12:07 +1000 Subject: [PATCH 02/10] Add css property font-display on fonts generator --- src/Controllers/CSSGeneratorController.php | 34 +++++++++++++++++-- .../WebfontCSSGenerator.php | 18 +++++----- .../WebfontCSSGeneratorTest.php | 29 +++++++++++++--- 3 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/Controllers/CSSGeneratorController.php b/src/Controllers/CSSGeneratorController.php index 87f6399..08d93a1 100644 --- a/src/Controllers/CSSGeneratorController.php +++ b/src/Controllers/CSSGeneratorController.php @@ -59,9 +59,13 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res $this->container->get('logger')->error($error); return $this->createErrorResponse('The app settings are invalid: '.$error->getMessage(), 500); } + try { - $displaySwap = isset($requestParams['display']) ? $requestParams['display'] === 'swap' : false; - $cssCode = $webfontCSSGenerator->makeCSS($requestedFonts, $displaySwap); + $fontDisplay = null; + if (isset($requestParams['display'])) { + $fontDisplay = $this->checkFontDisplay($requestParams['display']); + } + $cssCode = $webfontCSSGenerator->makeCSS($requestedFonts, $fontDisplay); } catch (\InvalidArgumentException $error) { return $this->createErrorResponse($error->getMessage()); } @@ -115,6 +119,32 @@ protected function parseFamilyParameter(string $value): array return $result; } + /** + * Check font-display value. Value must be one of: auto, block, swap, fallback, optional. + * + * @param string|null $fontDisplay Font-display value or null + * @return string|null Valid font-display css value, or null, 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 checkFontDisplay(string $fontDisplay): string + { + if (!isset($fontDisplay) || $fontDisplay === '') { + return null; + } + $validFontDisplayValues = [ + 'auto', + 'block', + 'swap', + 'fallback', + 'optional' + ]; + if (!array_search($fontDisplay, $validFontDisplayValues)) { + 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 index 6bd0e52..67cdb1f 100644 --- a/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php +++ b/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php @@ -90,16 +90,16 @@ public static function createFromSettings(array $settings, string $rootURL = '') * 'Roboto' => ['100', '100i', '400', '400i'] * ] * - * @param bool $displaySwap + * @param string|null $fontDisplay Font-display css property * @return string * @throws \InvalidArgumentException */ - public function makeCSS(array $requestedFamilies, bool $displaySwap): string + public function makeCSS(array $requestedFamilies, string $fontDisplay = null): string { $cssCode = ''; foreach ($requestedFamilies as $fontName => $styles) { - $cssCode .= $this->makeFontFamilyCSS($fontName, $styles, $displaySwap); + $cssCode .= $this->makeFontFamilyCSS($fontName, $styles, $fontDisplay); } return $cssCode; @@ -121,11 +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 bool $displaySwap Enable font-display css properties to `swap`. Default value is `false` + * @param string|null $fontDisplay Font-display css property value * @return string * @throws \InvalidArgumentException */ - protected function makeFontFamilyCSS(string $name, array $styles = ['400'], bool $displaySwap = false): string + protected function makeFontFamilyCSS(string $name, array $styles = ['400'], string $fontDisplay = null): string { $cssCode = ''; $readyStyles = []; @@ -135,7 +135,7 @@ protected function makeFontFamilyCSS(string $name, array $styles = ['400'], bool continue; } - $styleCssCode = $this->makeFontStyleCSS($name, $style, $displaySwap); + $styleCssCode = $this->makeFontStyleCSS($name, $style, $fontDisplay); if ($styleCssCode !== '') { $cssCode .= $styleCssCode."\n"; } @@ -151,11 +151,11 @@ protected function makeFontFamilyCSS(string $name, array $styles = ['400'], bool * * @param string $familyName Font family name * @param string $styleName Font style. The styles must have format `[0-9]+i?`. - * @param bool $displaySwap Enable font-display css properties to `swap` + * @param string $fontDisplay Font-display css property value * @return string * @throws \InvalidArgumentException */ - protected function makeFontStyleCSS(string $familyName, string $styleName, bool $displaySwap): string + protected function makeFontStyleCSS(string $familyName, string $styleName, string $fontDisplay = null): string { // Does the given family exist? $family = $this->getFontFamily($familyName); @@ -207,7 +207,7 @@ protected function makeFontStyleCSS(string $familyName, string $styleName, bool . "\tfont-style: ".($style->isItalic ? 'italic' : 'normal').";\n" . (isset($files['eot']) ? "\tsrc: url(".CSSHelpers::formatString($files['eot']).");\n" : '') . "\tsrc: ".implode(', ', $sources).";\n" - . ($displaySwap ? "\tfont-display: swap;\n" : "") + . (isset($fontDisplay) && $fontDisplay !== '' ? "\tfont-display: " . $fontDisplay . ";\n" : "") . "}"; } diff --git a/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php b/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php index f9bc14b..37e0ccd 100644 --- a/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php +++ b/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php @@ -118,7 +118,7 @@ public function testMakeCSS() ", $generator->makeCSS([ 'Open Sans' => ['400', '400i', '700', '700i', '100', '200', '300', '800'], 'Roboto' => ['400', '400i', '700', '700i'] - ], false)); + ])); $this->assertCSSEquals(" @font-face { @@ -127,7 +127,6 @@ public function testMakeCSS() font-style: italic; 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-display: swap; } @font-face { font-family: 'Open Sans'; @@ -135,18 +134,38 @@ public function testMakeCSS() font-style: normal; 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'); - font-display: swap; } @font-face { font-family: 'Roboto'; font-weight: 400; font-style: normal; src: local('Roboto Regular'), local('Roboto-Regular'), url('/generator/fonts/Roboto/roboto.woff2') format('woff2'); - font-display: swap; } ", $generator->makeCSS([ 'Open Sans' => ['400i', '500'], 'Roboto' => ['400', '400', '900'] // Regular style two times - ], true)); + ])); + + + $this->assertCSSEquals(" +@font-face { + font-family: 'Open Sans'; + font-weight: 400; + font-style: italic; + 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-display: swap; +} +@font-face { + font-family: 'Open Sans'; + font-weight: 500; + font-style: normal; + 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'); + font-display: swap; +} + ", $generator->makeCSS([ + 'Open Sans' => ['400i', '500'] + ], 'swap')); } } From 445b514ec3a6332ca8ba0343d2d7a00301d5a8eb Mon Sep 17 00:00:00 2001 From: Iliy Date: Sat, 22 Feb 2020 16:20:57 +1000 Subject: [PATCH 03/10] Update README.md --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 1e97181..6204888 100644 --- a/README.md +++ b/README.md @@ -153,7 +153,8 @@ body { } ``` -You can set use reserve fonts before custom fonts will full download. For this, set 'display' parameter to 'swap' into url. As example: +You can specify a value for the font-display property. To do this, add the display parameter to the request and set its value. +The value can be one of: auto, block, swap, fallback, optional. ```html From 194b9e9a6d1642a947634b95a8de65aac12dad71 Mon Sep 17 00:00:00 2001 From: Surgie Finesse Date: Thu, 27 Feb 2020 21:05:27 +1000 Subject: [PATCH 04/10] Update codeclimate-action to fix PR file report --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f96fefc..ef72ac4 100644 --- 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 From 60807fc904bd6c307d2d2aa20d98cf76f73dd52d Mon Sep 17 00:00:00 2001 From: Iliy Date: Fri, 28 Feb 2020 17:20:27 +1000 Subject: [PATCH 05/10] Bug fixes --- src/Controllers/CSSGeneratorController.php | 16 ++++------------ .../WebfontCSSGenerator/WebfontCSSGenerator.php | 7 ++++++- tests/Functional/CssGeneratorTest.php | 5 ++++- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Controllers/CSSGeneratorController.php b/src/Controllers/CSSGeneratorController.php index 08d93a1..470ed65 100644 --- a/src/Controllers/CSSGeneratorController.php +++ b/src/Controllers/CSSGeneratorController.php @@ -45,8 +45,11 @@ 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 = isset($requestParams['display']) ? $this->checkFontDisplay($requestParams['display']) : null; } catch (\InvalidArgumentException $error) { return $this->createErrorResponse($error->getMessage()); } @@ -61,10 +64,6 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res } try { - $fontDisplay = null; - if (isset($requestParams['display'])) { - $fontDisplay = $this->checkFontDisplay($requestParams['display']); - } $cssCode = $webfontCSSGenerator->makeCSS($requestedFonts, $fontDisplay); } catch (\InvalidArgumentException $error) { return $this->createErrorResponse($error->getMessage()); @@ -132,14 +131,7 @@ protected function checkFontDisplay(string $fontDisplay): string if (!isset($fontDisplay) || $fontDisplay === '') { return null; } - $validFontDisplayValues = [ - 'auto', - 'block', - 'swap', - 'fallback', - 'optional' - ]; - if (!array_search($fontDisplay, $validFontDisplayValues)) { + if (!is_string($fontDisplay)) { throw new \InvalidArgumentException('Invalid font display value'); } return $fontDisplay; diff --git a/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php b/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php index 67cdb1f..399a5d3 100644 --- a/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php +++ b/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php @@ -201,13 +201,18 @@ protected function makeFontStyleCSS(string $familyName, string $styleName, strin $sources[] = "url(".CSSHelpers::formatString($files['svg'].'#webfontregular').") format('svg')"; } + $fontDisplayCSS = ''; + if (isset($fontDisplay) && $fontDisplay !== '') { + $fontDisplayCSS = "\tfont-display: " . $fontDisplay . ";\n"; + } + return "@font-face {\n" . "\tfont-family: ".CSSHelpers::formatString($family->name).";\n" . "\tfont-weight: $style->weight;\n" . "\tfont-style: ".($style->isItalic ? 'italic' : 'normal').";\n" . (isset($files['eot']) ? "\tsrc: url(".CSSHelpers::formatString($files['eot']).");\n" : '') . "\tsrc: ".implode(', ', $sources).";\n" - . (isset($fontDisplay) && $fontDisplay !== '' ? "\tfont-display: " . $fontDisplay . ";\n" : "") + . $fontDisplayCSS . "}"; } diff --git a/tests/Functional/CssGeneratorTest.php b/tests/Functional/CssGeneratorTest.php index c832ad3..efd1960 100644 --- a/tests/Functional/CssGeneratorTest.php +++ b/tests/Functional/CssGeneratorTest.php @@ -37,11 +37,14 @@ public function testBadRequests() $container = $app->getContainer(); $container['webfontCSSGenerator'] = function () { $generator = \Mockery::mock(WebfontCSSGenerator::class); - $generator->shouldReceive('makeCSS')->once()->andThrow(new \InvalidArgumentException('test')); + $generator->shouldReceive('makeCSS')->times(4)->andThrow(new \InvalidArgumentException('test')); 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:400,700', ['display' => 1])->getStatusCode()); + $this->assertEquals(422, $this->runSpecificApp($app, 'GET', '/css?family=Open+Sans:400,700', ['display' => []])->getStatusCode()); + $this->assertEquals(422, $this->runSpecificApp($app, 'GET', '/css?family=Open+Sans:400,700', ['display' => false])->getStatusCode()); } /** From 173d26e5dd4b9091afe22ed6b71f36638d488b1d Mon Sep 17 00:00:00 2001 From: Iliy Date: Fri, 28 Feb 2020 17:27:15 +1000 Subject: [PATCH 06/10] Remove invalid tests for invalid display param --- tests/Functional/CssGeneratorTest.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/Functional/CssGeneratorTest.php b/tests/Functional/CssGeneratorTest.php index efd1960..c832ad3 100644 --- a/tests/Functional/CssGeneratorTest.php +++ b/tests/Functional/CssGeneratorTest.php @@ -37,14 +37,11 @@ public function testBadRequests() $container = $app->getContainer(); $container['webfontCSSGenerator'] = function () { $generator = \Mockery::mock(WebfontCSSGenerator::class); - $generator->shouldReceive('makeCSS')->times(4)->andThrow(new \InvalidArgumentException('test')); + $generator->shouldReceive('makeCSS')->once()->andThrow(new \InvalidArgumentException('test')); 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:400,700', ['display' => 1])->getStatusCode()); - $this->assertEquals(422, $this->runSpecificApp($app, 'GET', '/css?family=Open+Sans:400,700', ['display' => []])->getStatusCode()); - $this->assertEquals(422, $this->runSpecificApp($app, 'GET', '/css?family=Open+Sans:400,700', ['display' => false])->getStatusCode()); } /** From 986e47b6ea38c66bb2bb6dbf4c75edcdd02dd3c7 Mon Sep 17 00:00:00 2001 From: Iliy Date: Fri, 28 Feb 2020 17:42:57 +1000 Subject: [PATCH 07/10] Create test for invalid display param --- src/Controllers/CSSGeneratorController.php | 4 ++-- tests/Functional/CssGeneratorTest.php | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Controllers/CSSGeneratorController.php b/src/Controllers/CSSGeneratorController.php index 470ed65..be40f07 100644 --- a/src/Controllers/CSSGeneratorController.php +++ b/src/Controllers/CSSGeneratorController.php @@ -121,12 +121,12 @@ protected function parseFamilyParameter(string $value): array /** * Check font-display value. Value must be one of: auto, block, swap, fallback, optional. * - * @param string|null $fontDisplay Font-display value or null + * @param $fontDisplay Font-display value or null * @return string|null Valid font-display css value, or null, 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 checkFontDisplay(string $fontDisplay): string + protected function checkFontDisplay($fontDisplay): string { if (!isset($fontDisplay) || $fontDisplay === '') { return null; diff --git a/tests/Functional/CssGeneratorTest.php b/tests/Functional/CssGeneratorTest.php index c832ad3..0c2c9e8 100644 --- a/tests/Functional/CssGeneratorTest.php +++ b/tests/Functional/CssGeneratorTest.php @@ -32,6 +32,7 @@ public function testBadRequests() $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()); + $this->assertEquals(422, $this->runApp('GET', '/css?family=Open+Sans:400,700&display[]=bad')->getStatusCode()); $app = $this->makeApp(); $container = $app->getContainer(); From 90eb962c4062646929df5d58aae5ac63597d6bee Mon Sep 17 00:00:00 2001 From: Iliy Date: Sun, 1 Mar 2020 19:47:00 +1000 Subject: [PATCH 08/10] Fix bugs --- src/Controllers/CSSGeneratorController.php | 6 +++--- src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Controllers/CSSGeneratorController.php b/src/Controllers/CSSGeneratorController.php index be40f07..3a1dc4b 100644 --- a/src/Controllers/CSSGeneratorController.php +++ b/src/Controllers/CSSGeneratorController.php @@ -49,7 +49,7 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res try { $requestedFonts = $this->parseFamilyParameter($requestParams['family']); - $fontDisplay = isset($requestParams['display']) ? $this->checkFontDisplay($requestParams['display']) : null; + $fontDisplay = isset($requestParams['display']) ? $this->checkFontDisplay($requestParams['display']) : ''; } catch (\InvalidArgumentException $error) { return $this->createErrorResponse($error->getMessage()); } @@ -122,14 +122,14 @@ protected function parseFamilyParameter(string $value): array * Check font-display value. Value must be one of: auto, block, swap, fallback, optional. * * @param $fontDisplay Font-display value or null - * @return string|null Valid font-display css value, or null, if $fontDisplay is null or empty. + * @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 checkFontDisplay($fontDisplay): string { if (!isset($fontDisplay) || $fontDisplay === '') { - return null; + return ''; } if (!is_string($fontDisplay)) { throw new \InvalidArgumentException('Invalid font display value'); diff --git a/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php b/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php index 399a5d3..38a0dba 100644 --- a/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php +++ b/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php @@ -94,7 +94,7 @@ public static function createFromSettings(array $settings, string $rootURL = '') * @return string * @throws \InvalidArgumentException */ - public function makeCSS(array $requestedFamilies, string $fontDisplay = null): string + public function makeCSS(array $requestedFamilies, string $fontDisplay = ''): string { $cssCode = ''; @@ -121,11 +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|null $fontDisplay Font-display css property value + * @param string $fontDisplay Font-display css property value * @return string * @throws \InvalidArgumentException */ - protected function makeFontFamilyCSS(string $name, array $styles = ['400'], string $fontDisplay = null): string + protected function makeFontFamilyCSS(string $name, array $styles = ['400'], string $fontDisplay = ''): string { $cssCode = ''; $readyStyles = []; @@ -155,7 +155,7 @@ protected function makeFontFamilyCSS(string $name, array $styles = ['400'], stri * @return string * @throws \InvalidArgumentException */ - protected function makeFontStyleCSS(string $familyName, string $styleName, string $fontDisplay = null): string + protected function makeFontStyleCSS(string $familyName, string $styleName, string $fontDisplay = ''): string { // Does the given family exist? $family = $this->getFontFamily($familyName); From 6c3fda6b6275523e428f87873c04214eb3f6a502 Mon Sep 17 00:00:00 2001 From: Surgie Finesse Date: Sun, 1 Mar 2020 20:00:10 +1000 Subject: [PATCH 09/10] Amendments --- LICENSE | 2 +- README.md | 14 +++++++------- src/Controllers/CSSGeneratorController.php | 12 +++++------- .../WebfontCSSGenerator/WebfontCSSGenerator.php | 9 ++------- .../WebfontCSSGeneratorTest.php | 5 ++--- 5 files changed, 17 insertions(+), 25 deletions(-) diff --git a/LICENSE b/LICENSE index 8f4ea21..fe4554a 100644 --- 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 index 6204888..82de100 100644 --- 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. ``` +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 + +``` + Then embed a font in a CSS code: ```css @@ -153,13 +160,6 @@ body { } ``` -You can specify a value for the font-display property. To do this, add the display parameter to the request and set its value. -The value can be one of: auto, block, swap, fallback, optional. - -```html - -``` - ## Versions compatibility The project follows the [Semantic Versioning](http://semver.org). diff --git a/src/Controllers/CSSGeneratorController.php b/src/Controllers/CSSGeneratorController.php index 3a1dc4b..883e1a7 100644 --- a/src/Controllers/CSSGeneratorController.php +++ b/src/Controllers/CSSGeneratorController.php @@ -46,10 +46,9 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res return $this->createErrorResponse('The `family` query parameter is not set'); } - try { $requestedFonts = $this->parseFamilyParameter($requestParams['family']); - $fontDisplay = isset($requestParams['display']) ? $this->checkFontDisplay($requestParams['display']) : ''; + $fontDisplay = $this->parseDisplayParameter($requestParams['display'] ?? null); } catch (\InvalidArgumentException $error) { return $this->createErrorResponse($error->getMessage()); } @@ -62,7 +61,6 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res $this->container->get('logger')->error($error); return $this->createErrorResponse('The app settings are invalid: '.$error->getMessage(), 500); } - try { $cssCode = $webfontCSSGenerator->makeCSS($requestedFonts, $fontDisplay); } catch (\InvalidArgumentException $error) { @@ -119,16 +117,16 @@ protected function parseFamilyParameter(string $value): array } /** - * Check font-display value. Value must be one of: auto, block, swap, fallback, optional. + * Checks `display` request query value. * - * @param $fontDisplay Font-display value or null + * @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 checkFontDisplay($fontDisplay): string + protected function parseDisplayParameter($fontDisplay): string { - if (!isset($fontDisplay) || $fontDisplay === '') { + if ($fontDisplay === null) { return ''; } if (!is_string($fontDisplay)) { diff --git a/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php b/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php index 38a0dba..53767e4 100644 --- a/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php +++ b/src/Services/WebfontCSSGenerator/WebfontCSSGenerator.php @@ -90,7 +90,7 @@ public static function createFromSettings(array $settings, string $rootURL = '') * 'Roboto' => ['100', '100i', '400', '400i'] * ] * - * @param string|null $fontDisplay Font-display css property + * @param string $fontDisplay Font-display css property * @return string * @throws \InvalidArgumentException */ @@ -201,18 +201,13 @@ protected function makeFontStyleCSS(string $familyName, string $styleName, strin $sources[] = "url(".CSSHelpers::formatString($files['svg'].'#webfontregular').") format('svg')"; } - $fontDisplayCSS = ''; - if (isset($fontDisplay) && $fontDisplay !== '') { - $fontDisplayCSS = "\tfont-display: " . $fontDisplay . ";\n"; - } - return "@font-face {\n" . "\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" - . $fontDisplayCSS . "}"; } diff --git a/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php b/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php index 37e0ccd..88bc912 100644 --- a/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php +++ b/tests/Unit/WebfontCSSGenerator/WebfontCSSGeneratorTest.php @@ -146,23 +146,22 @@ public function testMakeCSS() '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-display: swap; } @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'); - font-display: swap; } ", $generator->makeCSS([ 'Open Sans' => ['400i', '500'] From f1e6f1bc204f4efd24ce0355af2e12e3d2581752 Mon Sep 17 00:00:00 2001 From: Surgie Finesse Date: Sun, 1 Mar 2020 20:34:18 +1000 Subject: [PATCH 10/10] Add tests to check GET parameters parsing --- src/Controllers/CSSGeneratorController.php | 6 +- tests/Functional/CssGeneratorTest.php | 74 +++++++++++++++++++--- 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/src/Controllers/CSSGeneratorController.php b/src/Controllers/CSSGeneratorController.php index 883e1a7..41b7e7f 100644 --- a/src/Controllers/CSSGeneratorController.php +++ b/src/Controllers/CSSGeneratorController.php @@ -89,7 +89,7 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res * ] * * @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 { @@ -121,8 +121,8 @@ protected function parseFamilyParameter(string $value): array * * @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. + * @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 { diff --git a/tests/Functional/CssGeneratorTest.php b/tests/Functional/CssGeneratorTest.php index 0c2c9e8..19c1296 100644 --- a/tests/Functional/CssGeneratorTest.php +++ b/tests/Functional/CssGeneratorTest.php @@ -13,27 +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()); - $this->assertEquals(422, $this->runApp('GET', '/css?family=Open+Sans:400,700&display[]=bad')->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 () { @@ -42,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()); } /**