Here to help :)

Also a heads up that the current dev version of AdvAgg has had this working in a limited fashion for a little while now.
#2396609: Inline critical css

Comments

melon’s picture

melon CreditAttribution: melon commented

Thanks for stopping by, I'm a huge fan of advagg. The code pushed has a very basic feature set which actually relies on advagg currently. We've been using stable versions but not entirely happy with the CSS delivery. Looked through the issue and I personally do not see automating the discovery of critical CSS a feasible option just yet, although there are promising tools in JS land.

I first thought of going the manual way, allowing themes to declare CSS files to be inlined. The idea being that if one uses a modular framework in SASS or similar like we do then it's easy to generate a manual set of core styles.

Also improved the JS loader code but as I see now code in 7.x-2.x shares some ideas already on this.

mikeytown2’s picture

mikeytown2 CreditAttribution: mikeytown2 commented

Some of those JS tools look interesting. If you've used the advagg validator before then you know that it uses the browser to run linting tools for css/js. Thinking the same could be done here; put the tool in the footer like devel's query log output. Collect the data and then try to optimize the above the fold content.

The automatic way I was thinking was with the bundler enabled inline the first file; archaic but it might help to reduce the severity of the Flash of unstyled content. Currently with async all css in the footer enabled the FOUC is pretty bad.

I do like the idea of being able to specify the css for in lining inside the themes info file
settings[css_inline][{media}][] = path/to/styles.css
Just with the way css cascades you might end up with all the css being inline.

melon’s picture

melon CreditAttribution: melon commented

Automations in Drupal will always be a hindrance. If you try to collect the data and optimise for above the fold content you may end up with many bundles, essentially forcing the browser to download variations of the same css while trying to cater for all the different page types.

Also, using the first file could be completely useless if it's for example css from the base theme or some ill module. With scenarios like that Pagespeed takes away precious points because the inlined css isn't helping in rendering the page. You end up having the same 'penalties' as you have with normal
tags plus the FOUC.

Also, async loading should happen in the head IMO. Even non-critical css is important, for example any css styling js controlled bits, e.g a carousel should ideally be there before its js arrives.

melon’s picture

melon CreditAttribution: melon commented

Re cascading, it requires a certain approach to styling as any inline css will take precedence over external css so inlining a full css reset won't help for example :)
I've seen examples of inlining the entire stylesheet but that's rather extreme and could be useful for mobile sites mostly.

mikeytown2’s picture

mikeytown2 CreditAttribution: mikeytown2 commented

Inside the advagg_mod module you can "Inline CSS/JS on specific pages". So if you have a mobile landing page this can be useful; I've never used it on any projects I've done because its a big hammer. I should add an radio selection to this so you can select css, js, or both for inline (thinking inline css is more valuable).

I knew the first bundle idea wasn't very good thus I haven't worked on it. Was looking for a better alternative. Sometimes you have to put out a bad idea in order to get feedback and find the good ones. You have a lot of good ideas.

Async CSS in the header should be the default. Simple sites might benefit from pushing it down to the footer though, so keeping it as an option seems like the thing to do.

Thinking about this more and the only way to get this to work without a lot of crazy tricks is to inline the critical css and then also include it in the big aggregated css file. So we'll display the above the fold content styled correctly and then we'll load up the css for the entire page via async css as if we never included the inline css. This way helps to keep things simple; same bundles independent of the inline critical css. And looking at your code, this is what you currently do :)

In terms of making this module not require advagg, starting point would be to check if the advagg_load_css_stylesheet() function exists and then fallback to drupal_load_stylesheet() if it doesn't. Always be looking for ways to make this more accessible in the future. Although hook_advagg_modify_css_pre_render_alter() is key to this right now, it could change in the future as changes to how CSS/JS is done in D7 is still being worked on #1279226: jQuery and Drupal JavaScript libraries and settings are output even when no JS is added to the page.

With most of my code I try my best to make the minimum viable product work on shared hosting and then allow for better options. Did some research and using chrome you can get the critical path css with some simple javascript.
http://paul.kinlan.me/detecting-critical-above-the-fold-css/
https://gist.github.com/raulgundin/e2c1de33c8fdf30d7b16 <- modification of the script to include before and after selectors.
https://gist.github.com/ssafejava/6605832 <- fallback for non webkit browsers; crashed firefox so not usable IMHO at this point.

