controlling load order of stylesheets #1324

craigh · 2013-10-12T14:12:32Z

The load order of css stylesheets is buggy I believe. With the new "auto-loading" of assets from the /web directory, the order seems not to be correct or reliable. Somehow the order must be controlled so that classes override appropriately.

In Dizkus right now, the is the order:

<link rel="stylesheet" href="/core.git/src/web/font-awesome/css/font-awesome.min.css" type="text/css" />
<link rel="stylesheet" href="/core.git/src/web/bootstrap/css/bootstrap.min.css" type="text/css" />
<link rel="stylesheet" href="/core.git/src/style/core.css" type="text/css" />
<link rel="stylesheet" href="/core.git/src/style/legacy.css" type="text/css" />
<link rel="stylesheet" href="/core.git/src/modules/zikula-dizkus/Zikula/Module/DizkusModule/Resources/public/css/style.css" type="text/css" />
<link rel="stylesheet" href="/core.git/src/web/bootstrap/css/bootstrap-theme.min.css" type="text/css" />
<link rel="stylesheet" href="/core.git/src/themes/Zikula/Theme/Andreas08Theme/Resources/public/css/fluid960gs/reset.css" type="text/css" />
<link rel="stylesheet" href="/core.git/src/themes/Zikula/Theme/Andreas08Theme/Resources/public/css/fluid960gs/grid.css" type="text/css" />
<link rel="stylesheet" href="/core.git/src/themes/Zikula/Theme/Andreas08Theme/Resources/public/css/style.css" type="text/css" />
<link rel="stylesheet" href="/core.git/src/system/Zikula/Module/BlocksModule/Resources/public/css/style.css" type="text/css" />
<link rel="stylesheet" href="/core.git/src/system/Zikula/Module/BlocksModule/Resources/public/css/extmenu.css" type="text/css" />
<link rel="stylesheet" href="/core.git/src/system/Zikula/Module/SearchModule/Resources/public/css/style.css" type="text/css" />

IMO, the core.css and legacy.css should probably load before bootstrap.min.css and font-awesome.min.css but the bigger problem is that the theme reset.css overrides many items in bootstrap.min.css and causes problems.

Anyway - I don't really understand how css load order works exactly, so I cannot solve this and it seems broken but maybe I just don't understand. Can someone with the knowledge and skill take a look?

@cmfcmf @phaidon @jusuff @rojblake

The text was updated successfully, but these errors were encountered:

ghost · 2013-10-12T14:15:47Z

legacy definitely MUST come after. It needs to provide legacy override of bootstrap. Same for core.css I believe.

craigh · 2013-10-12T14:18:32Z

the bigger problem is the reset.css overriding bootstrap.

espaan · 2013-12-07T08:29:56Z

Isn't this a blocker then ? Since loading reset after the rest can really mess up the layout .

I was checking PageUtil in Zikula Core 1.3.6 and there is around line 329

        if (is_array($value)) {
            $value = array_unique($value);
        }

In Core 1.3.7 it is https://github.com/zikula/core/blob/1.3/src/lib/util/PageUtil.php#L350
this checks for unique entries, BUT as I read on http://php.net/function.array-unique it also does a sort. That could mess up things probably.

Several comments on the PHP page are interesting, I think the foreach way will not mess up the order, but only remove the duplicates. But the array_flip method is quite elegant

<?php 
$unique = array_keys(array_flip($array)); 
?>

and substantially faster than array_unique as it seems, which for us is not that important, since it's usually only a small array, but speed gain is speed gain.

@rgash can you test if that helps in your case ?

espaan · 2013-12-12T12:22:51Z

As suggested also in this topic http://community.zikula.org/index.php?module=Forum&func=viewtopic&topic=59641&start=0

it might be an idea to add a weight or priority to the pageaddvar call. So that certain parts can always be loaded with high prio and others with lower or default prio

rgasch · 2013-12-15T00:06:30Z

This seems really broken in 1.3.5/6: If I look at the order to style sheets loaded under Andreas08 then I get this:

<link rel="stylesheet" href="style/core.css" type="text/css" />
<link rel="stylesheet" href="modules/ZWebstore/style/style.css" type="text/css" />
<link rel="stylesheet" href="system/Admin/style/style.css" type="text/css" />
<link rel="stylesheet" href="modules/ZWebstore/style/bootstrap.css" type="text/css" />
<link rel="stylesheet" href="modules/ZWebstore/style/bootstrap-custom.css" type="text/css" />
<link rel="stylesheet" href="modules/ZWebstore/style/jquery.tipTip.css" type="text/css" />
<link rel="stylesheet" href="themes/Andreas08/style/fluid960gs/reset.css" type="text/css" />
<link rel="stylesheet" href="themes/Andreas08/style/fluid960gs/grid.css" type="text/css" />
<link rel="stylesheet" href="themes/Andreas08/style/style.css" type="text/css" />

