? Success

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
25 Aug 2014

The system component.php file which is only used as a fallback if the site template doesnt have one attempts to include up to 2 css files that do not exist (as reported here #4174 )

This PR removes the unneeded code

avatar brianteeman brianteeman - open - 25 Aug 2014
avatar jissues-bot jissues-bot - change - 25 Aug 2014
Status Pending New
Labels Added: ?
avatar brianteeman
brianteeman - comment - 25 Aug 2014

Its kind of tricky to test this as its rarely used. The only thing I can think of is
1. remove the component.php file from protostar so that the system template one is used
2. open a "email this article" link
3. view source and you will see the call to load template.css (which doesnt exist)
4. Apply this pull request
5. open a "email this article" link
6. view source and you will see the call to load template.css has been removed

avatar Hackwar
Hackwar - comment - 25 Aug 2014

I would prefer if we were using the API calls to include the CSS files in all core templates. That would allow plugins to also react on those CSS files more easily and it would result in a cleaner head. It's actually bad style from our side that we don't do that.

avatar Bakual
Bakual - comment - 25 Aug 2014

You can also test with any URL by appending ?tmpl=component to the URL. This will load the component.php instead of the index.php

Btw: The same thing is present in Joomla 2.5.

I would prefer if we were using the API calls to include the CSS files in all core templates.

I don't think so and I think it's actually bad style to use the API within the template. The template is the highest ruler for output and styling. There should be no way that you can remove or alter template CSS rules from an extension. Also the template CSS has to be loaded after all the head is compiled (<jdoc:include type="head" />) so it can override extension output and not vice versa.

But it's not the topic of this PR anyway :smile:

avatar Hackwar
Hackwar - comment - 25 Aug 2014

The order of the files in the header depends on the order that the files are added via the API. Since the template is the last piece of code to be executed before compiling the head, its the last script and/or style that is added to the buffer and thus adding it via the API is the right thing to do. Not using the API means, that you have to use some regex-voodoo on onAfterCompileHead (or whatever its called) to then get all the CSS or JS from the head to minify it and all that stuff. I try to not invoke a regex engine if possible.

avatar brianteeman
brianteeman - comment - 25 Aug 2014

Please can we stick to the topic.

On 25 August 2014 13:13, Hannes Papenberg notifications@github.com wrote:

The order of the files in the header depends on the order that the files
are added via the API. Since the template is the last piece of code to be
executed before compiling the head, its the last script and/or style that
is added to the buffer and thus adding it via the API is the right thing to
do. Not using the API means, that you have to use some regex-voodoo on
onAfterCompileHead (or whatever its called) to then get all the CSS or JS
from the head to minify it and all that stuff. I try to not invoke a regex
engine if possible.


Reply to this email directly or view it on GitHub
#4177 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar Hackwar
Hackwar - comment - 25 Aug 2014

will try. When this one is accepted, I will create a PR that uses the API instead of hardcoding it.

avatar dgt41
dgt41 - comment - 26 Aug 2014

@Hackwar @brianteeman that's why I propose an easy way to override the renders (head is the one you have to work out to make minification concatenation etc). But I agree with Hannes here: as is right now the joomla API, it is better everything to go through it. I think my PR makes templating very powerful if you invest some time to organize the logic for metas, CSS and js, and is also just 1 file lookup so it won't heart performance #4074

avatar brianteeman
brianteeman - comment - 26 Aug 2014

That comment is not relevant to the topic of this issue

On 26 August 2014 13:28, dgt41 notifications@github.com wrote:

@Hackwar https://github.com/Hackwar @brianteeman
https://github.com/brianteeman that's why I propose an easy way to
override the renders (head is the one you have to work out to make
minification concatenation etc). But I agree with Hannes here: as is right
now the joomla API is better everything to go through it. I think my PR
makes templating very powerful if you invest some time to organize the
logic for metas, CSS and js, and is also just 1 file lookup so it won't
heart performance #4074 #4074


Reply to this email directly or view it on GitHub
#4177 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar dgt41
dgt41 - comment - 27 Aug 2014

tested ok! Sorry for hijacking this PR earlier

avatar b2z
b2z - comment - 31 Aug 2014

@dgt41 now it is possible to submit your test results using issues.joomla.org. Thank you :)

This comment was created with the J!Tracker Application at http://issues.joomla.org/.

avatar phproberto phproberto - change - 31 Aug 2014
Status Pending Ready to Commit
avatar brianteeman brianteeman - change - 31 Aug 2014
Status New Pending
avatar brianteeman brianteeman - change - 2 Sep 2014
Category Template
avatar phproberto phproberto - reference | - 3 Sep 14
avatar brianteeman brianteeman - change - 3 Sep 2014
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2014-09-03 22:30:55
avatar brianteeman brianteeman - close - 3 Sep 2014
avatar brianteeman brianteeman - close - 3 Sep 2014
avatar phproberto
phproberto - comment - 3 Sep 2014

Merged. Thanks!

avatar brianteeman brianteeman - head_ref_deleted - 3 Sep 2014
avatar Sophist-UK Sophist-UK - reference | 29f47ff - 7 Oct 14

Add a Comment

Login with GitHub to post a comment