From 20daf1f8e11746ef6dc4f925c75754c3b6063106 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 19 Jul 2016 14:09:15 +1200 Subject: [PATCH] API Abstract ThemeManifest into ThemeList BUG Fix Requirements not resolving $default theme --- admin/code/LeftAndMain.php | 38 ++---- core/Core.php | 4 +- dev/SapphireTest.php | 4 +- docs/en/04_Changelogs/4.0.0.md | 2 + forms/htmleditor/TinyMCEConfig.php | 24 ++-- ...erTest.php => ThemeResourceLoaderTest.php} | 61 ++++++++- .../templatemanifest/module/css/content.css | 3 + .../templatemanifest/module/css/project.css | 3 + .../module/javascript/content.js | 1 + .../module/javascript/project.js | 1 + .../myproject/css/project.css | 3 + .../myproject/javascript/project.js | 1 + .../themes/theme/css/content.css | 3 + .../themes/theme/css/project.css | 3 + .../themes/theme/javascript/content.js | 1 + .../themes/theme/javascript/project.js | 1 + tests/forms/RequirementsTest.php | 68 +++++----- tests/i18n/i18nSSLegacyAdapterTest.php | 8 +- tests/i18n/i18nTest.php | 8 +- tests/i18n/i18nTextCollectorTest.php | 8 +- view/Requirements.php | 90 ++++-------- view/SSViewer.php | 46 +++++-- view/ThemeList.php | 29 ++++ view/ThemeManifest.php | 95 +++++++++---- ...lateLoader.php => ThemeResourceLoader.php} | 128 +++++++++++++++--- 25 files changed, 413 insertions(+), 220 deletions(-) rename tests/core/manifest/{TemplateLoaderTest.php => ThemeResourceLoaderTest.php} (66%) create mode 100644 tests/core/manifest/fixtures/templatemanifest/module/css/content.css create mode 100644 tests/core/manifest/fixtures/templatemanifest/module/css/project.css create mode 100644 tests/core/manifest/fixtures/templatemanifest/module/javascript/content.js create mode 100644 tests/core/manifest/fixtures/templatemanifest/module/javascript/project.js create mode 100644 tests/core/manifest/fixtures/templatemanifest/myproject/css/project.css create mode 100644 tests/core/manifest/fixtures/templatemanifest/myproject/javascript/project.js create mode 100644 tests/core/manifest/fixtures/templatemanifest/themes/theme/css/content.css create mode 100644 tests/core/manifest/fixtures/templatemanifest/themes/theme/css/project.css create mode 100644 tests/core/manifest/fixtures/templatemanifest/themes/theme/javascript/content.js create mode 100644 tests/core/manifest/fixtures/templatemanifest/themes/theme/javascript/project.js create mode 100644 view/ThemeList.php rename view/{TemplateLoader.php => ThemeResourceLoader.php} (57%) diff --git a/admin/code/LeftAndMain.php b/admin/code/LeftAndMain.php index 1d0c6cf6d..3740e6b7f 100644 --- a/admin/code/LeftAndMain.php +++ b/admin/code/LeftAndMain.php @@ -20,7 +20,7 @@ use SilverStripe\Security\Member; use SilverStripe\Security\Permission; use SilverStripe\Security\Security; use SilverStripe\Security\PermissionProvider; - +use SilverStripe\View\ThemeResourceLoader; /** @@ -408,7 +408,8 @@ class LeftAndMain extends Controller implements PermissionProvider { && $candidate->MenuItem->controller && singleton($candidate->MenuItem->controller)->canView() ) { - return $this->redirect($candidate->Link); + $this->redirect($candidate->Link); + return; } } @@ -434,11 +435,14 @@ class LeftAndMain extends Controller implements PermissionProvider { ), ); - return Security::permissionFailure($this, $messageSet); + Security::permissionFailure($this, $messageSet); + return; } // Don't continue if there's already been a redirection request. - if($this->redirectedTo()) return; + if($this->redirectedTo()) { + return; + } // Audit logging hook if(empty($_REQUEST['executeForm']) && !$this->getRequest()->isAjax()) $this->extend('accessedCMS'); @@ -452,29 +456,6 @@ class LeftAndMain extends Controller implements PermissionProvider { // file because insufficient information exists when that is being processed $htmlEditorConfig = HTMLEditorConfig::get_active(); $htmlEditorConfig->setOption('language', i18n::get_tinymce_lang()); - if(!$htmlEditorConfig->getOption('content_css')) { - $cssFiles = array(); - $cssFiles[] = FRAMEWORK_ADMIN_DIR . '/client/dist/styles/editor.css'; - - // Use theme from the site config - if(class_exists('SiteConfig') && ($config = SiteConfig::current_site_config()) && $config->Theme) { - $theme = $config->Theme; - } elseif(Config::inst()->get('SSViewer', 'theme_enabled') && Config::inst()->get('SSViewer', 'theme')) { - $theme = Config::inst()->get('SSViewer', 'theme'); - } else { - $theme = false; - } - - if($theme) $cssFiles[] = THEMES_DIR . "/{$theme}/css/editor.css"; - else if(project()) $cssFiles[] = project() . '/css/editor.css'; - - // Remove files that don't exist - foreach($cssFiles as $k => $cssFile) { - if(!file_exists(BASE_PATH . '/' . $cssFile)) unset($cssFiles[$k]); - } - - $htmlEditorConfig->setOption('content_css', implode(',', $cssFiles)); - } Requirements::customScript(" window.ss = window.ss || {}; @@ -1713,9 +1694,8 @@ class LeftAndMain extends Controller implements PermissionProvider { if($page) { $navigator = new SilverStripeNavigator($page); return $navigator->renderWith($this->getTemplatesWithSuffix('_SilverStripeNavigator')); - } else { - return false; } + return null; } /** diff --git a/core/Core.php b/core/Core.php index 76e646012..ff6372705 100644 --- a/core/Core.php +++ b/core/Core.php @@ -77,7 +77,7 @@ require_once 'core/manifest/ConfigManifest.php'; require_once 'core/manifest/ConfigStaticManifest.php'; require_once 'core/manifest/ClassManifest.php'; require_once 'core/manifest/ManifestFileFinder.php'; -require_once 'view/TemplateLoader.php'; +require_once 'view/ThemeResourceLoader.php'; require_once 'core/manifest/TokenisedRegularExpression.php'; require_once 'control/injector/Injector.php'; @@ -112,7 +112,7 @@ $configManifest = new SS_ConfigManifest(BASE_PATH, false, $flush); Config::inst()->pushConfigYamlManifest($configManifest); // Load template manifest -SilverStripe\View\TemplateLoader::instance()->addSet('$default', new SilverStripe\View\ThemeManifest( +SilverStripe\View\ThemeResourceLoader::instance()->addSet('$default', new SilverStripe\View\ThemeManifest( BASE_PATH, project(), false, $flush )); diff --git a/dev/SapphireTest.php b/dev/SapphireTest.php index 9a7f50e28..c16269cb5 100644 --- a/dev/SapphireTest.php +++ b/dev/SapphireTest.php @@ -13,7 +13,7 @@ use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\Security\Group; use SilverStripe\Security\Permission; -use SilverStripe\View\TemplateLoader; +use SilverStripe\View\ThemeResourceLoader; use SilverStripe\View\ThemeManifest; /** @@ -848,7 +848,7 @@ class SapphireTest extends PHPUnit_Framework_TestCase { SS_ClassLoader::instance()->pushManifest($classManifest, false); SapphireTest::set_test_class_manifest($classManifest); - TemplateLoader::instance()->addSet('$default', new ThemeManifest( + ThemeResourceLoader::instance()->addSet('$default', new ThemeManifest( BASE_PATH, project(), true, $flush )); diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 056b332c4..71b6c4be4 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -51,6 +51,8 @@ a shorthand substitute. * `FormField->dontEscape` has been removed. Escaping is now managed on a class by class basis. * `DBString->LimitWordCountXML` removed. Use `LimitWordCount` for XML safe version. + * `$module` parameter in `themedCSS` and `themedJavascript` removed. + * Theme selector has been removed from SiteConfig. Please use `SSViewer.themes` config instead. ## New API diff --git a/forms/htmleditor/TinyMCEConfig.php b/forms/htmleditor/TinyMCEConfig.php index 423ee7ca2..fee3e8ec6 100644 --- a/forms/htmleditor/TinyMCEConfig.php +++ b/forms/htmleditor/TinyMCEConfig.php @@ -1,6 +1,6 @@ getPath($theme); - $editorDir = $path . '/css/editor.css';; + // Add standard editor.css + $editor[] = Director::absoluteURL(FRAMEWORK_ADMIN_DIR . '/client/dist/styles/editor.css'); - if(file_exists(BASE_PATH . '/' . $editorDir)) { - $editor[] = Controller::join_links( - Director::absoluteBaseURL(), - $editorDir - ); - - break; - } + // Themed editor.css + $themedEditor = ThemeResourceLoader::instance()->findThemedCSS('editor', SSViewer::get_themes()); + if($themedEditor) { + $editor[] = Director::absoluteURL($themedEditor, Director::BASE); } + return $editor; } diff --git a/tests/core/manifest/TemplateLoaderTest.php b/tests/core/manifest/ThemeResourceLoaderTest.php similarity index 66% rename from tests/core/manifest/TemplateLoaderTest.php rename to tests/core/manifest/ThemeResourceLoaderTest.php index f41845fd7..1d9cddc2e 100644 --- a/tests/core/manifest/TemplateLoaderTest.php +++ b/tests/core/manifest/ThemeResourceLoaderTest.php @@ -1,6 +1,6 @@ base = dirname(__FILE__) . '/fixtures/templatemanifest'; // New ThemeManifest for that root $this->manifest = new ThemeManifest($this->base, 'myproject', false, true); // New Loader for that root - $this->loader = new TemplateLoader($this->base); + $this->loader = new ThemeResourceLoader($this->base); $this->loader->addSet('$default', $this->manifest); } @@ -128,6 +129,58 @@ class TemplateLoaderTest extends SapphireTest { ); } + public function testFindThemedCSS() { + $this->assertEquals( + "myproject/css/project.css", + $this->loader->findThemedCSS('project', ['$default', 'theme']) + ); + $this->assertEquals( + "themes/theme/css/project.css", + $this->loader->findThemedCSS('project', ['theme', '$default']) + ); + $this->assertEmpty( + $this->loader->findThemedCSS('nofile', ['theme', '$default']) + ); + $this->assertEquals( + 'module/css/content.css', + $this->loader->findThemedCSS('content', ['/module', 'theme']) + ); + $this->assertEquals( + 'module/css/content.css', + $this->loader->findThemedCSS('content', ['/module', 'theme', '$default']) + ); + $this->assertEquals( + 'module/css/content.css', + $this->loader->findThemedCSS('content', ['$default', '/module', 'theme']) + ); + } + + public function testFindThemedJavascript() { + $this->assertEquals( + "myproject/javascript/project.js", + $this->loader->findThemedJavascript('project', ['$default', 'theme']) + ); + $this->assertEquals( + "themes/theme/javascript/project.js", + $this->loader->findThemedJavascript('project', ['theme', '$default']) + ); + $this->assertEmpty( + $this->loader->findThemedJavascript('nofile', ['theme', '$default']) + ); + $this->assertEquals( + 'module/javascript/content.js', + $this->loader->findThemedJavascript('content', ['/module', 'theme']) + ); + $this->assertEquals( + 'module/javascript/content.js', + $this->loader->findThemedJavascript('content', ['/module', 'theme', '$default']) + ); + $this->assertEquals( + 'module/javascript/content.js', + $this->loader->findThemedJavascript('content', ['$default', '/module', 'theme']) + ); + } + protected function createTestTemplates($templates) { foreach ($templates as $template) { file_put_contents($template, ''); diff --git a/tests/core/manifest/fixtures/templatemanifest/module/css/content.css b/tests/core/manifest/fixtures/templatemanifest/module/css/content.css new file mode 100644 index 000000000..30ccaefbc --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/module/css/content.css @@ -0,0 +1,3 @@ +.class { + display: block; +} diff --git a/tests/core/manifest/fixtures/templatemanifest/module/css/project.css b/tests/core/manifest/fixtures/templatemanifest/module/css/project.css new file mode 100644 index 000000000..30ccaefbc --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/module/css/project.css @@ -0,0 +1,3 @@ +.class { + display: block; +} diff --git a/tests/core/manifest/fixtures/templatemanifest/module/javascript/content.js b/tests/core/manifest/fixtures/templatemanifest/module/javascript/content.js new file mode 100644 index 000000000..6fa47f8ba --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/module/javascript/content.js @@ -0,0 +1 @@ +var i = 0; diff --git a/tests/core/manifest/fixtures/templatemanifest/module/javascript/project.js b/tests/core/manifest/fixtures/templatemanifest/module/javascript/project.js new file mode 100644 index 000000000..6fa47f8ba --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/module/javascript/project.js @@ -0,0 +1 @@ +var i = 0; diff --git a/tests/core/manifest/fixtures/templatemanifest/myproject/css/project.css b/tests/core/manifest/fixtures/templatemanifest/myproject/css/project.css new file mode 100644 index 000000000..30ccaefbc --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/myproject/css/project.css @@ -0,0 +1,3 @@ +.class { + display: block; +} diff --git a/tests/core/manifest/fixtures/templatemanifest/myproject/javascript/project.js b/tests/core/manifest/fixtures/templatemanifest/myproject/javascript/project.js new file mode 100644 index 000000000..6fa47f8ba --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/myproject/javascript/project.js @@ -0,0 +1 @@ +var i = 0; diff --git a/tests/core/manifest/fixtures/templatemanifest/themes/theme/css/content.css b/tests/core/manifest/fixtures/templatemanifest/themes/theme/css/content.css new file mode 100644 index 000000000..30ccaefbc --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/themes/theme/css/content.css @@ -0,0 +1,3 @@ +.class { + display: block; +} diff --git a/tests/core/manifest/fixtures/templatemanifest/themes/theme/css/project.css b/tests/core/manifest/fixtures/templatemanifest/themes/theme/css/project.css new file mode 100644 index 000000000..30ccaefbc --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/themes/theme/css/project.css @@ -0,0 +1,3 @@ +.class { + display: block; +} diff --git a/tests/core/manifest/fixtures/templatemanifest/themes/theme/javascript/content.js b/tests/core/manifest/fixtures/templatemanifest/themes/theme/javascript/content.js new file mode 100644 index 000000000..6fa47f8ba --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/themes/theme/javascript/content.js @@ -0,0 +1 @@ +var i = 0; diff --git a/tests/core/manifest/fixtures/templatemanifest/themes/theme/javascript/project.js b/tests/core/manifest/fixtures/templatemanifest/themes/theme/javascript/project.js new file mode 100644 index 000000000..6fa47f8ba --- /dev/null +++ b/tests/core/manifest/fixtures/templatemanifest/themes/theme/javascript/project.js @@ -0,0 +1 @@ +var i = 0; diff --git a/tests/forms/RequirementsTest.php b/tests/forms/RequirementsTest.php index 554e37288..f9f62a4f9 100644 --- a/tests/forms/RequirementsTest.php +++ b/tests/forms/RequirementsTest.php @@ -117,16 +117,16 @@ class RequirementsTest extends SapphireTest { ) ); } - + protected function setupCombinedRequirementsJavascriptAsyncDefer($backend, $async, $defer) { $basePath = $this->getCurrentRelativePath(); $this->setupRequirements($backend); - + // require files normally (e.g. called from a FormField instance) $backend->javascript($basePath . '/RequirementsTest_a.js'); $backend->javascript($basePath . '/RequirementsTest_b.js'); $backend->javascript($basePath . '/RequirementsTest_c.js'); - + // require two of those files as combined includes $backend->combineFiles( 'RequirementsTest_bc.js', @@ -227,49 +227,49 @@ class RequirementsTest extends SapphireTest { 'combined files are not included twice' ); } - + public function testCombinedJavascriptAsyncDefer() { /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); - + $this->setupCombinedRequirementsJavascriptAsyncDefer($backend, true, false); - + $combinedFileName = '/_combinedfiles/RequirementsTest_bc-2a55d56.js'; $combinedFilePath = AssetStoreTest_SpyStore::base_path() . $combinedFileName; - + $html = $backend->includeInHTML(false, self::$html_template); - + /* ASYNC IS INCLUDED IN SCRIPT TAG */ $this->assertRegExp( '/src=".*' . preg_quote($combinedFileName, '/') . '" async/', $html, 'async is included in script tag' ); - + /* DEFER IS NOT INCLUDED IN SCRIPT TAG */ $this->assertNotContains('defer', $html, 'defer is not included'); - + /* COMBINED JAVASCRIPT FILE EXISTS */ clearstatcache(); // needed to get accurate file_exists() results $this->assertFileExists($combinedFilePath, 'combined javascript file exists'); - + /* COMBINED JAVASCRIPT HAS CORRECT CONTENT */ $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('b')") !== false), 'combined javascript has correct content'); $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('c')") !== false), 'combined javascript has correct content'); - + /* COMBINED FILES ARE NOT INCLUDED TWICE */ $this->assertNotRegExp('/src=".*\/RequirementsTest_b\.js/', $html, 'combined files are not included twice'); $this->assertNotRegExp('/src=".*\/RequirementsTest_c\.js/', $html, 'combined files are not included twice'); - + /* NORMAL REQUIREMENTS ARE STILL INCLUDED */ $this->assertRegExp('/src=".*\/RequirementsTest_a\.js/', $html, 'normal requirements are still included'); - + /* NORMAL REQUIREMENTS DON'T HAVE ASYNC/DEFER */ $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async/', $html, 'normal requirements don\'t have async'); @@ -277,47 +277,47 @@ class RequirementsTest extends SapphireTest { 'normal requirements don\'t have defer'); $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async defer/', $html, 'normal requirements don\'t have async/defer'); - + // setup again for testing defer unlink($combinedFilePath); /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); - + $this->setupCombinedRequirementsJavascriptAsyncDefer($backend, false, true); - + $html = $backend->includeInHTML(self::$html_template); - + /* DEFER IS INCLUDED IN SCRIPT TAG */ $this->assertRegExp( '/src=".*' . preg_quote($combinedFileName, '/') . '" defer/', $html, 'defer is included in script tag' ); - + /* ASYNC IS NOT INCLUDED IN SCRIPT TAG */ $this->assertNotContains('async', $html, 'async is not included'); - + /* COMBINED JAVASCRIPT FILE EXISTS */ clearstatcache(); // needed to get accurate file_exists() results $this->assertFileExists($combinedFilePath, 'combined javascript file exists'); - + /* COMBINED JAVASCRIPT HAS CORRECT CONTENT */ $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('b')") !== false), 'combined javascript has correct content'); $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('c')") !== false), 'combined javascript has correct content'); - + /* COMBINED FILES ARE NOT INCLUDED TWICE */ $this->assertNotRegExp('/src=".*\/RequirementsTest_b\.js/', $html, 'combined files are not included twice'); $this->assertNotRegExp('/src=".*\/RequirementsTest_c\.js/', $html, 'combined files are not included twice'); - + /* NORMAL REQUIREMENTS ARE STILL INCLUDED */ $this->assertRegExp('/src=".*\/RequirementsTest_a\.js/', $html, 'normal requirements are still included'); - + /* NORMAL REQUIREMENTS DON'T HAVE ASYNC/DEFER */ $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async/', $html, 'normal requirements don\'t have async'); @@ -325,44 +325,44 @@ class RequirementsTest extends SapphireTest { 'normal requirements don\'t have defer'); $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async defer/', $html, 'normal requirements don\'t have async/defer'); - + // setup again for testing async and defer unlink($combinedFilePath); /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create('Requirements_Backend'); - + $this->setupCombinedRequirementsJavascriptAsyncDefer($backend, true, true); - + $html = $backend->includeInHTML(self::$html_template); - + /* ASYNC/DEFER IS INCLUDED IN SCRIPT TAG */ $this->assertRegExp( '/src=".*' . preg_quote($combinedFileName, '/') . '" async defer/', $html, 'async and defer are included in script tag' ); - + /* COMBINED JAVASCRIPT FILE EXISTS */ clearstatcache(); // needed to get accurate file_exists() results $this->assertFileExists($combinedFilePath, 'combined javascript file exists'); - + /* COMBINED JAVASCRIPT HAS CORRECT CONTENT */ $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('b')") !== false), 'combined javascript has correct content'); $this->assertTrue((strpos(file_get_contents($combinedFilePath), "alert('c')") !== false), 'combined javascript has correct content'); - + /* COMBINED FILES ARE NOT INCLUDED TWICE */ $this->assertNotRegExp('/src=".*\/RequirementsTest_b\.js/', $html, 'combined files are not included twice'); $this->assertNotRegExp('/src=".*\/RequirementsTest_c\.js/', $html, 'combined files are not included twice'); - + /* NORMAL REQUIREMENTS ARE STILL INCLUDED */ $this->assertRegExp('/src=".*\/RequirementsTest_a\.js/', $html, 'normal requirements are still included'); - + /* NORMAL REQUIREMENTS DON'T HAVE ASYNC/DEFER */ $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async/', $html, 'normal requirements don\'t have async'); @@ -370,7 +370,7 @@ class RequirementsTest extends SapphireTest { 'normal requirements don\'t have defer'); $this->assertNotRegExp('/src=".*\/RequirementsTest_a\.js\?m=\d+" async defer/', $html, 'normal requirements don\'t have async/defer'); - + unlink($combinedFilePath); } diff --git a/tests/i18n/i18nSSLegacyAdapterTest.php b/tests/i18n/i18nSSLegacyAdapterTest.php index 475cba780..f3f4d0494 100644 --- a/tests/i18n/i18nSSLegacyAdapterTest.php +++ b/tests/i18n/i18nSSLegacyAdapterTest.php @@ -1,6 +1,6 @@ update('Director', 'alternate_base_folder', $this->alternateBasePath); // Replace old template loader with new one with alternate base path - $this->_oldLoader = TemplateLoader::instance(); - TemplateLoader::set_instance(new TemplateLoader($this->alternateBasePath)); + $this->_oldLoader = ThemeResourceLoader::instance(); + ThemeResourceLoader::set_instance(new ThemeResourceLoader($this->alternateBasePath)); $this->_oldTheme = Config::inst()->get('SSViewer', 'theme'); Config::inst()->update('SSViewer', 'theme', 'testtheme1'); @@ -42,7 +42,7 @@ class i18nSSLegacyAdapterTest extends SapphireTest { } public function tearDown() { - TemplateLoader::set_instance($this->_oldLoader); + ThemeResourceLoader::set_instance($this->_oldLoader); SS_ClassLoader::instance()->popManifest(); i18n::set_locale($this->originalLocale); Config::inst()->update('Director', 'alternate_base_folder', null); diff --git a/tests/i18n/i18nTest.php b/tests/i18n/i18nTest.php index 3f774f5b2..770a84d94 100644 --- a/tests/i18n/i18nTest.php +++ b/tests/i18n/i18nTest.php @@ -1,7 +1,7 @@ update('Director', 'alternate_base_folder', $this->alternateBasePath); // Replace old template loader with new one with alternate base path - $this->_oldLoader = TemplateLoader::instance(); - TemplateLoader::set_instance($loader = new TemplateLoader($this->alternateBasePath)); + $this->_oldLoader = ThemeResourceLoader::instance(); + ThemeResourceLoader::set_instance($loader = new ThemeResourceLoader($this->alternateBasePath)); $loader->addSet('$default', new ThemeManifest( $this->alternateBasePath, project(), false, true )); @@ -64,7 +64,7 @@ class i18nTest extends SapphireTest { } public function tearDown() { - TemplateLoader::set_instance($this->_oldLoader); + ThemeResourceLoader::set_instance($this->_oldLoader); i18n::set_locale($this->originalLocale); Config::inst()->update('Director', 'alternate_base_folder', null); Config::inst()->update('SSViewer', 'theme', $this->_oldTheme); diff --git a/tests/i18n/i18nTextCollectorTest.php b/tests/i18n/i18nTextCollectorTest.php index ca599b452..448fd374c 100644 --- a/tests/i18n/i18nTextCollectorTest.php +++ b/tests/i18n/i18nTextCollectorTest.php @@ -1,6 +1,6 @@ _oldLoader = TemplateLoader::instance(); - TemplateLoader::set_instance(new TemplateLoader($this->alternateBasePath)); + $this->_oldLoader = ThemeResourceLoader::instance(); + ThemeResourceLoader::set_instance(new ThemeResourceLoader($this->alternateBasePath)); } public function tearDown() { - TemplateLoader::set_instance($this->_oldLoader); + ThemeResourceLoader::set_instance($this->_oldLoader); // Pop if added during testing if(SS_ClassLoader::instance()->getManifest() === $this->manifest) { SS_ClassLoader::instance()->popManifest(); diff --git a/view/Requirements.php b/view/Requirements.php index c7b072388..a1e55c971 100644 --- a/view/Requirements.php +++ b/view/Requirements.php @@ -1,7 +1,7 @@ themedCSS($name, $module, $media); + public static function themedCSS($name, $media = null) { + self::backend()->themedCSS($name, $media); } /** @@ -207,13 +205,11 @@ class Requirements implements Flushable { * the module is used. * * @param string $name The name of the file - eg '/javascript/File.js' would have the name 'File' - * @param string $module The module to fall back to if the javascript file does not exist in the - * current theme. * @param string $type Comma-separated list of types to use in the script tag * (e.g. 'text/javascript,text/ecmascript') */ - public static function themedJavascript($name, $module = null, $type = null) { - return self::backend()->themedJavascript($name, $module, $type); + public static function themedJavascript($name, $type = null) { + return self::backend()->themedJavascript($name, $type); } /** @@ -1810,34 +1806,19 @@ class Requirements_Backend * the module is used. * * @param string $name The name of the file - eg '/css/File.css' would have the name 'File' - * @param string $module The module to fall back to if the css file does not exist in the - * current theme. * @param string $media Comma-separated list of media types to use in the link tag * (e.g. 'screen,projector') */ - public function themedCSS($name, $module = null, $media = null) { - $css = "/css/$name.css"; - - $project = project(); - $absbase = BASE_PATH . DIRECTORY_SEPARATOR; - $absproject = $absbase . $project; - - if(file_exists($absproject . $css)) { - return $this->css($project . $css, $media); + public function themedCSS($name, $media = null) { + $path = ThemeResourceLoader::instance()->findThemedCSS($name, SSViewer::get_themes()); + if($path) { + $this->css($path, $media); + } else { + throw new \InvalidArgumentException( + "The css file doesn't exists. Please check if the file $name.css exists in any context or search for " + . "themedCSS references calling this file in your templates." + ); } - - foreach(SSViewer::get_themes() as $theme) { - $path = TemplateLoader::instance()->getPath($theme); - $abspath = BASE_PATH . '/' . $path; - - if(file_exists($abspath . $css)) { - return $this->css($path . $css, $media); - } - } - throw new \InvalidArgumentException( - "The css file doesn't exists. Please check if the file $name.css exists in any context or search for " - . "themedCSS references calling this file in your templates." - ); } /** @@ -1848,38 +1829,23 @@ class Requirements_Backend * the module is used. * * @param string $name The name of the file - eg '/js/File.js' would have the name 'File' - * @param string $module The module to fall back to if the javascript file does not exist in the - * current theme. * @param string $type Comma-separated list of types to use in the script tag * (e.g. 'text/javascript,text/ecmascript') */ - public function themedJavascript($name, $module = null, $type = null) { - $js = "/javascript/$name.js"; - - $opts = array( - 'type' => $type, - ); - - $project = project(); - $absbase = BASE_PATH . DIRECTORY_SEPARATOR; - $absproject = $absbase . $project; - - if(file_exists($absproject . $js)) { - return $this->javascript($project . $js, $opts); - } - - foreach(SSViewer::get_themes() as $theme) { - $path = TemplateLoader::instance()->getPath($theme); - $abspath = BASE_PATH . '/' . $path; - - if(file_exists($abspath . $js)) { - return $this->javascript($path . $js, $opts); - } - } - throw new \InvalidArgumentException( - "The javascript file doesn't exists. Please check if the file $name.js exists in any context or search for " - . "themedJavascript references calling this file in your templates." - ); + public function themedJavascript($name, $type = null) { + $path = ThemeResourceLoader::instance()->findThemedJavascript($name, SSViewer::get_themes()); + if($path) { + $opts = []; + if($type) { + $opts['type'] = $type; + } + $this->javascript($path, $opts); + } else { + throw new \InvalidArgumentException( + "The javascript file doesn't exists. Please check if the file $name.js exists in any " + . "context or search for themedJavascript references calling this file in your templates." + ); + } } /** diff --git a/view/SSViewer.php b/view/SSViewer.php index 7f771f2fa..6d3168534 100644 --- a/view/SSViewer.php +++ b/view/SSViewer.php @@ -3,7 +3,7 @@ use SilverStripe\ORM\FieldType\DBField; use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\Security\Permission; -use SilverStripe\View\TemplateLoader; +use SilverStripe\View\ThemeResourceLoader; /** * This tracks the current scope for an SSViewer instance. It has three goals: @@ -686,6 +686,11 @@ class SSViewer_DataPresenter extends SSViewer_Scope { */ class SSViewer implements Flushable { + /** + * Identifier for the default theme + */ + const DEFAULT_THEME = '$default'; + /** * @config * @var boolean $source_file_comments @@ -809,6 +814,12 @@ class SSViewer implements Flushable { return $viewer; } + /** + * Assign the list of active themes to apply. + * If default themes should be included add $default as the last entry. + * + * @param array $themes + */ public static function set_themes($themes = []) { Config::inst()->remove('SSViewer', 'themes'); Config::inst()->update('SSViewer', 'themes', $themes); @@ -819,14 +830,23 @@ class SSViewer implements Flushable { } public static function get_themes() { - $res = ['$default']; + $default = [self::DEFAULT_THEME]; - if (Config::inst()->get('SSViewer', 'theme_enabled')) { - if ($list = Config::inst()->get('SSViewer', 'themes')) $res = $list; - elseif ($theme = Config::inst()->get('SSViewer', 'theme')) $res = [$theme, '$default']; + if (!Config::inst()->get('SSViewer', 'theme_enabled')) { + return $default; } - return $res; + // Explicit list is assigned + if ($list = Config::inst()->get('SSViewer', 'themes')) { + return $list; + } + + // Support legacy behaviour + if ($theme = Config::inst()->get('SSViewer', 'theme')) { + return [$theme, self::DEFAULT_THEME]; + } + + return $default; } /** @@ -835,7 +855,7 @@ class SSViewer implements Flushable { */ public static function set_theme($theme) { Deprecation::notice('4.0', 'Use the "SSViewer#set_themes" instead'); - self::set_themes([$theme]); + self::set_themes([$theme, self::DEFAULT_THEME]); } /** @@ -850,9 +870,9 @@ class SSViewer implements Flushable { public static function get_templates_by_class($className, $suffix = '', $baseClass = null) { // Figure out the class name from the supplied context. if(!is_string($className) || !class_exists($className)) { - throw new InvalidArgumentException('SSViewer::get_templates_by_class() expects a valid class name as ' . - 'its first parameter.'); - return array(); + throw new InvalidArgumentException( + 'SSViewer::get_templates_by_class() expects a valid class name as its first parameter.' + ); } $templates = array(); $classes = array_reverse(ClassInfo::ancestry($className)); @@ -904,7 +924,7 @@ class SSViewer implements Flushable { public function setTemplate($templates) { $this->templates = $templates; - $this->chosen = TemplateLoader::instance()->findTemplate($templates, self::get_themes()); + $this->chosen = ThemeResourceLoader::instance()->findTemplate($templates, self::get_themes()); $this->subTemplates = []; } @@ -937,7 +957,7 @@ class SSViewer implements Flushable { * @return boolean */ public static function hasTemplate($templates) { - return (bool)TemplateLoader::instance()->findTemplate($templates, self::get_themes()); + return (bool)ThemeResourceLoader::instance()->findTemplate($templates, self::get_themes()); } /** @@ -1014,7 +1034,7 @@ class SSViewer implements Flushable { * @return string Full system path to a template file */ public static function getTemplateFileByType($identifier, $type) { - return TemplateLoader::instance()->findTemplate(['type' => $type, $identifier], self::get_themes()); + return ThemeResourceLoader::instance()->findTemplate(['type' => $type, $identifier], self::get_themes()); } /** diff --git a/view/ThemeList.php b/view/ThemeList.php new file mode 100644 index 000000000..8bfe66869 --- /dev/null +++ b/view/ThemeList.php @@ -0,0 +1,29 @@ + + * [ + * '/mysite', + * 'vendor/module:themename', + * '/framework/admin' + * 'simple' + * ] + * + * + * These may be in any format, including vendor/namespace:path, or /absolute-path, + * but will not include references to any other {@see ThemeContainer} as + * SSViewer::get_themes() does. + * + * @return array + */ + public function getThemes(); +} diff --git a/view/ThemeManifest.php b/view/ThemeManifest.php index 06f04fdb8..a766edaf8 100644 --- a/view/ThemeManifest.php +++ b/view/ThemeManifest.php @@ -2,7 +2,8 @@ namespace SilverStripe\View; -use \ManifestFileFinder; +use ManifestCache; +use ManifestFileFinder; /** * A class which builds a manifest of all themes (which is really just a directory called "templates") @@ -10,17 +11,50 @@ use \ManifestFileFinder; * @package framework * @subpackage manifest */ -class ThemeManifest { +class ThemeManifest implements ThemeList { const TEMPLATES_DIR = 'templates'; + /** + * Base path + * + * @var string + */ protected $base; + + /** + * Include tests + * + * @var bool + */ protected $tests; + + /** + * Path to application code + * + * @var string + */ protected $project; + /** + * Cache + * + * @var ManifestCache + */ protected $cache; + + /** + * Cache key + * + * @var string + */ protected $cacheKey; + /** + * List of theme root directories + * + * @var string + */ protected $themes = null; /** @@ -71,21 +105,10 @@ class ThemeManifest { )); } - /** - * Returns a map of all themes information. The map is in the following format: - * - * - * [ - * 'mysite', - * 'framework', - * 'framework/admin' - * ] - * - * - * @return array - */ public function getThemes() { - if ($this->themes === null) $this->init(); + if ($this->themes === null) { + $this->init(); + } return $this->themes; } @@ -111,26 +134,38 @@ class ThemeManifest { } } + /** + * Add a directory to the manifest + * + * @param string $basename + * @param string $pathname + * @param int $depth + */ public function handleDirectory($basename, $pathname, $depth) { - if ($basename == self::TEMPLATES_DIR) { - // We only want part of the full path, so split into directories - $parts = explode('/', $pathname); - // Take the end (the part relative to base), except the very last directory - $themeParts = array_slice($parts, -$depth, $depth-1); - // Then join again - $path = '/'.implode('/', $themeParts); + if ($basename !== self::TEMPLATES_DIR) { + return; + } - // If this is in the project, add to beginning of list. Else add to end. - if ($themeParts[0] == $this->project) { - array_unshift($this->themes, $path); - } - else { - array_push($this->themes, $path); - } + // We only want part of the full path, so split into directories + $parts = explode('/', $pathname); + // Take the end (the part relative to base), except the very last directory + $themeParts = array_slice($parts, -$depth, $depth-1); + // Then join again + $path = '/'.implode('/', $themeParts); + + // If this is in the project, add to beginning of list. Else add to end. + if ($themeParts[0] == $this->project) { + array_unshift($this->themes, $path); + } + else { + array_push($this->themes, $path); } } + /** + * Initialise the manifest + */ protected function init() { if ($data = $this->cache->load($this->cacheKey)) { $this->themes = $data; diff --git a/view/TemplateLoader.php b/view/ThemeResourceLoader.php similarity index 57% rename from view/TemplateLoader.php rename to view/ThemeResourceLoader.php index 5c66e1d8a..28afd3f96 100644 --- a/view/TemplateLoader.php +++ b/view/ThemeResourceLoader.php @@ -2,31 +2,44 @@ namespace SilverStripe\View; +use Deprecation; + /** * Handles finding templates from a stack of template manifest objects. * * @package framework * @subpackage view */ -class TemplateLoader { +class ThemeResourceLoader { /** - * @var TemplateLoader + * @var ThemeResourceLoader */ private static $instance; protected $base; /** - * @var ThemeManifest[] + * List of template "sets" that contain a test manifest, and have an alias. + * E.g. '$default' + * + * @var ThemeList[] */ protected $sets = []; + /** + * @return ThemeResourceLoader + */ public static function instance() { return self::$instance ? self::$instance : self::$instance = new self(); } - public static function set_instance(TemplateLoader $instance) { + /** + * Set instance + * + * @param ThemeResourceLoader $instance + */ + public static function set_instance(ThemeResourceLoader $instance) { self::$instance = $instance; } @@ -38,12 +51,25 @@ class TemplateLoader { * Add a new theme manifest for a given identifier. E.g. '$default' * * @param string $set - * @param ThemeManifest $manifest + * @param ThemeList $manifest */ - public function addSet($set, $manifest) { + public function addSet($set, ThemeList $manifest) { $this->sets[$set] = $manifest; } + /** + * Get a named theme set + * + * @param string $set + * @return ThemeList + */ + public function getSet($set) { + if(isset($this->sets[$set])) { + return $this->sets[$set]; + } + return null; + } + /** * Given a theme identifier, determine the path from the root directory * @@ -56,7 +82,7 @@ class TemplateLoader { * of that module. ('/mymodule'). * * @param string $identifier Theme identifier. - * @return string Path from root, not including leading forward slash. E.g. themes/mytheme + * @return string Path from root, not including leading or trailing forward slash. E.g. themes/mytheme */ public function getPath($identifier) { $slashPos = strpos($identifier, '/'); @@ -111,7 +137,9 @@ class TemplateLoader { * theme-coupled resolution. * @param array $themes List of themes to use to resolve themes. In most cases * you should pass in {@see SSViewer::get_themes()} - * @return string Path to resolved template file, or null if not resolved. + * @return string Absolute path to resolved template file, or null if not resolved. + * File location will be in the format themes//templates///.ss + * Note that type (e.g. 'Layout') is not the root level directory under 'templates'. */ public function findTemplate($template, $themes) { $type = ''; @@ -141,14 +169,13 @@ class TemplateLoader { $tail = array_pop($parts); $head = implode('/', $parts); - foreach($themes as $themename) { - $subthemes = isset($this->sets[$themename]) ? $this->sets[$themename]->getThemes() : [$themename]; - - foreach($subthemes as $theme) { - $themePath = $this->base . '/' . $this->getPath($theme); - - $path = $themePath . '/templates/' . implode('/', array_filter([$head, $type, $tail])) . '.ss'; - if (file_exists($path)) return $path; + $themePaths = $this->getThemePaths($themes); + foreach($themePaths as $themePath) { + // Join path + $pathParts = [ $this->base, $themePath, 'templates', $head, $type, $tail ]; + $path = implode('/', array_filter($pathParts)) . '.ss'; + if (file_exists($path)) { + return $path; } } } @@ -157,4 +184,73 @@ class TemplateLoader { return null; } + /** + * Resolve themed CSS path + * + * @param string $name Name of CSS file without extension + * @param array $themes List of themes + * @return string Path to resolved CSS file (relative to base dir) + */ + public function findThemedCSS($name, $themes) + { + $css = "/css/$name.css"; + $paths = $this->getThemePaths($themes); + foreach ($paths as $themePath) { + $abspath = $this->base . '/' . $themePath; + + if (file_exists($abspath . $css)) { + return $themePath . $css; + } + } + + // CSS exists in no context + return null; + } + + /** + * Registers the given themeable javascript as required. + * + * A javascript file in the current theme path name 'themename/javascript/$name.js' is first searched for, + * and it that doesn't exist and the module parameter is set then a javascript file with that name in + * the module is used. + * + * @param string $name The name of the file - eg '/js/File.js' would have the name 'File' + * @param array $themes List of themes + * @return string Path to resolved javascript file (relative to base dir) + */ + public function findThemedJavascript($name, $themes) { + $js = "/javascript/$name.js"; + $paths = $this->getThemePaths($themes); + foreach ($paths as $themePath) { + $abspath = $this->base . '/' . $themePath; + + if (file_exists($abspath . $js)) { + return $themePath . $js; + } + } + + // js exists in no context + return null; + } + + /** + * Resolve all themes to the list of root folders relative to site root + * + * @param array $themes List of themes to resolve. Supports named theme sets. + * @return array List of root-relative folders in order of precendence. + */ + public function getThemePaths($themes) { + $paths = []; + foreach($themes as $themename) { + // Expand theme sets + $set = $this->getSet($themename); + $subthemes = $set ? $set->getThemes() : [$themename]; + + // Resolve paths + foreach ($subthemes as $theme) { + $paths[] = $this->getPath($theme); + } + } + return $paths; + } }