filamentgroup / grunt-criticalcss

Grunt wrapper for criticalcss
MIT License
530 stars 27 forks source link

Allow multiple URLs in options.url #22

Closed manuelbieh closed 1 year ago

manuelbieh commented 9 years ago

Since you don't know on which (sub)page a user will enter your site I think it might be useful to be able to specify a set of URLs to generate a critical.css from.

In case of a blog you might want to have something like

options: {
  url: [
    'http://my.blog.sexy/home.html',
    'http://my.blog.sexy/2015/03/11/sample-article.html'
  ]
}

That way a user always sees styled output no matter if he's landing on your homepage or coming via deeplink. Or do I get the idea of criticalcss wrong?

Sure, you could create multiple critical css files but I'm not sure if it is always useful to have 2-n 15k CSS files instead for each pagetype instead of a single critical css which might be a bit larger in filesize (maybe 20 or 25k).

scottjehl commented 9 years ago

Hmm. I'm not sure if this will answer your question but we recommend generating a different critical css file for each unique template on our site (home, article, utility, contact, etc). Each template can then include the file relevant to it. Since these files are intended to be inlined in the HTML, we think it's best to keep them as small and relevant to the page as possible. A typical site we build has 10-15 unique templates but this of course varies.

here's an example setup for a couple templates:

criticalcss: {
  home: {
    options:  {
      outputfile : 'css/critical/critical-home.css',
      filename : 'all.css',
      url : 'http://fgwebsite.local'
  }
  },
  services: {
    options:  {
      outputfile : 'css/critical/critical-services.css',
      filename : 'all.css',
      url : 'http://fgwebsite.local/services/'
    }
  },
  about: {
  ...

I hope that helps!

manuelbieh commented 9 years ago

Yep, that's what I meant by saying "you could create multiple critical css files" ;)

I'm not sure if it is worth the effort to create a critical-css for each template when 90-95% of the critical css might possibly always be the same.

Also: I don't know how grunt-criticalcss works under the hood but being able to specify only one width and one height doesn't seem to be the luckiest solution(?).

For example, what happens when I have the following config:

  example: {
    options:  {
      outputfile : 'css/critical/critical-main.css',
      filename : 'all.css',
      width: 1200,
      height: 768,
      url : 'http://mywebsite.local'
    }
  },

and in my CSS I have something like:

header .button-menu {
  display: block;
  background: url(hamburger.svg);
}

@media all and (min-width: 801px) {
  header .button-menu {
    display: none;
  }
}

Will the .button-menu be included in the critical css? Because at the specified resolution of 1200x768 it is not visible and thus probably not included in the cricital-main.css - am I right?

How do I deal with differences in visible content depending on the viewport size?

scottjehl commented 9 years ago

Sorry for the delay, @manuelbieh

For your first question, I'm not sure whether we'll want to address it with this tool, but it doesn't currently do that sort of analyzing of multiple pages to make one file. Since we advise inlining only for first-visits to a site, before the full css is cached, I'm not sure there's much value in a shared include across files. Of course, each template will have a different include path, and I understand why that might be less preferable. Your best bet might be to concatenate all the critical css files and run a tool on them that removes duplicate rules (maybe unCSS does this? not sure).

For your second question, there are indeed some cases where an element that is critical to some breakpoints is not critical in the particular breakpoint that you're running this task with. In our own work, we find those cases to be fairly rare, but they do happen now and then and the forceInclude option should enable you to patch those up case-by-case.

I hope that helps

landsman commented 8 years ago

@manuelbieh I was thinking same:

    /**
     * Prepare object for critical css task
     * @returns {{}}
     */
    var modulesOptions = function()
    {
        // change this to your local development url
        var devUrl = "http://localhost/www/my-site/";

        var pages = [
            'shop',
            'category',
                'product-123',
            'homepage'
        ];

        var modules = {};
        pages.forEach(function(fn)
        {
            modules[fn] = {};
            modules[fn].options = {
                url: devUrl + fn,
                width: 1200,
                height: 3000,
                filename:   "css/front/styles.css",
                outputfile: "css/front/critical_" + fn + ".css",
                ignoreConsole: true
            };

        });

        return modules;
    };

and:

criticalcss: modulesOptions(),

What you think about this?