Removing the array_unique() calls has no effect on the order, so it seems that this is just the result of the order in which the relevant sections are loaded/processed.

It would be easy to fix this for Andreas08, but a general solution will require some changes in the way this is handled. IMHO, a priority setting would be good, although it might suffice to simply order the priorities based on the section from which a certain CSS is loaded:

Theme
Admin
Module Standard CSS
Module Custom CSS (via pageaddvar)

Right now it almost seems that things are backwards, i.e.: that theme CSS is added last when in fact it should be added first.

espaan · 2013-12-15T09:43:19Z

Just to add, if you do a pageaddvar via a module to load jQueryUI/jQuery for instance where this is normally not loaded. Then it would be nice if that would not load last, but also somewhere in front. If the theme does not load it already.

rgasch · 2013-12-15T12:18:42Z

Well, if we assume that each section (see above for sections 1,2,3,4) loads it's stylesheets (and JS) in the correct order (and that would seem a safe assumption), then a simple duplicate check will get rid of that problem.

So if your theme (or core) loads jQuery and the module attempts to load jQuery as well, then the module load command should be ignored. This would however mean that we need some kind of component key (ex: 'jQuery' for jQuery) since the actual files we're attempting to load might not have the same path, filename, etc.. This is how some other systems (WP, Typo3, etc) do it and it's a smart thing to do since it avoids loading multiple versions of the same file (or multiple versions of different versions of the same component).

craigh · 2013-12-15T12:21:11Z

pageaddvar already does duplicate checking.

jQuery is loaded by default all the time in Core 1.3.7 atm. as is Bootstrap because both are integral to the core.

rgasch · 2013-12-15T12:26:41Z

That's great news, but lets make sure that we don't have the same mess in
1.3.7 that we have now: it currently seems that theme stylesheets are
loaded last, and sorry for saying so, but that's just stupid. So lets make
sure we get the loading order right. See my previous comment about a
component (or loading/processing stage, for a better word) approach in
order to avoid this mess.

