Add support of group and weight options to Themkey CSS

Seems like we have to add support of group and weight options to Themkey CSS according to https://api.drupal.org/api/drupal/includes!common.inc/function/drupal_ad...

Hi. This patch should resolve this issue.

Forgot new line at end of the *.install file

O, come on! One more patch :D

@bigmonmulgrew:
It would be great if you can give something back by just writing a few lines how to use the feature and add them as a sub page to https://www.drupal.org/node/2145521 . And a screenshot would be great.

@Loparev: First of all, great work! The patch already looks very good.

1. +++ b/themekey_admin.inc
   @@ -838,12 +888,21 @@ function theme_themekey_abstract_rule_chain_form($variables, $options) {
   -    $header = array(
   -      '',
   -      $options['header'][0],
   -      $options['header'][1],
   -      $options['header'][2],
   -    );
   +    // Remove 'Operation', 'Parent', 'Weight' and 'Page Cache' from header array
   +    // for 'new_item' table.
   +    $header = $options['header'];
   +    array_unshift($header, '');
   +
   +    foreach ($header as $key => $header_label) {
   +      if (in_array($header_label, array(
   +        t('Operation'),
   +        t('Parent'),
   +        t('Weight'),
   +        t('Page<br />Cache'),
   +      ))) {
   +        unset($header[$key]);
   +      }
   +    }
   

   I'm not sure about this. I will have a closer look and think about it again before committing it.

2. +++ b/themekey_css/themekey_css_admin.inc
   @@ -26,10 +26,34 @@ require_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'themekey') . '/theme
   +  $css_group_options = array(
   +    'CSS_DEFAULT' => 'CSS_DEFAULT',
   +    'CSS_THEME' => 'CSS_THEME',
   +    'CSS_SYSTEM' => 'CSS_SYSTEM',
   +  );
   

   We should shorten the displayed string. What about this?

   'CSS_DEFAULT' => 'default',
   'CSS_THEME' => 'theme',
   'CSS_SYSTEM' => 'system'
   

To make the patch even better we should add at least a small test that checks the addition of css files at the right position. I assume this test is straight forward if you look at the existing ones. Just create a rule that adds any css file that already exists somewhere in core.

Finally another feature that would be nice to have but is not a must: I think it would nice to be able to optionally show the list of css files and their order on the current page within ThemeKey Debug using drupal_get_css(). That would ease debugging if css aggregation is turned on.

Added short css_group_options and patched ThemeKey Debug. Now it can optionally show list of css files on a page.

@bigmonmulgrew, you need to enable ThemeKey Debug module. Then on admin/config/user-interface/themekey/settings/debug you should check "Show list of css files" checkbox. After that at the bottom of every page will be block with css debug message.

@bigmonmulgrew, all works as expected?

1. +++ b/themekey_css/themekey_css_admin.inc
   @@ -26,10 +26,34 @@ require_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'themekey') . '/theme
   +  $css_weight_options = array();
   +
   +  for ($i = -50; $i <= 50; $i++) {
   +    $css_weight_options[$i] = $i;
   +  }
   

   $css_weight_options = range(-50, 50);
   

2. +++ b/themekey_debug.module
   @@ -171,6 +171,18 @@ function themekey_debug_page_alter(&$page) {
   +    // drupal_get_css() returns html markup with "@import" statements combined in <style> tags.
   +    // themekey_set_debug_message() use filter_xss() to filtering html from string, but filter_xss() leaves
   +    // empty string where was html markup.
   +    // So we should filter elements that contains <style> tag
   

   The detailed description should be moved to the leading comment block of function themekey_debug_filter_css_array($element)

3. +++ b/themekey_debug_admin.inc
   @@ -39,6 +39,13 @@ function themekey_debug_settings_form() {
   +  $form['themekey_debug']['themekey_debug_show_css_files'] = array(
   +    '#type' => 'checkbox',
   +    '#title' => t('Show list of css files'),
   +    '#default_value' => variable_get('themekey_debug_show_css_files', FALSE),
   +    '#description' => t('Prints out detailed information about css files added on a page. Useful to debug ThemeKey Css rules.'),
   +  );
   +
   

   This should be moved to themekey_css.module and implemented as themekey_css_themekey_debug_settings_form_alter() to only display this option if themekey_css is enabled.
   In addition themekey_css_uninstall() has to call variable_del('themekey_debug_show_css_files').
   To respect the namespace a better variable name would be themekey_css_debug_css_files.

i have written a quick how-to on what I did, hope its up to your standards

@bigmonmulgrew: we don't have a standard right now and any piece of documentation we can get is great :-)

I had a quick look at https://www.drupal.org/node/2536930
I think your example could be improved. For such use cases we have a special property that is evaluated quicker than user:uid. Just use system:dummy = dummy.

BTW did you read the tutorials that are included in the module? To see them you need to enable core's help module. I think we should "copy" these tutorials to the handbook on drupal.org.

@mkalkbrenner, done.

1.

  // range(-50, 50) returns array with keys from 0 to 100
  // but we need array with keys from -50 to 50
  // because keys will be values for select.
  $css_weight_options = drupal_map_assoc(range(-50, 50));

3. I think we should delete 'themekey_css_debug_css_files' variable in hook_disable. Because if user selected 'Show list of css files' checkbox on debug settings form and then he disabled themekey_css then 'Show list of css files' checkbox will disappear from debug settings form but css debug message will not disappear. So we don't need call variable_del('themekey_css_debug_css_files') in hook_uninstall because this variable will be deleted in hook_disable.

It's easier to review an interdiff between #11 and #20.

1. +++ b/themekey_css/themekey_css.install
   @@ -39,6 +39,17 @@
    /**
   + * Implements hook_disable().
   + *
   + * We should delete 'themekey_css_debug_css_files' when module is disabled
   + * because if 'themekey_css_debug_css_files' == TRUE but module was disabled
   + * there will be css debug message on a page.
   + */
   +function themekey_css_disable() {
   +  variable_del('themekey_css_debug_css_files');
   +}
   

   That's a good argument to delete the variable in hook_disable instead of hook_uninstall!
   But we should move the comment into the function. Otherwise it will occur in the API documentation and it's only about internal information.

2. +++ b/themekey_debug.module
   @@ -199,6 +194,11 @@
   + * Filters elements with empty strings and elements with strings that contains '<style>' tag from array
   + * generated by explode("\n", drupal_get_css()). drupal_get_css() returns html markup with "@import" statements
   + * combined in <style> tags. themekey_set_debug_message() use filter_xss() to filtering html from string, but
   + * filter_xss() leaves empty string where was html markup.
   + *
   

   According to the drupal coding standards these comments must not exceed 80 characters per line.

I hope to find some time to provide a simple test for this patch soon and then we can commit it :-)
(Within the ThemeKey project I'm a bit more restrictive because of more than 20,000 installations.)

Final changes

Unfortunately we're not done yet :-(
Unless today I just looked at the code, because Drupal 8 core occupied most of my time. But now I applied the patch a gave it a try. I found a major issue: table drag isn't working anymore on the CSS rule chain!

1. +++ b/themekey_admin.inc
   @@ -700,6 +715,13 @@ function theme_themekey_abstract_rule_chain_form($variables, $options) {
   +            $form['old_items'][$key]['parent']['#attributes']['class'] = array("themekey-property-$field_name");
   
   @@ -816,6 +849,13 @@ function theme_themekey_abstract_rule_chain_form($variables, $options) {
   +          $form['new_item'][$field_name]['#attributes']['class'] = array("themekey-property-$field_name");
   

   css class names should avoid some characters. Somewhere in core there's a function to generate css class names from a given value. For example it converts '_' to '-'.

2. +++ b/themekey_admin.inc
   @@ -838,12 +888,21 @@ function theme_themekey_abstract_rule_chain_form($variables, $options) {
   -    $header = array(
   -      '',
   -      $options['header'][0],
   -      $options['header'][1],
   -      $options['header'][2],
   -    );
   +    // Remove 'Operation', 'Parent', 'Weight' and 'Page Cache' from header array
   +    // for 'new_item' table.
   +    $header = $options['header'];
   +    array_unshift($header, '');
   +
   +    foreach ($header as $key => $header_label) {
   +      if (in_array($header_label, array(
   +        t('Operation'),
   +        t('Parent'),
   +        t('Weight'),
   +        t('Page<br />Cache'),
   +      ))) {
   +        unset($header[$key]);
   +      }
   +    }
   
   +++ b/themekey_css/themekey_css_admin.inc
   @@ -97,12 +120,18 @@ function theme_themekey_css_rule_chain_form($variables) {
      return theme_themekey_abstract_rule_chain_form($variables, array(
        'header' => array(
          t('Adding CSS Rule Chain'),
   -      t('CSS'),
   +      t('CSS file'),
   +      t('CSS group'),
   +      t('CSS weight'),
          t('Enabled'),
          t('Operation'),
          t('Parent'),
          t('Weight'),
          t('Page<br />Cache'),
   -    )
   +    ),
   +    'additional_fields' => array(
   +      'css_group',
   +      'css_weight',
   +    ),
      ));
   

   As I already mentioned earlier in #10 this looks suspicious to me. Right now I noticed that the table drag feature is broken! The parent item is not hidden anymore and the table structure modified via drag & drop is not saved. Core's table drag feature requires parent and weight!
   See function theme_themekey_abstract_rule_chain_form():

   if (empty($form['pager']['#value'])) {
     drupal_add_tabledrag('themekey-properties', 'match', 'parent', 'themekey-property-parent', 'themekey-property-parent', 'themekey-property-id', TRUE);
     drupal_add_tabledrag('themekey-properties', 'order', 'sibling', 'themekey-property-weight');
   }
   

Hi!

This patch resolves problem with table drag&drop. I tested it and for now it's works.

Core's table drag feature requires parent and weight!

I just remove parent and weight from header for "New item" table. Just like in your code you set up header only with 3 first values ("ADDING CSS RULE CHAIN", "CSS FILE" and "ENABLED"). There was no "parent" and "weight" in table header.

$header = array(
      '',
      $options['header'][0],
      $options['header'][1],
      $options['header'][2],
   );

Try this patch please.

I will check it on clean drupal installation.

There is no problems with status report page on clean installation.

Committed to git. Thanks a lot for the work!