Ability to inherit Skinr styles from parent themes in 7.x-2.x?

All of the skins are already available to use in my base theme. It's the actual applied skin styles that are not being inherited.

For example, I set the default theme to my_base_theme, and configure a load of skinr styles for various elements on my site. I'm using the SwitchTheme module to allow users to switch the theme to one of several subthemes, where things like colours and background images are changed. When the theme is switched, none of the blocks etc have the skinr styles applied. I can edit them manually and add them for each specific subtheme - but I want them to inherit them automatically.

Seems like a pretty common thing to want to do. In my experience Sub themes (I'm talking about theme variations - not base theme subthemes) would generally only override a small amount of styles. We have hundreds of skinr styles set on our main theme, and don't want to override any of these in the the various subthemes.

Why would that not be in the scope of Skinr? Sounds like a pretty useful option to have.

I have this semi-working in a custom module, however, there is a problem with Skinr that's stopping me from being able to do what I need to do. The code below works, but only for elements where at least one skinr attribute has been set on the subtheme. The problem seems to be that you only actually call skinr_preprocess_alter on elements that have some skinr properties set. That would generally make sense, but for this use case it doesn't as the active theme won't have any/many skinr settings on any elements.

/**
 * Implements hook_skinr_preprocess_alter().
 */
function alumni_base_skinr_preprocess_alter(&$skins, $context) {
  // Fix for update script.
  if ($context['hook'] == 'maintenance_page') {
    return;
  }

  $current_theme = skinr_current_theme();
  $themes = system_rebuild_theme_data();

  $skinr_settings = (array) $themes[$current_theme]->info['skinr'];

  // Inherit styles from theme parents, if inherit_styles set to true
  if (isset($skinr_settings['options']['inherit_styles']) && $skinr_settings['options']['inherit_styles']) {
    if ($parent_styles = alumni_base_skinr_inherited_styles($current_theme, $themes, $context)) {
      $skins = $parent_styles + $skins;
    }
  }
}

/**
 * Retrives all the Skinr skins from theme parents.  Theme skins 
 * will override any skins of the same name from its parents.
 */
function alumni_base_skinr_inherited_styles($theme, $themes, $context) {
  $base_theme = (!empty($themes[$theme]->info['base theme'])) ? $themes[$theme]->info['base theme'] : '';
  $skin_info = skinr_get_skin_info();
  
  // Get a list of skin configuration IDs to pass to
  // skinr_skin_load_multiple().
  $params = array(
    'theme' => $base_theme,
    'module' => $context['module'],
    'element' => $context['elements'],
    'status' => 1,
  );
  $sids = skinr_skin_get_sids($params);

  return skinr_skin_load_multiple($sids);
}

I can't see a viable way to do this using the prepress_alter function. I can't really see a good reason why this would be out of scope in Skinr, so here is a patch that adds style inheritance support to Skinr, enabling the use of a skinr[options][inherit_styles] = true setting, which will allow applied styles to be inherited from the base theme.

skinr[options][inherit_skins] = true was used for that - that's why I names this inherit_styles, as we are talking about inheriting applied styles, not defined skins. I haven't tested your code above, but it's a similar approach to what I was trying initially in preprocess. But, it wont work because preprocess only gets called for elements that have at least one style applied in the current theme.

Yes - thought about the other preprocess approach too - but seems like it would add a fair amount of duplication.

I'm not really clear why we are bringing inherit_skins into this discussion. inherit_skins is about inheriting defined skins, and was a D6 thing which is included in D7 in a slightly different way (you implement a hook to modify the behavior rather than add an option to the theme's .info file). This issue is about something completely different (although when I first opened the ticket I had thought that the answer lay in inherit_skins). This is about inheriting styles, hence I have used a different name for the 'inherit_styles' .info option.

Are you still of the opinion that this doesn't belong in Skinr core? And if so, why not? Seems like a pretty useful addition. Skinr already has the ability to inherit skins. Why not allow it to inherit applied styles too? Sorry if I'm missing the point on this one.

@moonray - I don't think there is much in the way of overhead here. We call skinr_implements_api() to find out if 'Skinr settings' inheritance is enabled, and the skinr_implements_api() function uses a static cache. So all we are talking about is looking at the value of an already statically cached variable.

How does this take away from the modularity that skin plugins provide?

Setting back to needs review, as I don't know what further work is required.

I don't really see how skin inheritance is an edge case. Themes have inheritance. Skins relate to themes, and so should have an inheritance option too. It makes no sense to require users to duplicate every single Skin configuration out to every single subtheme manually to make the base theme skin configurations available in subthemes too. Subthemes should be able to inherit skin configs just as they can other configs. It shouldn't need a module or custom code to do that. It should be an option in the theme settings, IMO.

Your logic makes little sense to me. Themes can be configured to inherit from a base theme. That means that they share all attributes of the base theme. That is configured by setting a theme setting. The theme setting itself is not inherited - it is explicitly set in the sub theme. IMO, the same should be true of skinr. It should be able to inherit from the base theme, if configured to do so in the theme settings, making the sub theme act just like the base theme.

• css can be inherited from a base theme
• panels layouts can be inherited from a base theme
• display suite layouts can be inherited from a base theme

All of the above help to reduce the need to duplicate countless amounts of duplication following the concept that a theme that inherits from a base theme should start out exactly like the parent theme, and should be able to extend that theme as simply as possible. Why should it not also be possible to inherit skinr configurations from a base theme? The reasoning I have heard so far includes:

> My main issue with this is the added overhead.
Where is the overhead? checking a statically cached variable from RAM hardly constitutes much in the way of overhead. Skinr has plenty of performance problems to be sure, but I don't think thats one of them.

> And the fact that it takes away from the modularity that skin plugins provide since version 7.x
How does letting people decide if the want their sub theme to inherit skinr applied styles from their base theme take away from the modularity that skin plugins provide?

> Inheriting skin configuration from parent themes is an edge case. Enabling this might horribly break people's existing sites that aren't expecting that behavior. So, I think with the above fix in place it should allow extending Skinr with a custom module to inject skin configuration and solve your problem while not breaking the site for 99% of use cases.
The setting would only affect users that explicitly enable it in their .info file - 99% of users would likely not do that, unless they all actually wanted style inheritance.