Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding the ability to inline CSS and Js code via Asset Manager #1377

Merged
merged 2 commits into from
Apr 4, 2017
Merged

Adding the ability to inline CSS and Js code via Asset Manager #1377

merged 2 commits into from
Apr 4, 2017

Conversation

OliverO2
Copy link
Contributor

@OliverO2 OliverO2 commented Mar 23, 2017

This pull request makes Grav's Asset Manager support 'inline' as a new loading option for CSS and Js assets. It also tries to simplify things by extracting common code from addCss() and addJs() into a new helper function (though the same could still be done for output and pipeline functions).

Inlining CSS and Js resources can speed up rendering, particularly over mobile networks. It is not always feasible to directly include assets into the template, for example, where Sass-complied CSS is used or where CSS or Js is imported from external sites to load fonts.

Enabling any CSS and Js asset to become inline makes it easy to optimize for speed and to reduce flashes of unstyled content without having to re-structure or re-process asset files.

A slightly simplified real world use case:

system.yaml uses collections to group assets by loading method:

assets:
  collections:
    css-inline:
      - 'http://fonts.googleapis.com/css?family=Ubuntu:400|Open+Sans:400,400i,700'
      - 'theme://css-compiled/app.css'
    js-inline:
      - 'https://use.fontawesome.com/<embedcode>.js'
    js-async:
      - 'theme://foundation/dist/assets/js/app.js'
      - 'theme://js/header-display.js'

The template inserts these into different asset groups depending on loading requirements (defaulting to inline):

        {% block stylesheets %}
            {% do assets.addCss('css-inline') %}
            {% do assets.addCss('css-link', {'group': 'head-link'}) %}
        {% endblock %}
        {{ assets.css('head-link') }}
        {{ assets.css('head', {'loading': 'inline'}) }}
        {% block javascripts %}
            {% do assets.addJs('js-inline') %}
            {% do assets.addJs('js-async', {'group': 'head-async'}) %}
        {% endblock %}
        {{ assets.js('head-async', {'loading': 'async'}) }}
        {{ assets.js('head', {'loading': 'inline'}) }}

See also: #754

* Assets.php: Extract common functionaly of addCss() and addJs() into
  addTo(), make css() and js() honor $attributes['loading'],
  make pipelineCss() and pipelineJs() return generated content
  instead of URL depending on new option $returnURL.

* AssetsTest.php: Accept new 'loading' option for CSS, add tests for
  adding and pipelining local and remote assets with inline loading.
@OleVik
Copy link
Contributor

OleVik commented Mar 23, 2017

Could elaborate on how this differs from the already existing AddInlineCSS and AddInlineJS? Does it, like 754, add the ability to inline the entirety of the file in a <style>- or <script>-tag?

@OliverO2
Copy link
Contributor Author

@OleVik Yes, exactly: This PR adds the ability to inline the entirety of the file's (or an entire pipeline's) contents in a <style>- or <script>-tag. It works for local files as well as remote URLs.

@rhukster
Copy link
Member

This PR looks promising. I'm traveling for a week so won't really have chance to thoroughly test this until I get back.

@OliverO2
Copy link
Contributor Author

@rhukster Thanks! Don't worry, take your time. I've included some test cases and I'm already using this on a production site.

@rhukster rhukster merged commit f510e13 into getgrav:develop Apr 4, 2017
@rhukster
Copy link
Member

rhukster commented Apr 4, 2017

Looks good 👍

@OliverO2
Copy link
Contributor Author

OliverO2 commented Apr 4, 2017

Cool, thanks! 😃

rhukster pushed a commit to getgrav/grav-learn that referenced this pull request May 12, 2017
Includes
* #356: Added inline CSS and JS via Asset manager getgrav/grav#1377
* Add missing documentation on configuration options
* Add an overview section
* Move "Static Assets" to the bottom
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants