Much like how drupal_add_js is getting a make over, revamp drupal_add_css by:

  • Allowing reordering of CSS files (#weight)
  • Having it able to reference and cache CSS files not on the local server (external CSS)
  • Adding an 'inline' type for adding straight inline CSS to the page (aggregate the CSS if caching is enabled)
  • Replacing the parameter list with $options
  • Allowing removal of select CSS files

Comments

RobLoach’s picture

Status: Needs work » Active

A hook_css_alter could be added to allow modification of CSS being added. Might be overkill though.

RobLoach’s picture

Status: Active » Needs work
FileSize
4.73 KB

Here's a patch to start it off:

/**
 * Adds CSS to the stylesheet queue.
 *
 * @param $data
 *   (optional) If given, the value depends on the $options parameter.
 *   - 'file': Path to the file relative to base_path().
 *   - 'inline': The CSS code that should be placed in the given scope.
 *   - 'external': The URL to a CSS file that's hosted on an external server.
 * @param $options
 *   (optional) A string for the type ('file', 'inline', 'external'), or an
 *   array which can have any or all of the following keys:
 *   - #type
 *       (optional) The type of CSS that should be added to the page. Allowed
 *       values are 'file', 'inline' and 'external'.
 *   - #media
 *       (optional) The media type for the stylesheet, e.g., all, print,
 *       screen.
 *   - #preprocess
 *       (optional) Whether or not this CSS should be aggregated and compressed
 *       if this feature has been turned on under the performance section.
 * @return
 *   An array of CSS stylesheets to include - the array has three keys:
 *   - 'files'
 *       An array of files keyed by path (the value is the $options that were
 *       passed in).
 *   - 'inline'
 *       An array of CSS code to be added inline (the value is the $options
 *       that were passed in).
 *   - 'external'
 *       An array of CSS URLs to be added to the queue (the value is the
 *       $options that were passed in).
 */
function drupal_add_css($data = NULL, $options = array()) {
RobLoach’s picture

Status: Active » Postponed

Has to wait until the drupal_add_js fix goes in.

Susurrus’s picture

that issue is also just waiting review. If people could look that issue over and run the associated sinpletests under System, we could get this issue started sooner.

RobLoach’s picture

Title: Revamp drupal_add_css() » Use $options in drupal_add_css()
Status: Postponed » Needs review
FileSize
15.25 KB

This patch takes the scope down a bit, just adding the $options parameter to drupal_add_css()....

/**
 * Adds a CSS file to the stylesheet queue.
 *
 * @param $path
 *   (optional) The path to the CSS file relative to the base_path(), e.g.,
 *   /modules/devel/devel.css.
 *
 *   Modules should always prefix the names of their CSS files with the module
 *   name, for example: system-menus.css rather than simply menus.css. Themes
 *   can override module-supplied CSS files based on their filenames, and this
 *   prefixing helps prevent confusing name collisions for theme developers.
 *   See drupal_get_css where the overrides are performed.
 *
 *   If the direction of the current language is right-to-left (Hebrew,
 *   Arabic, etc.), the function will also look for an RTL CSS file and append
 *   it to the list. The name of this file should have an '-rtl.css' suffix.
 *   For example a CSS file called 'name.css' will have a 'name-rtl.css'
 *   file added to the list, if exists in the same directory. This CSS file
 *   should contain overrides for properties which should be reversed or
 *   otherwise different in a right-to-left display.
 * @param $options
 *   An optional associative array of additional options, with the following
 *   keys:
 *     - 'type'
 *       The type of stylesheet that is being added. Types are: module or
 *       theme.
 *     - 'media'
 *       The media type for the stylesheet, e.g., all, print, screen.
 *     - 'preprocess':
 *       Should this CSS file be aggregated and compressed if this feature has
 *       been turned on under the performance section?
 *
 *       What does this actually mean?
 *       CSS preprocessing is the process of aggregating a bunch of separate CSS
 *       files into one file that is then compressed by removing all extraneous
 *       white space.
 *
 *       The reason for merging the CSS files is outlined quite thoroughly here:
 *       http://www.die.net/musings/page_load_time/
 *       "Load fewer external objects. Due to request overhead, one bigger file
 *       just loads faster than two smaller ones half its size."
 *
 *       However, you should *not* preprocess every file as this can lead to
 *       redundant caches. You should set $preprocess = FALSE when your styles
 *       are only used rarely on the site. This could be a special admin page,
 *       the homepage, or a handful of pages that does not represent the
 *       majority of the pages on your site.
 *
 *       Typical candidates for caching are for example styles for nodes across
 *       the site, or used in the theme.
 * @return
 *   An array of CSS files.
 */
function drupal_add_css($path = NULL, $options = array()) {
  static $css = array();
RobLoach’s picture

Status: Needs review » Needs work

Needs update to HEAD.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
16.09 KB

Updated to HEAD and added a couple tests hitting drupal_add_css() and drupal_get_css().

mfer’s picture

subscribe. I'll test this later in the week.

aaron’s picture

looks good. i'll try to test later as well.

mfer’s picture

I'll jump on the @drewish bandwagon and point out that the test, though only 2 of them, should be two separate test methods.

The common setup code between the two test should go in setUp().

RobLoach’s picture

Status: Needs review » Needs work

Agreed!

RobLoach’s picture

Status: Needs work » Needs review
FileSize
16.74 KB

Refactored the tests and added $reset so that we can test it.

mfer’s picture

Status: Needs review » Needs work

It looks good and passes tests with the exception of the drupal_add_css calls in theme.maintenance.inc. They still need to be updated.

mfer’s picture

@Rob Loach - I'm not planning on touching this patch so I can test it with a clean conscience.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
18.03 KB

Those actually are okay because the new drupal_add_css()'s $options parameter can be either the type or the $options array. I removed the second parameter anyway.....

mfer’s picture

Status: Needs review » Reviewed & tested by the community

I knew those would work the way they were layed out. Just being picky and wanting consistency. This looks good to go.

Dries’s picture

Status: Reviewed & tested by the community » Needs work

- Can we rename '$simpletestcss' to '$css'? We don't glue words together.
- t('Cascading Stylesheets') should be t('Cascading stylesheets') (small s)
- We should probably document the default variables of the different $options.

RobLoach’s picture

Status: Needs work » Needs review
FileSize
18.07 KB

Makes the changes that Dries brought up in #17.

mfer’s picture

Status: Needs review » Reviewed & tested by the community

#17 is why Dries is the man when it comes to reviewing patches.

The patch in #18 looks good to me, the tests pass, and it works.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks! :)

RobLoach’s picture

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.