search

ndeet / plg_system_less

A Joomla! system plugin to compile .less files on the fly.
GNU Lesser General Public License v3.0
17 stars 13 forks source link

Client-side compiler Feature #10

Closed piotr-cz closed 11 years ago

piotr-cz commented 11 years ago

This patch adds possibility to use Client-side compilers:

This helped me a lot when I started with Bootstrap to find out where exactly css selectors are present. At the moment feature has only one option: Basic Options > Client-side Compiler Options > Use client-side compiler: On/ Off, doesn't have updated language strings and may require some code tweaks/ optimization.

Note The patch adds javascript compiler script and removes template.css (or any configured template file) from <head />. For this to work, template file (template.css) must to be added in template either using JFactory::getDocument()->addStyleSheet($url); or JHtml::_('stylesheet', $url);. Theoretically it's possible to parse JResponse output and remove stylesheet from html, but I left it out at the moment to not overcomplicate.

Preview FireLESS example

ndeet commented 11 years ago

Hi Piotr!

Thank you very much for your huge contribution! I did not even know these tools existed. I will check them out, play with it and merge your pull request asap. Thanks a lot!

ndeet commented 11 years ago

Ok I tried it out and it is really cool to that you see exactly in which .less file the style is defined. This is really a great helper and it makes totally sense to me to include this. Thank you @piotr-cz !

I did some testing. This works well with Firefox and installed FireLess addon. It takes some time for initial compiling and loading (because less.js has to generate the .css files and such).

On my OSX with Chrome 27 and chrome developer tools it does not work. It seems that it has been working for Chrome 24-26 but does not anymore. It seems that Chrome switched to Sourcemaps V3 but this is not (yet) implemented in less.js. See this links comments section, comment from Paul Irish regarding sourcemaps v3. http://robdodson.me/blog/2012/12/28/debug-less-with-chrome-developer-tools/

So Firefox works perfectly, for Chrome it is not working, yet.

Thank you!

ndeet commented 11 years ago

just for reference, seems that Chrome removed -sass-debug support but there is the sourcemap support on the way: https://github.com/cloudhead/less.js/issues/1345

piotr-cz commented 11 years ago

Hi, Thanks for the plugin, it would be really hard to jump on Bootstrap without it.

Few notes on client-side feature:

Klipper commented 11 years ago

Hi, I tested the new client-side features in the Less Compiler plugin together with FireLess on Firefox (Windows 7 with latest Firefox and Firebug). This is really really great!!!

I have two comments:

1.) Maybe you should advise users to temporary exclude (in index.php) the template.css from loading, even if you do not compile on server, in case you had an already compiled template.css in your template directory. If you don't do, you will get the template.css css rules and the .less file css rules in Firebug.

2.) Directory "media/jui/less" should be accessible! For users who also use Akeeba Admin Tools .htaccessmaker : at "Allow direct access, except .php files, to these directories" one should add: "media/jui/less". If you don't do, you will get 403 errors for these .less files stored in "media/jui/less". Of course this is not only important for Admin Tools users, but for all access-restriction tools and/or custom made .htaccess files.

Andreas and piotr-cz Thanks a lot for these new features!!!

ndeet commented 11 years ago

Hi Klipper,

thanks for your suggestions!

1) I see in the source code comments that removing the template.css does not work in admin templates, yet. Are you using the client-side features on admin template? For me it is working on frontend templates - so there is no template.css anymore.

See https://github.com/ndeet/plg_system_less/blob/master/less.php#L254 and https://github.com/ndeet/plg_system_less/blob/master/less.php#L285

@piotr-cz Is it correct that the removeCSS function on Line 285 does nothing yet. As you return; after //TODO comments. So this function should then pick all .css links left and remove them? So in admin the unset of the .css array does not work and we have to strip the .css links with regex?

piotr-cz commented 11 years ago

Thanks for testing, @Klipper! 1) I'm aware of this issue. If you look at my first post, I suggest that the stylesheets should be added to JDocument instance, so using methods like JFactory::getDocument()->addStyleSheet($url); or JHtml::_('stylesheet', $url);.

This has an advantage over using Html markup: It's easier to work with registered assets, concatenate or minimize them in other extensions.

