From 4a8331409599358d4e7cb728594b4c3ccff60f9b Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Thu, 27 Sep 2018 12:27:07 -0600 Subject: [PATCH] Should fix more legacy issues --- system/src/Grav/Common/Assets.php | 22 +++-- .../Assets/Traits/LegacyAssetsTrait.php | 61 ++++++------- tests/unit/Grav/Common/AssetsTest.php | 86 +++++++++++++++++-- 3 files changed, 125 insertions(+), 44 deletions(-) diff --git a/system/src/Grav/Common/Assets.php b/system/src/Grav/Common/Assets.php index 735a69d1a..040ab7fea 100644 --- a/system/src/Grav/Common/Assets.php +++ b/system/src/Grav/Common/Assets.php @@ -125,15 +125,19 @@ class Assets extends PropertyObject */ public function add($asset) { - $options = $this->unifyLegacyArguments(func_get_args()); + $args = func_get_args(); // More than one asset if (\is_array($asset)) { foreach ($asset as $a) { - $this->add($a, $options); + array_shift($args); + $args = array_merge([$a], $args); + call_user_func_array([$this, 'add'], $args); } } elseif (isset($this->collections[$asset])) { - $this->add($this->collections[$asset], $options); + array_shift($args); + $args = array_merge([$this->collections[$asset]], $args); + call_user_func_array([$this, 'add'], $args); } else { // Get extension $extension = pathinfo(parse_url($asset, PHP_URL_PATH), PATHINFO_EXTENSION); @@ -142,9 +146,9 @@ class Assets extends PropertyObject if (\strlen($extension) > 0) { $extension = strtolower($extension); if ($extension === 'css') { - $this->addCss($asset, $options); + call_user_func_array([$this, 'addCss'], $args); } elseif ($extension === 'js') { - $this->addJs($asset, $options); + call_user_func_array([$this, 'addJs'], $args); } } } @@ -208,7 +212,7 @@ class Assets extends PropertyObject */ public function addCss($asset) { - return $this->addType(Assets::CSS_TYPE,Assets::CSS_TYPE, $asset, $this->unifyLegacyArguments(\func_get_args())); + return $this->addType(Assets::CSS_TYPE,Assets::CSS_TYPE, $asset, $this->unifyLegacyArguments(\func_get_args(), Assets::CSS_TYPE)); } /** @@ -218,7 +222,7 @@ class Assets extends PropertyObject */ public function addInlineCss($asset) { - return $this->addType(Assets::CSS_TYPE, Assets::INLINE_CSS_TYPE, $asset, $this->unifyLegacyArguments(\func_get_args())); + return $this->addType(Assets::CSS_TYPE, Assets::INLINE_CSS_TYPE, $asset, $this->unifyLegacyArguments(\func_get_args(), Assets::INLINE_CSS_TYPE)); } /** @@ -228,7 +232,7 @@ class Assets extends PropertyObject */ public function addJs($asset) { - return $this->addType(Assets::JS_TYPE, Assets::JS_TYPE, $asset, $this->unifyLegacyArguments(\func_get_args()), Assets::JS_TYPE); + return $this->addType(Assets::JS_TYPE, Assets::JS_TYPE, $asset, $this->unifyLegacyArguments(\func_get_args(), Assets::JS_TYPE)); } /** @@ -238,7 +242,7 @@ class Assets extends PropertyObject */ public function addInlineJs($asset) { - return $this->addType(Assets::JS_TYPE, Assets::INLINE_JS_TYPE, $asset, $this->unifyLegacyArguments(\func_get_args()), Assets::JS_TYPE); + return $this->addType(Assets::JS_TYPE, Assets::INLINE_JS_TYPE, $asset, $this->unifyLegacyArguments(\func_get_args(), Assets::INLINE_JS_TYPE)); } diff --git a/system/src/Grav/Common/Assets/Traits/LegacyAssetsTrait.php b/system/src/Grav/Common/Assets/Traits/LegacyAssetsTrait.php index a8e8fa139..9523e9ad1 100644 --- a/system/src/Grav/Common/Assets/Traits/LegacyAssetsTrait.php +++ b/system/src/Grav/Common/Assets/Traits/LegacyAssetsTrait.php @@ -15,8 +15,6 @@ trait LegacyAssetsTrait protected function unifyLegacyArguments($args, $type = Assets::CSS_TYPE) { - $arguments = []; - // First argument is always the asset array_shift($args); @@ -27,31 +25,34 @@ trait LegacyAssetsTrait return $args[0]; } - // $asset, $priority = null, $pipeline = true, $group = null, $loading = null + switch ($type) { + case(Assets::INLINE_CSS_TYPE): + $keys = ['priority', 'group']; + $arguments = array_combine(array_slice($keys, 0, count($args)), $args); + break; - foreach ($args as $index => $arg) { - switch ($index) { - case 0: - if (isset($args[0])) { $arguments['priority'] = $args[0]; } - break; - case 1: - if (isset($args[1])) { $arguments['pipeline'] = $args[1]; } - break; - case 2: - if ($type === Assets::CSS_TYPE) { - if (isset($args[2])) { $arguments['group'] = $args[2]; } - } else { - if (isset($args[2])) { $arguments['loading'] = $args[2]; } - } - break; - case 3: - if ($type === Assets::CSS_TYPE) { - if (isset($args[3])) { $arguments['loading'] = $args[3]; } - } else { - if (isset($args[3])) { $arguments['group'] = $args[3]; } - } - break; - } + case(Assets::JS_TYPE): + $keys = ['priority', 'pipeline', 'loading', 'group']; + $arguments = array_combine(array_slice($keys, 0, count($args)), $args); + break; + + case(Assets::INLINE_JS_TYPE): + $keys = ['priority', 'group', 'attributes']; + $arguments = array_combine(array_slice($keys, 0, count($args)), $args); + + // special case to handle old attributes being passed in + if (isset($arguments['attributes'])) { + $old_attributes = $arguments['attributes']; + $arguments = array_merge($arguments, $old_attributes); + } + unset($arguments['attributes']); + + break; + + default: + case(Assets::CSS_TYPE): + $keys = ['priority', 'pipeline', 'group', 'loading']; + $arguments = array_combine(array_slice($keys, 0, count($args)), $args); } return $arguments; @@ -69,9 +70,9 @@ trait LegacyAssetsTrait * * @return \Grav\Common\Assets */ - public function addAsyncJs($asset, $priority = null, $pipeline = true, $group = null) + public function addAsyncJs($asset, $priority = 10, $pipeline = true, $group = 'head') { - return $this->addJs($asset, $priority, $pipeline, $group, 'async'); + return $this->addJs($asset, $priority, $pipeline, 'async', $group); } /** @@ -86,9 +87,9 @@ trait LegacyAssetsTrait * * @return \Grav\Common\Assets */ - public function addDeferJs($asset, $priority = null, $pipeline = true, $group = null) + public function addDeferJs($asset, $priority = 10, $pipeline = true, $group = 'head') { - return $this->addJs($asset, $priority, $pipeline, $group, 'defer'); + return $this->addJs($asset, $priority, $pipeline, 'defer', $group); } } diff --git a/tests/unit/Grav/Common/AssetsTest.php b/tests/unit/Grav/Common/AssetsTest.php index a871da589..4e664d168 100644 --- a/tests/unit/Grav/Common/AssetsTest.php +++ b/tests/unit/Grav/Common/AssetsTest.php @@ -187,7 +187,7 @@ class AssetsTest extends \Codeception\TestCase\Test //Test CSS Groups $this->assets->reset(); - $this->assets->addCSS('test.css', null, null, 'footer'); + $this->assets->addCSS('test.css', ['group' => 'footer']); $css = $this->assets->css(); $this->assertEmpty($css); $css = $this->assets->css('footer'); @@ -221,7 +221,7 @@ class AssetsTest extends \Codeception\TestCase\Test //Test JS Groups $this->assets->reset(); - $this->assets->addJs('test.js', null, null, 'footer'); + $this->assets->addJs('test.js', ['group' => 'footer']); $js = $this->assets->js(); $this->assertEmpty($js); $js = $this->assets->js('footer'); @@ -251,7 +251,7 @@ class AssetsTest extends \Codeception\TestCase\Test //Test async / defer $this->assets->reset(); - $this->assets->addJs('test.js', null, null, null, 'async'); + $this->assets->addJs('test.js', ['loading' => 'async']); $js = $this->assets->js(); $this->assertSame('' . PHP_EOL, $js); @@ -280,11 +280,10 @@ class AssetsTest extends \Codeception\TestCase\Test $this->assertJsonStringEqualsJsonString($expected, $actual); $this->assets->reset(); - $this->assets->addJs('test.js', null, null, null, 'defer'); + $this->assets->addJs('test.js', ['loading' => 'defer']); $js = $this->assets->js(); $this->assertSame('' . PHP_EOL, $js); - $array = $this->assets->getJs(); $array = $this->assets->getJs(); /** @var Assets\BaseAsset $item */ $item = reset($array); @@ -383,6 +382,83 @@ class AssetsTest extends \Codeception\TestCase\Test '' . PHP_EOL, $js); } + public function testAddingLegacyFormat() + { + // regular CSS add + //test addCss(). Test adding asset to a separate group + $this->assets->reset(); + $this->assets->addCSS('test.css', 15, true, 'bottom', 'async'); + $css = $this->assets->css('bottom'); + $this->assertSame('' . PHP_EOL, $css); + + $array = $this->assets->getCss(); + /** @var Assets\BaseAsset $item */ + $item = reset($array); + $actual = json_encode($item); + $expected = ' + { + "type":"css", + "elements":{ + "asset":"\/test.css", + "asset_type":"css", + "order":0, + "group":"bottom", + "position":"pipeline", + "priority":15, + "attributes":{ + "type":"text\/css", + "rel":"stylesheet", + "loading":"async" + }, + "timestamp":null, + "modified":false, + "query":"" + } + }'; + $this->assertJsonStringEqualsJsonString($expected, $actual); + + $this->assets->reset(); + $this->assets->addJs('test.js', 15, false, 'defer', 'bottom'); + $js = $this->assets->js('bottom'); + $this->assertSame('' . PHP_EOL, $js); + + $array = $this->assets->getJs(); + /** @var Assets\BaseAsset $item */ + $item = reset($array); + $actual = json_encode($item); + $expected = ' + { + "type": "js", + "elements": { + "asset": "/test.js", + "asset_type": "js", + "order": 0, + "group": "bottom", + "position": "after", + "priority": 15, + "attributes": { + "loading": "defer" + }, + "timestamp": null, + "modified": false, + "query": "" + } + }'; + $this->assertJsonStringEqualsJsonString($expected, $actual); + + + $this->assets->reset(); + $this->assets->addInlineCss('body { color: black }', 15, 'bottom'); + $css = $this->assets->css('bottom'); + $this->assertSame('' . PHP_EOL, $css); + + $this->assets->reset(); + $this->assets->addInlineJs('alert("test")', 15, 'bottom', ['id' => 'foo']); + $js = $this->assets->js('bottom'); + $this->assertSame('' . PHP_EOL, $js); + + } + public function testAddingCSSAssetPropertiesWithArrayFromCollection() { $this->assets->registerCollection('test', ['/system/assets/whoops.css']);