Using this I think the "auto" MVP could be achieved. Put this in the footer, put a $('html,body').scrollTop(0); in there so it jumps to the top and then issue an alert at then end letting the user know that the analysis is done; post/save that data back as well. Create a GUI for editing the critical CSS snippets and on what paths they go on with wildcard support (Just like the Inline CSS/JS on specific pages section).

Once the auto MVP is done, provide a non browser option using command line tools that should give better results.

melon’s picture

melon CreditAttribution: melon commented
Version: » 7.x-1.x-dev

Thanks for your valuable input.

Although hook_advagg_modify_css_pre_render_alter() is key to this right now, it could change in the future as changes to how CSS/JS is done in D7 is still being worked on #1279226: jQuery and Drupal JavaScript libraries and settings are output even when no JS is added to the page

First, I do not see how this could be beneficial for this, although I admit it's a very welcome feature.

I agree that if inlined css is auto-detected then it doesn't necessarily need removals from existing bundles, that's a lot of overhead. These gists look promising though, and I agree that an auto MVP could be possible with these, but it requires some initial setup.

But there are some caveats still, for example the question of when and where such discovery script should run on the page. If your CSS is already set to async, it shouldn't execute too soon to ensure vital css gets included. At the same time, if you run this too late you'll end up with 3rd party css not required for initial page rendering. The Chrome-only snippet looks like it could be enhanced easily, with some configurables, like whitelisting CSS hostname origins to exclude 3rd party clutter.

Then there is the problem of having certain types of pages which require a rather different inline stylesheet. One approach may be the stored CSS per path filter (e.g , taxonomy/term/2, etc) or to generate a merged subset for all required types of pages. Or possibly merge the collected styles and have a unified set for every page.

I reckon it would be best to add a "build" mode in browser where you have the script inserted and have the above the fold css collected as you visit pages. But a command line node.js-based tool could also be useful to properly configured build pipelines in larger scale deployments.

Anyways, I'll start evaluating options to see what's feasible.

mikeytown2’s picture

mikeytown2 CreditAttribution: mikeytown2 commented

The in browser tool looks like it has potential. Thanks for doing that!

Long term I think you'd want to have a database table that stores the URL, viewport resolution, & critical css. Overwriting the setting on submit has it's drawbacks. Using the menu_router table you can then see if the URL has a wildcard in it. In terms of entity URLs I've done an audit of all the drupal modules and created a function for that purpose. http://cgit.drupalcode.org/apdqc/tree/apdqc.module#n881

melon’s picture

melon CreditAttribution: melon commented

Great to hear you like it! It is fairly limited in terms of flexiblity. I made a command line version similar to this and also a grunt task but I'm waiting for the phantomjs-2.0.0 npm package to make it fully functional.

The current one critical css stored isn't very flexible. An example application was when I built several different CSS files from different sections on a site, saved them then I just ran awk '!a[$0]++' *.css > critical.css to store them in the theme. This is kind of a blunt solution, ideally I'd want to have this automated, here's when a command line tool comes handy.

I've been already thinking about serving different critical CSS on different paths or path patterns. I'm not sure if storing the viewport size has any benefits, not to mention that different node types may have completely different css required. To get to a less intuitive but more flexible solution I'm thinking of having critical css served via a context reaction or something similar.

mikeytown2’s picture

mikeytown2 CreditAttribution: mikeytown2 commented

Thanks for sharing that. We use phantomjs quite a bit for some other tasks so it's good to hear this could piggyback on that.

Viewport might come in handy for serving mobile vs desktop as that will change what CSS is critical. Have different css media queries for the inline css. Right now kinda useless but in the future it might be useful as the tools for this improve. Odds are it will be useless though so I wouldn't focus on this.

In terms of context, advagg can provide the context for what CSS is included on the page; in combination with some simple path pattern matching, that should provide a solution that covers about 90% of the use cases. For the other 10% when the CSS is the same & we have a url pattern match but the markup is different thus what is critical CSS is different, setting the context would then come into play. Using the bundle part from entities should help to refine this even more as different node types will render differently.