I prefer removing stylesheet file using plugin, because when you are ready for production, you just switch off the client-side compiler and don't have to add stylesheet back to the template manually.

However, how @ndeet suggested, when plugin is not able to remove the stylesheet from JDocument instance, there's fallback to parse HTML output. I've disabled it this part, because I didn't need it and I could not come with all scenarios to test it out properly.

So, if it's more natural for developers to add stylesheets in tempalte using <link rel="stylesheet" href="" /> (like in Joomla admin templates), let's enable and test it, but maybe we will have to come up with better regex pattern.

piotr-cz commented 11 years ago

What about this?

public function removeCss()
{
    // Initialise variables
    $doc = JFactory::getDocument();

    // Get Uri to template stylesheet file
    $templateUri = JUri::base(true) . 'templates/' . $doc->template . '/';
    $cssUri = $templateUri . $this->params->get('cssfile', 'css/template.css');

    // Replace line with link element and path to stylesheet file
    $replaced = preg_replace( '~(\r\n?\s*?<link.* href=".*?' . $cssUri . '".*/>)~', '', $path);

    // ...

Pattern doesn't check for attribute rel="stylesheet" (may be before or after href), but that shouldn't be a problem. Attribute type="text/css" is not mandatory in HTML5 so I omited that

I'll make a new pull request with that code, have found another problem in the code (JUri::base(true)).

Klipper commented 11 years ago

Hi Andreas and Piotr,

I've tested the clientside compiler on frontend templates, not on admin templates. I found out why the template.css was not removed from my header automaticly. I use next code for calling the template.css in index.php:

JFactory::getDocument()->addStyleSheet('templates/'.$this->template.'/css/template.css?'. round(microtime(true) * 1000));

The added time-stamp is the reason why template.css is not automatically removed when using client-side compiler. I added the time-stamp because my hostprovider is using a generic cachingsystem (not Joomla cache) for static files. I've disabled this cache in .htaccess file, but the server ignores the command :-( Using this time-stamp I can bypass this cachesystem.

When I remove the time-stamp argument from the template.css in index.php the template.css becomes removed from the header correct!.

So it would be nice if css/template.css will also be removed when there are any arguments added in index.php. But maybe I should contact my hostprovider to let me bypass his cache by adding commands in .htaccess. ;-)

piotr-cz commented 11 years ago

Hi @Klipper. If you look at the line 269, there's note that code doesn't process stylesheets with added query string.

I didn't have to use same trick as you on localhost so I left it like that, but I'm having same problem as you on one of the hostings and I fixed it just like you did. I'll see what I can do about it.

ndeet commented 11 years ago

@piotr-cz Thank you, the removeCss looks good. 

    $cssUri = $templateUri . $this->params->get('cssfile', 'css/template.css');

Default param, why not only template.css (without css subfolder)? Altough using css subfolder is common and may be 99,9% the case.

    // Replace line with link element and path to stylesheet file
    $replaced = preg_replace( '~(\r\n?\s*?<link.* href=".*?' . $cssUri . '".*/>)~', '', $path);

I'm no css expert but maybe we should search for <link and .css and strip that line. Or as you mentioned we search only for <link and rel="stylesheet" and remove it? I really have to take a look on regex and play with it. Maybe I can come up with a regex for above mentioned cases.

Thanks Piotr!

piotr-cz commented 11 years ago

@ndeet

  1. I used same fallback value as in extension manifest: cssfile and in onBeforeRender method. Actually I'm not sure that the specified fallback value is ever used. When parameter doesn't have any value, 'default' should be used.
  2. I'm not sure I understand you. We should remove only the configured css file. There might be other stylesheets registered by extensions and those should be left alone.

I added pull request that should deal with aforementioned issue. At least it works for me.

ndeet commented 11 years ago

@piotr-cz 1) Ha, thanks for the hint and sorry - this was my own assumption of a default css path :smile:

2) Ok I understand now, I thought we have to pull out all .css files. Removing only the template.css for which we get the references to template.less file makes sense. Thanks for clarification.

Thank you very much for your new pull request - looks really great.

@Klipper @piotr-cz also added support for your query string :+1: