Further decouple the current mess of having to load skin info when we only need and want to apply skins and skin info is totally not required.

We basically need to clean up skinr_preprocess() as much as possible by removing dependencies on skinr_get_skin_info() and similar resource heavy functions (either through caching, or other options).

Comments

rooby’s picture

rooby commented

Is it possible that during this process the loading of the skins (for applying) be moved from hook_preprocess into its own function?

for example it would be very useful to be able to call something like:

<?php
function skinr_load_skins($module, $element, $theme = NULL) {
  // If not theme, use current theme.

  // Load skins for $module $element.

  // Return skins.
}
?>

to get the skin settings and do with them what you want.

For example, I have some node skins, that I wanted to also apply to the node form so that it is a bit more of a WYSIWYG experience for editors as the form can more closely represent the view.

To do this I had to load the skins for the given node and apply the attachments and classes.
To do that I have some pretty messy code with a lot of parts duplicated in some way from skinr_preprocess.

It would be great to have a cleaner way of doing this type of thing.

It might also be nice to be able to hook into the process as it happens in skinr_preprocess, in the case you might want to do something more custom depending on what has been selected for a skin option.

moonray’s picture

moonray commented

All you need for what you're describing above is:

<?php
    // Get a list of skin configuration IDs to pass to
    // skinr_skin_load_multiple().
    $params = array(
      'theme' => $current_theme,
      'module' => $module,
      'element' => $elements,
      'status' => 1,
    );
    $sids = skinr_skin_get_sids($params);
    if (empty($sids)) {
      // Noting to do.
      continue;
    }
    $skins = skinr_skin_load_multiple($sids);
?>

That hardly seems worth breaking out into a separate function.

EDIT: As for hooking into the preprocess function, there's always hook_skinr_preprocess_alter($skins, $context)

rooby’s picture

rooby commented

The snippet you have there is what I originally had but what I ended up with in the end had more of what skinr_preprocess has, for example, they really should be passed through drupal_alter('skinr_preprocess', $skins, $context); also or else you might not get the same results.

I also got classes using skinr_flatten_skins_array() and got the attached css & js, so it's getting pretty close to what skinr_preprocess() is.

However that said, it is not the same as skinr_preprocess and I understand if it couldn't be abstracted in a useful way.

moonray’s picture

moonray commented
Status: Active » Closed (works as designed)

I don't believe there's a good way to abstract this. If you have a good suggestion, feel free to post a patch and reopen.