New CSS Loading idea

[Qphoria]

CSS files are currently being hardcoded into their corresponding tpl files.

This is bad because:
- It's not w3c valid to load CSS outside of the tags.
- Hardcoding is the Anti-Christ. Dynamic is the Savior. (yea UK & Aussies.. no "u" in savior)
- Limits future enhancements to the css system

One alternative to this would be to combine all css files into one main css file.
I dislike this idea because:
- It makes css files very hard to read & manage. Multiple CSS files is actually considered a good design idea from my research.
- It removes modularity, which is the antithesis of what we are trying to accomplish.
- It still makes it impossible to add enhancements like dynamically loading CSS files based on the page type.

So the idea here is to simply load ALL css files at the top. I know this has been done with the xhtml contrib that takes the output and manipulates it before printing to the page, but I'm not all that familiar with that one so I've created this method.

If the other method is better, then we can use that. So long as we use something.

This is how I have this working:

-------

The way I have it working is actually not making its own controller. I use the template library file, and added the orange lines:


library/template/template.php

class Template {
  var $data = array();

function __construct($directory) {
$this->directory = $directory;
$this->data['template'] = $directory;
$this->data['cssfiles'] = $this->getCSSFiles(DIR_TEMPLATE . $this->directory . '/css');
}
......
......

   
    function getCSSFiles($path)  {
        $delim = strstr(PHP_OS, "WIN") ? "\" : "/";
        if ($dir=@opendir($path)) {
            while (($element=readdir($dir))!== false) {
                if (is_dir($path.$delim.$element) && $element!= "." && $element!= "..") {
                    $array[$element] = retrieveTree($path.$delim.$element);
                } elseif ($element!= "." && $element!= "..") {
                    $array[] = 'catalog/template/' . $this->directory . '/css/' . $element;
                }
            }
            closedir($dir);
        }
        return (isset($array) ? $array : false);
    }
}
?>

Then change the layout file to load all css files at the top.

template/default/layout.tpl

Code: Select all

<link rel="stylesheet" type="text/css" href="catalog/template/<?php echo $this->directory; ?>/css/default.css">

to

Code: Select all

<?php foreach($cssfiles as $css) {?>
<link rel="stylesheet" type="text/css" href="<?php echo $css; ?>">
<?php } ?>


Finally, all hardcoded references to css files the existing templates would be removed.


This then:
- Loads all css files at the top in the tags.
- Allows for dynamic css files. Simply add a new file and it will load. No need for hardcoding or defining
- After all files get loaded to the top, there can be future enhancements for loading specific css based on the target page.
- Makes the site w3c validated.


Now, one point to make is that now you will load ALL files on pages that don't really need to be loaded. That's actually the step 2 of this, based on the idea in this post (see green text) to handle the proper loading based on the target.

[JNeuhoff]

Sounds good to me.

Just 2 points:

1) Not all CSS-files are needed all the time, certain CSS-files are only relevant for certain templates.
2) The goal is to make it W3C-compliant, so we might as well go through all the *.tpl and *.js files and fix them in other areas, too. Especially the radio and checkboxes need fixes in order to make it XHTML-compliant.

[hm2k]

I had that same issue with this CSS loader, and my conclusion was that if we were going to do it like this, we might as well merge all the css files into one file as it would take less time to load.

Perhaps something needs to be done to look for a css file that has the name of the controller, and load it if it's there, in addition to the default css file. I don't think any more than that is required. ie:

Code: Select all

<?php
$name=substr($this->file,0,strpos($this->file,'.'));
$css=$this->directory . '/css'.$name.'.css';
if (file_exists($css)) {?>
<link rel="stylesheet" type="text/css" href="<?php echo $css; ?>">
<php } ?>


Note: I've not worked out where to pull $this->file from yet.

At the moment they are HTML compliment, not XHTML. In future releases we had agreed to have a transition from HTML to XHTML as it seems more suitable.

[Qphoria]

Sounds good to me.

Just 2 points:

1) Not all CSS-files are needed all the time, certain CSS-files are only relevant for certain templates.

I mentioned that:

Now, one point to make is that now you will load ALL files on pages that don't really need to be loaded. That's actually the step 2 of this, based on the idea in this post (see green text) to handle the proper loading based on the target.

2) The goal is to make it W3C-compliant, so we might as well go through all the *.tpl and *.js files and fix them in other areas, too. Especially the radio and checkboxes need fixes in order to make it XHTML-compliant.

I'm with you on that one, but then I realized that OpenCart is actually HTML 4.01 based and tags like
are actually valid for html. A lot of my contribs I write using
and such, which actually throw an error on the validator for HTML. So at some point we need to make a conscious effort to really make the switch.

[bruce]

Why not simply use the xhtml contribution which does it all, as required with no waste.?

[Qphoria]

Why not simply use the xhtml contribution which does it all, as required with no waste.?

yes I referenced that in my post stating that i just didn't know much about it. whether it was bettes to do it post-processing like that or if it would be better to do it natively as it loads. if the contrib method is efficient then that works for me.

sometimes i like to reinvent the wheel to ensure it is still round

[hm2k]

Why not simply use the xhtml contribution which does it all, as required with no waste.?

We're not moving to XHTML yet. There's far more important things to deal with.

I decided to go with HTML in the first place because it's more compatible, and it was more consistent with the existing markup.