The problem though remains that this is seriously broken in 1.3.6 and
probably not easy to fix. :-(

On Sun, Dec 15, 2013 at 1:21 PM, Craig Heydenburg
[email protected]:

pageaddvar already does duplicate checking.

jQuery is loaded by default all the time in Core 1.3.7 atm. as is
Bootstrap because both are integral to the core.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/1324#issuecomment-30603415
.

matheo · 2013-12-15T16:53:30Z

I disagree.

Theme is last because it's the place for the site's customization.
If there's a conflict between the Theme stylesheets and a module that's a different story, anyways you have !important and different ways to override Theme stuff from a Module. Andreas08 should be replaced with the Bootstrap theme as default, because it's awkward to use two frameworks at the same time (960gs and Bootstrap).

craigh · 2013-12-15T17:34:06Z

I agree with @matheo wrt order - this is what I posted in the referenced forum post:

reset
core
module
theme

this is much harder than you might think however to control from what I can see.

ghost · 2013-12-15T17:35:58Z

from reading the discussions, it would seem that the entire methodology is flawed - that is, having random modules attempting to control the environment and vie for dominance. We need a better strategy for the long term.

craigh · 2013-12-15T17:39:43Z

agreed. I believe that the theme is the bigger issue though in terms of vying dominance.

perhaps stylesheets and site-wide js could be 'registered' via an event or something (dependency injection?) and then managed with some intelligence but ultimately configurable via some admin interface...

ghost · 2013-12-15T18:41:50Z

I'll tell you my thoughts based on the difficulties I have been having integrating our way of doing things into Twig, Assetic and Symfony. (hint: I am working on an end-game prototype).

Under our paradigm we have modules making output, then we have the theme which is a completely separate entity which uses module output as a variable. Top that of with random blocks. The theme is the dominant factors (leaving aside from custom config level overrides).

The only say a module has over the theme is to prevent a theme from loading at all (by returning true in the old ways, or a PlainResponse). Here is the first code smell: a theme cannot work in all circumstances. Classic example is ajax output - that must be non-themed, so we developed a separate front controller for it and used the return true trick. More elegantly we can check the Request with isXmlHttpRequest() and decide not to load the theme.

In Symfony's paradigm of course, developing single applications, bundles will simply extend a base theme template thus the module's output and the theme are in fact the same template. If the bundle chooses, it can extend any base template it chooses - doesnt have to be the system wide theme.

This is problematic for us because we have systems with multiple themes. But multiple themes are still a problem because it takes power away from modules. We make assumptions about which theme to use based on random stuff like type=admin - all of which has been shown over time to be problematic - it doesn't scale and it doesn't provide real flexibility.

What we have are contracts that modules need to follow (like using certain style-sheet conventions) and assumptions that all modules talk to each-other (when actually they cant) - so what actually evolved over time are modules that are coupled to each other's paradigm. The fact is you cant really have independent modules (or blocks).

So what we have now is the core which forces certain sheets/js and modules/theme registering their own all making assumptions. Ultimately it's quite possible a block and a module conflict with each other. Not to mention script order.

So basically we have a theme which is supposed to be the leader, except we hack it in multiple cases (like ajax requests) to not display at all. We have to couple the theme to individual module templates if we want to do something radical, thus creating coupling to modules. So what if someone releases a new module that also needs that radical styling - you are suddenly stuck until the theme author updates it or you have the savvy to update it yourself. Clearly the whole "generic theme" concept is quite flawed. If Zikula is more like a web developer tool - that's fine, but if so then there need to be more programmatic ways of securing what you want.

Otherwise, some kind of registry as @craigh says, but that could be a performance hog - somehow, Maybe some kind of hard values in the theme - but then you have to configure per theme. It would make module installation complex. I don't really know... but these are my thoughts. We need a solution.

ping @phaidon @cmfcmf

craigh · 2014-06-12T18:12:10Z

should this be bumped to 1.5.0?

Guite · 2014-06-12T18:13:06Z

Yes, we should think about it together with #1753

phaidon · 2014-06-23T18:50:03Z

I think the order is ok. imho it should be like it is:

core
module
theme

If reset.css is still useful then lets add it to the core.

But do we really still need reset.css? Does not bootstrap do this job?

shefik · 2014-06-23T18:55:01Z

I think we should deprecate the use of reset.css, since Bootstrap does what we need.

craigh · 2014-06-23T19:22:57Z

remember, 1.4.0 is a BC release and as such, all current 1.3.x modules should look the same in 1.4.0

phaidon · 2014-06-23T20:19:12Z

remember, 1.4.0 is a BC release and as such, all current 1.3.x modules should look the same in 1.4.0

The question is more what is reset.css doing? If I understood it right it sets default css values, which are different in mozilla, chrome, explorer, ....
See: http://meyerweb.com/eric/tools/css/reset/

But I think that bootstrap is doing that as well.

shefik · 2014-06-23T20:38:16Z

Bootstrap uses normalize.css, so reset.css is not needed.

http://nicolasgallagher.com/about-normalize-css/

craigh · 2014-06-23T20:45:52Z

cool. so remove it locally and start testing 😉

rgasch · 2014-06-23T22:38:07Z

Sorry, I think this order is not good. IMHO it should be:

Core
Theme
Module

or

Theme
Core
Module

Either way, IMHO module should be last.

On Mon, Jun 23, 2014 at 8:50 PM, phaidon [email protected] wrote:

I think the order is ok. imho it should be:

core

module

theme

If reset.css is still useful then lets add it to the core.

But do we really still need reset. Does not bootstrap do this job?

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

craigh · 2014-06-23T23:18:22Z

As was discussed earlier in this ticket, the Theme should be last as it is the place for customization.

shefik · 2014-06-24T00:20:24Z

Please keep the order as:

core
module
theme

phaidon · 2014-06-24T06:22:23Z

I agree @craigh and @shefik

The theme must be able to override core and modules. That is the sense of a theme. It must be after core and modules. In addition this would be a massive bc.

So I suggest to close this issue and remove reset.css.

craigh · 2014-06-24T11:59:48Z

this is the current load order in dizkus at the ntq site

<link rel="stylesheet" href="/core/src/web/font-awesome/css/font-awesome.min.css" type="text/css" />
<link rel="stylesheet" href="/core/src/themes/Zikula/Theme/ModernBusinessTheme/Resources/public/css/spacelab.min.css" type="text/css" />
<link rel="stylesheet" href="/core/src/style/core.css" type="text/css" />
<link rel="stylesheet" href="/core/src/style/legacy.css" type="text/css" />
<link rel="stylesheet" href="/core/src/modules/dizkus-module/Zikula/Module/DizkusModule/Resources/public/css/style.css" type="text/css" />
<link rel="stylesheet" href="/core/src/themes/Zikula/Theme/ModernBusinessTheme/Resources/public/css/style.css" type="text/css" />
<link rel="stylesheet" href="/core/src/system/Zikula/Module/BlocksModule/Resources/public/css/style.css" type="text/css" />
<link rel="stylesheet" href="/core/src/system/Zikula/Module/BlocksModule/Resources/public/css/menutree/horizontal.css" type="text/css" />

is this right?

craigh · 2014-06-24T12:00:36Z

@Guite do you think this should be closed yet?

Guite · 2014-06-24T12:06:00Z

I think it should be reviewed in 1.5 wrt Twig and Drak's comments above.

phaidon · 2014-06-24T14:13:43Z

I changed the milestone to 1.5

…r.refs #2596, #1324

craigh · 2015-10-03T20:37:19Z

This ticket is closed with the implementation of an asset weighting system which can be used to control the load order of either stylesheets or javascripts in #2606.

craigh mentioned this issue Jan 8, 2014

1.4.0 Release task list #1540

Closed

23 tasks

phaidon modified the milestones: 1.5.0, 1.4.0 Jun 24, 2014

phaidon pushed a commit to phaidon/core that referenced this issue Jun 24, 2014

reset.css removed, refs zikula#1324

a33a5cb

This was referenced Jun 24, 2014

[RFC] reset.css removed, refs #1324 #1808

Closed

Remove reset.css #1819

Closed

Guite removed the Critical label Oct 4, 2014

Guite modified the milestones: 1.5.0, 2.0.0 Dec 22, 2014

craigh mentioned this issue Jan 8, 2015

[1.3][1.4] JCSSUtil.php pageaddvar prototype/jquery order and file naming #2170

Closed

craigh added a commit that referenced this issue Sep 23, 2015

add 'weight' feature to page assets to allow basic management of orde…

acdceae

…r.refs #2596, #1324

craigh mentioned this issue Sep 23, 2015

Improve Asset Management #2606

Merged

craigh added a commit that referenced this issue Sep 25, 2015

add 'weight' feature to page assets to allow basic management of orde…

bc179e4

…r.refs #2596, #1324

craigh added a commit that referenced this issue Sep 25, 2015

add 'weight' feature to page assets to allow basic management of orde…

e865538

…r.refs #2596, #1324

craigh added a commit that referenced this issue Sep 27, 2015

add 'weight' feature to page assets to allow basic management of orde…

c9cb3b2

…r.refs #2596, #1324

craigh added a commit that referenced this issue Sep 30, 2015

add 'weight' feature to page assets to allow basic management of orde…

9ad31c4

…r.refs #2596, #1324

craigh added a commit that referenced this issue Oct 1, 2015

add 'weight' feature to page assets to allow basic management of orde…

02e2c2d

…r.refs #2596, #1324

craigh modified the milestones: 1.4.1, 2.0.0 Oct 3, 2015

craigh closed this as completed Oct 3, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

controlling load order of stylesheets #1324

controlling load order of stylesheets #1324

craigh commented Oct 12, 2013

ghost commented Oct 12, 2013

craigh commented Oct 12, 2013

espaan commented Dec 7, 2013

espaan commented Dec 12, 2013

rgasch commented Dec 15, 2013

espaan commented Dec 15, 2013

rgasch commented Dec 15, 2013

craigh commented Dec 15, 2013

rgasch commented Dec 15, 2013

matheo commented Dec 15, 2013

craigh commented Dec 15, 2013

ghost commented Dec 15, 2013

craigh commented Dec 15, 2013

ghost commented Dec 15, 2013

craigh commented Jun 12, 2014

Guite commented Jun 12, 2014

phaidon commented Jun 23, 2014

shefik commented Jun 23, 2014

craigh commented Jun 23, 2014

phaidon commented Jun 23, 2014

shefik commented Jun 23, 2014

craigh commented Jun 23, 2014

rgasch commented Jun 23, 2014

craigh commented Jun 23, 2014

shefik commented Jun 24, 2014

phaidon commented Jun 24, 2014

craigh commented Jun 24, 2014

craigh commented Jun 24, 2014

Guite commented Jun 24, 2014

phaidon commented Jun 24, 2014

craigh commented Oct 3, 2015

controlling load order of stylesheets #1324

controlling load order of stylesheets #1324

Comments

craigh commented Oct 12, 2013

ghost commented Oct 12, 2013

craigh commented Oct 12, 2013

espaan commented Dec 7, 2013

espaan commented Dec 12, 2013

rgasch commented Dec 15, 2013

espaan commented Dec 15, 2013

rgasch commented Dec 15, 2013

craigh commented Dec 15, 2013

rgasch commented Dec 15, 2013

matheo commented Dec 15, 2013

craigh commented Dec 15, 2013

ghost commented Dec 15, 2013

craigh commented Dec 15, 2013

ghost commented Dec 15, 2013

craigh commented Jun 12, 2014

Guite commented Jun 12, 2014

phaidon commented Jun 23, 2014

shefik commented Jun 23, 2014

craigh commented Jun 23, 2014

phaidon commented Jun 23, 2014

shefik commented Jun 23, 2014

craigh commented Jun 23, 2014

rgasch commented Jun 23, 2014

craigh commented Jun 23, 2014

shefik commented Jun 24, 2014

phaidon commented Jun 24, 2014

craigh commented Jun 24, 2014

craigh commented Jun 24, 2014

Guite commented Jun 24, 2014

phaidon commented Jun 24, 2014

craigh commented Oct 3, 2015