[#4074] - [#34027] Add ability to override the renderer [New feature 3.x]
- Closed
- 17 Nov 2014
- Medium
- Build: staging
- # 4074
- Diff
- dgt41:override_renderer
- Foreign ID 34027
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
Scope
In Joomla! if you want to override something you have to put it in the active template directory (e.g.: the layouts goes in (active template)/html/layouts).
This PR introduces the ability to override the whole rendering code in the template by copying (and modifying per your preferences) the /libraries/joomla/document directory.
It shouldn't rise any backward compatibility issues (except that the directory [active template]/document might be in use?).
What you can do if this gets accepted?
Because renderers are the place where the code gets from arrays and strings to actual html, xml, raw, feed or json, you can do pretty much whatever you want.
Some examples of creative code that can be put in the renderers:
Put custom code in renderer html (eg minifier, remove <JDoc )
Create custom code for head (e.g.. scripts and css concatenation and minification, custom metas, scripts to bottom, etc)
Mess around with the error page and feed in creative ways
Or go all the way and get mustache or twig in the template
Why?
Because this is one area that if you want to override and use your own logic you have to do it through a plugin (this is way easier and cleaner).
Right now if you want to override the default renderers of Joomla! you have to use a plugin like https://gist.github.com/dongilbert/3237387 from @dongilbert
But then again templates should not depend on plugins
Test instructions:
Apply the patch
assuming selected template: protostar
create directory named document in /templates/protostar
copy the folder html from /libraries/joomla/document to protostar/document
So you have /templates/protostar/document/html/html.php
edit html.php (in protostar foolder...)
In the last function _fetchTemplate()
replace the line before return so it will be like
$this->_template = $this->_loadTemplate($directory . '/' . $template, $file). '<!-- comment -->’;
(Add a comment after the document)
Refresh the page, see the source at the bottom you should have <!-- comment -->
You just overrided the render!
Feature tracker: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=34027
This one replaces PR #4052
jissues-bot
- | Labels |
Added:
?
?
|
||
| Title |
|
||||||
| Title |
|
||||||
| Title |
|
||||||
| Description | <h1>Scope</h1> <p>In Joomla! if you want to override something you have to put it in the active template directory (e.g.: the layouts goes in (active template)/html/layouts).<br> This PR introduces the ability to override the whole rendering process in the template by copying (and modifying per your preferences) the /libraries/joomla/document directory.</p> <p>It shouldn't rise any backward compatibility issues (except that the directory [active template]/document might be in use?).</p> <h1>What you can do if this gets accepted?</h1> <p>Because renderers are the place where the code gets from arrays and strings to actual html, xml, raw, feed or json, you can do pretty much whatever you want.<br> Some examples of creative code that can be put in the renderers:<br> Put custom code in renderer html (eg minifier, remove <JDoc )<br> Create custom code for head (e.g.. scripts and css concatenation, custom metas)<br> Mesh around with the error page and feed in creative ways<br> Or go all the way and get mustache or twig in the template</p> <h1>Why?</h1> <p>Because this is one area that if you want to override and use your own logic you have to do it through a plugin (this is way easier and cleaner)</p> <h1>Test instructions:</h1> <p>Apply the patch<br> assuming selected template: protostar<br> create directory named document in /templates/protostar<br> copy the folder html from /libraries/joomla/document to protostar/document<br> So you have /templates/document/html/html.php<br> edit html.php (in protostar foolder...)<br> In the last function _fetchTemplate()<br> replace the line before return so it will be like<br><code>$this->_template = $this->_loadTemplate($directory . '/' . $template, $file). '<!-- comment -->’;</code><br> (Add a comment after the document)<br> Refresh the page, see the source at the bottom you should have <!-- comment --><br> You just overrided the render!</p> <p>Feature tracker: <a href="http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=34027">http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=34027</a></p> | ⇒ | <h1>Scope</h1> <p>In Joomla! if you want to override something you have to put it in the active template directory (e.g.: the layouts goes in (active template)/html/layouts).<br> This PR introduces the ability to override the whole rendering process in the template by copying (and modifying per your preferences) the /libraries/joomla/document directory.</p> <p>It shouldn't rise any backward compatibility issues (except that the directory [active template]/document might be in use?).</p> <h1>What you can do if this gets accepted?</h1> <p>Because renderers are the place where the code gets from arrays and strings to actual html, xml, raw, feed or json, you can do pretty much whatever you want.<br> Some examples of creative code that can be put in the renderers:<br> Put custom code in renderer html (eg minifier, remove <JDoc )<br> Create custom code for head (e.g.. scripts and css concatenation and minification, custom metas, scripts to bottom, etc)<br> Mess around with the error page and feed in creative ways<br> Or go all the way and get mustache or twig in the template</p> <h1>Why?</h1> <p>Because this is one area that if you want to override and use your own logic you have to do it through a plugin (this is way easier and cleaner)</p> <h1>Test instructions:</h1> <p>Apply the patch<br> assuming selected template: protostar<br> create directory named document in /templates/protostar<br> copy the folder html from /libraries/joomla/document to protostar/document<br> So you have /templates/document/html/html.php<br> edit html.php (in protostar foolder...)<br> In the last function _fetchTemplate()<br> replace the line before return so it will be like<br><code>$this->_template = $this->_loadTemplate($directory . '/' . $template, $file). '<!-- comment -->’;</code><br> (Add a comment after the document)<br> Refresh the page, see the source at the bottom you should have <!-- comment --><br> You just overrided the render!</p> <p>Feature tracker: <a href="http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=34027">http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=34027</a></p> <p>This one replaces PR <a href="https://github.com/joomla/joomla-cms/pull/4052" class="issue-link" title="[#34027] Add ability to override the renderer [New feature]">#4052</a> </p> |
A little one more: Take off space after parenthesis in file_exists( JPATH_THEMES .
Thanks
brianteeman
- | Labels |
Added:
?
|
||
nicksavov
- | Labels |
Removed:
?
|
||
| Status | New | ⇒ | Pending |
| Category | ⇒ | Libraries |
Successful test - thank you for the clear test instructions :)
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/4074.
Since the last merge we have some more logic on the class loading procedure:
- if the class exists, just take it
- elseif an override exists take that
- elseif the class file is found in libraries/joomla/document take that
- fallback to raw
Why?
At the moment all JDocument render classes have to be placed into "libraries/joomla/document". You cannot load your own JDocument child class with e.g. a plugin, because if getInstance does not find the class file in the mentioned path, it fallbacks to JDocumentRaw.
@bembelimen I think raw is the last resort, and thus the default library path should fit most cases. Also I cannot imagine doing something special in raw...
Travis says:
Whitespace found at end of line (303)
@wilsonge Can I ask for your UT superpowers here:
There were 3 errors:
1) JDocumentTest::testRetrievingAnInstanceOfTheHtmlDocument
Exception: Application Instantiation Error
/home/travis/build/joomla/joomla-cms/libraries/joomla/factory.php:122
/home/travis/build/joomla/joomla-cms/libraries/joomla/document/document.php:278
/home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/joomla/document/JDocumentTest.php:116
2) JDocumentTest::testRetrievingANonExistantTypeFetchesARawDocument
Exception: Application Instantiation Error
/home/travis/build/joomla/joomla-cms/libraries/joomla/factory.php:122
/home/travis/build/joomla/joomla-cms/libraries/joomla/document/document.php:278
/home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/joomla/document/JDocumentTest.php:124
3) JDocumentTest::testEnsureLoadRendererReturnsCorrectObject
Exception: Application Instantiation Error
/home/travis/build/joomla/joomla-cms/libraries/joomla/factory.php:122
/home/travis/build/joomla/joomla-cms/libraries/joomla/document/document.php:1021
/home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/joomla/document/JDocumentTest.php:562
--
There was 1 failure:
1) JDocumentTest::testEnsureLoadRendererThrowsException
Failed asserting that exception of type "Exception" matches expected exception "RuntimeException". Message was: "Application Instantiation Error" at
#0 /home/travis/build/joomla/joomla-cms/libraries/joomla/document/document.php(1021): JFactory::getApplication()
#1 /home/travis/build/joomla/joomla-cms/tests/unit/suites/libraries/joomla/document/JDocumentTest.php(575): JDocument->loadRenderer('unknown')
#2 [internal function]: JDocumentTest->testEnsureLoadRendererThrowsException()
#3 phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/Framework/TestCase.php(905): ReflectionMethod->invokeArgs(Object(JDocumentTest), Array)
#4 phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/Framework/TestCase.php(775): PHPUnit_Framework_TestCase->runTest()
#5 phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/Framework/TestResult.php(643): PHPUnit_Framework_TestCase->runBare()
#6 phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/Framework/TestCase.php(711): PHPUnit_Framework_TestResult->run(Object(JDocumentTest))
#7 phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/Framework/TestSuite.php(751): PHPUnit_Framework_TestCase->run(Object(PHPUnit_Framework_TestResult))
#8 phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/Framework/TestSuite.php(751): PHPUnit_Framework_TestSuite->run(Object(PHPUnit_Framework_TestResult))
#9 phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/Framework/TestSuite.php(751): PHPUnit_Framework_TestSuite->run(Object(PHPUnit_Framework_TestResult))
#10 phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/TextUI/TestRunner.php(423): PHPUnit_Framework_TestSuite->run(Object(PHPUnit_Framework_TestResult))
#11 phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/TextUI/Command.php(186): PHPUnit_TextUI_TestRunner->doRun(Object(PHPUnit_Framework_TestSuite), Array)
#12 phar:///home/travis/.phpenv/versions/5.3.29/bin/phpunit/phpunit/TextUI/Command.php(138): PHPUnit_TextUI_Command->run(Array, true)
#13 /home/travis/.phpenv/versions/5.3.29/bin/phpunit(605): PHPUnit_TextUI_Command::main()
#14 {main}.
Couple of things. First can you merge/rebase on master as there have been several changes to this test class (It will still fail - but if I make my changes based on the current class it will conflict later).
Secondly I have real issues with the PR itself. I really feel that template overriding classes is a very very dangerous precedent to set. Also overriding this class makes makes the expected behaviour unknown pretty much which is also a pretty dangerous thing to do - for that reason I'd really only like to see very experienced Joomla people doing that (and people who have that experience almost certainly can write a plugin to do the same thing).
As with all powerful stuff, great responsibility should be in place as well. The whole idea here is not to use a plugin for template stuff. And why I think this is crucial? If you depend on a plugin for your template, and the admin/user turns it off, there you go you have a non functional site...
yes i do mean staging :)
"As with all powerful stuff, great responsibility should be in place as well" Unfortunately in Joomla great responsibility isn't a phrase that turns up often :p We have 0 control over what all our users don't forget! It's not like we can regulate it.
Plus the next PR to allow a component override of the JMail class for com_contact comes through (think CMandrill or whatever) and they say well you did this to the template class before. In Joomla sometimes these things are half valid but it's still a really bad idea because of the precedent set for future work.
The final issue I'd say is that as soon as you install a 3rd party component - obviously it's not gonna be twig compatible and the user runs straight to the extension developer instead of doing the template override.
@bembelimen I think you should re open your PR, as I suspect this one won't have a chance
George I see your points and I have to admit they are valid. Although I strongly believe that templates are not just the css and the html mark up, but also prepare a lot of things in the background. Of course the majority can be done without even overriding something, but to override the html class itself you have to use a plugin. I guess I have to prepare a nicely post install script to lock this plugin, make it always published and prevent removal. It shouldn’t be that hard
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-11-17 21:46:20 |
I guess I have to prepare a nicely post install script to lock this plugin, make it always published and prevent removal.
I'm sure the user who tries your template will love that extension.
My templates will come with the motto good luck, you’re gonna need it. Ok I exaggerated a little bit here! On the removal part, because all the other parts are on the roadmap
I agree that supporting twig is something we want to do long term don't get me wrong. But I just think hacking like this is the wrong way to get there. We just need to completely redo JDocument for J4.x rather than hacking around it now.
Twig: doing that the right way needs some serious thought, architecture, and a lot of time. I didn’t expect those few lines would do that
Please correct code style
to