It's very likely that the XHTML contribution is now outdated.

But really, we need to stay on topic here.

We do plan on upgrading to XHTML, regardless of HTML 5.

[bruce]

In the shorter term and considering also what hm2k mentioned (except for the merging of all css files). You could add a public function to library\template\template.php that takes the css filename as a parameter and adds it to an attribute array in template.php. It would be similar in structure to the function Qphoria proposed.

This function could be called from the individual template files instead of adding the html code for the link.
eg: (from catalog search.tpl) and if the function name is "add_css()"

Code: Select all

<link rel="stylesheet" type="text/css" href="catalog/template/default/css/search.css" />


becomes

Code: Select all

<?php add_css('/css/search.css'); ?>

and whatever magic Qphoria has lined up to put the css definitions in the section of layout.tpl can take over from there.

[hm2k]

OPEN layout.tpl
FIND

Code: Select all

<link rel="stylesheet" type="text/css" href="catalog/template/<?php echo $this->directory; ?>/css/default.css">

ADD BELOW

Code: Select all

<?php foreach($this->css as $css) {?>
<link rel="stylesheet" type="text/css" href="<?php echo $css; ?>">
<?php } ?>


EXAMPLE OF USE
OPEN catalog/template/default/content/product.tpl
FIND

Code: Select all

<link rel="stylesheet" type="text/css" href="catalog/template/<?php echo $this->directory?>/css/product.css">

REPLACE WITH

Code: Select all

<?php
array_push($template->css,'catalog/template/'.$this->directory.'/css/product.css');
$template->set(array($template->css));
?>

The inner template is rendered first, then the layout, so it should work.

[Qphoria]

hmm.. I think you guys both might be on to something there. It's actually a good point that hm2k makes... since CSS files are "cascading", as long as the default is loaded first, the other specific files will override any spots that use the same parameters, and still only load when those files are required.

Gonna try this on my dev box

[hm2k]

Having looked into this a little bit more, I've come to realise that it's actually going to be too difficult to send something from a view template, to the layout template, because of the way the structure is designed, unless you use the database.

In other systems, i've seen a table for templates, and a table for template metas, in the meta table, you would see things such as css files that should be loaded when you use this template file.

This is of course one other method.

Does anyone have any further suggestions?

[bruce]

Yes.

There are two kinds of templates created.
1) A "master" template that is created in index.php and referenced via

Code: Select all

$template =& $this->locator->get('template');

in each controller.
2) Template objects used for views that are created via

Code: Select all

$view = $this->locator->create('template');

What is required is a reference in each template object to the "master" template

One way this could be achieved by adding (say) a createView() function to template.php so that the second call is replaced by

Code: Select all

$view = $template->createView();

and the createView function could place .a reference to the "master" template in the view object. This reference could be used by the functions we were discussing earlier in the thread.

Another way would be to add the createView() function to the locator and have it pass the reference to the "master" template as a parameter.

Either of these two mechanisms should prove to be more lightweight than using the database.

[hm2k]

Even what you could do, is instead of

Code: Select all

$view = $this->locator->create('template');

You could do

Code: Select all

$template->view &= $this->locator->create('template');

That would mean that the view variable is accessible from the $template variable, and thus you could pass things back and forth via that.

This is most defiantly something I will look into.

Note: As a quick fix, what "wsapplegate" did was simply remove all the css "links", and place them ALL into the layout.tpl instead. It's not very efficient this way, but it does mean the code is valid.

http://intarweb.goretsoft.net/~wsapplegate/misc/opencart/0.7.8/0001-catalog-valid-html.patch This patch fixes the default HTML templates to pass the W3C validator (mainly it's about LINK elements outside HEAD, and unescaped fields). NOTE: patch was for 0.7.7, I did not retest on 0.7.8

(note: DO NOT APPLY THIS PATCH to 0.7.9)

It might be an idea to apply this approach, until we figure out some code to master it, thoughts?

[Qphoria]

That's what I offered in a previous post, but was shunned for putting all css files on pages that don't require them.

[hm2k]

For performance it's not the best thing to do, however most people are on broadband these days, and the css files aren't that big anyway. The issue here is HTML validation not performance.

If we fix the issue by loading them all in the layout, it would fix the validation issue while we work on a better solution for better performance.

However, due to the nature of this issue, I propose to leave this until after the 0.7.9 release.

[Roysen]

I finally get default templates to XHTML validate without changing any other file than template files.

I use this code in "layout.tpl" too load css files:

Code: Select all

<?php
Sorry but not able to paste my code without error when posting.
?>


See www.nomorepasting.com for code.
Link to my  test shop.

[Qphoria]

ah! yes that looks like it's working properly!
But don't you also have to remove the existing reference from the controller tpl file?

Like product.tpl has a reference to product.css. I assume you need to remove that.

All the same, that is preferred and this makes it dynamic Very nice

[hm2k]

This will never be perfect, it's guess work.

[Roysen]

I removed reference to css in the tpl files. I also made this change in tpl files:
- replaced
with

- End all with
- Change with
- And some small change i don't remember

[Roysen]

This will never be perfect, it's guess work.

Not perfect but it work as long as css files has the same name as tpl files. It will automatic include the css file if it exisit and have the same name. Only problem is with files like review_info.tpl and review_write.tpl that have reference too two css files. And the code is easy!