diff --git modules/panels.skinr.inc modules/panels.skinr.inc index 01f633e..7e60bed 100644 --- modules/panels.skinr.inc +++ modules/panels.skinr.inc @@ -22,6 +22,9 @@ function panels_skinr_config_info() { $data['panels']['preprocess']['panels_pane'] = array( 'index_handler' => 'panels_skinr_preprocess_index_handler', ); + $data['panels']['contextual_links']['panels_pane'] = array( + 'contextual_links_handler' => 'panels_skinr_contextual_links', + ); return $data; } @@ -67,20 +70,26 @@ function panels_skinr_preprocess_index_handler(&$variables) { } /** - * Implements hook_panels_pane_content_alter(). + * Skinr contextual links handler. + * + * @param &$variables + * Passes in the $variables parameter from skinr_preprocess(). * - * Because of the way panels handles contextual links we can't use a - * contextual links handler. + * @return + * An array. Each value is an array that forms the function arguments for + * menu_contextual_links(). For example: + * @code + * array( + * 'admin/appearance/skinr/edit', array('system', 'navigation')), + * ) + * @endcode */ -function panels_panels_pane_content_alter(&$content, $pane, $display_args, $context) { - if (user_access('edit skin settings')) { - $element = 'pane__' . $pane->did . '__' . $pane->pid; - $content->admin_links[] = array( - 'title' => t('Edit skin'), - 'href' => 'admin/appearance/skinr/edit/nojs/panels/' . $element . '/configure', - 'query' => drupal_get_destination(), - ); - } +function panels_skinr_contextual_links(&$variables) { + $links = array(); + $links['skinr-panels'] = array( + 'admin/appearance/skinr/edit/nojs', array('panels', 'pane__' . $variables['pane']->did . '__' . $variables['pane']->pid), + ); + return $links; } /** diff --git skinr.module skinr.module index 8b6a3e3..597fb51 100644 --- skinr.module +++ skinr.module @@ -62,28 +62,6 @@ function skinr_cache_reset() { } /** - * Implements hook_module_implements_alter(). - */ -function skinr_module_implements_alter(&$implementations, $hook) { - // Run Skinr first to avoid issues with other modules during hook_init(). - if ($hook == 'init') { - $skinr['skinr'] = $implementations['skinr']; - unset($implementations['skinr']); - $implementations = array_merge($skinr, $implementations); - } -} - -/** - * Implements hook_init(). - * - * @todo Kill me. Entirely. - */ -function skinr_init() { - module_load_include('inc', 'skinr', 'skinr.handlers'); - skinr_load_includes(); -} - -/** * Implements hook_preprocess(). * * @todo Optimize this function by removing dependencies on @@ -444,6 +422,11 @@ function skinr_implements() { if (!isset($cache)) { $cache = array(); + + // Load built-in support code for Drupal core modules. + // @todo Remove this when we eliminate skinr handlers. + module_load_include('inc', 'skinr', 'skinr.handlers'); + // Collect hook_skinr_api_VERSION() module implementations. This will also // auto-load $module.skinr.inc files, which may contain skin/group hook // implementations (when not using the plugin system). @@ -465,9 +448,14 @@ function skinr_implements() { // $module.skinr.inc file and auto-load it. module_implements() only // auto-loads $module.skinr.inc in a module's root folder. if (isset($cache[$module]['path'])) { - $file = DRUPAL_ROOT . '/' . $cache[$module]['path'] . '/' . $module . '.skinr.inc'; - if (file_exists($file)) { - require_once $file; + $file = $cache[$module]['path'] . '/' . $module . '.skinr.inc'; + if (file_exists(DRUPAL_ROOT . '/' . $file)) { + $cache[$module]['include file'] = $file; + // When rebuilding the cache, we auto-load the include file for + // convenience. Calling code should invoke skinr_load_includes() + // on the extensions they want to act with. + // @todo Remove this. + include_once DRUPAL_ROOT . '/' . $file; } } // Populate defaults. @@ -537,9 +525,14 @@ function skinr_implements() { ); // Lastly, for API consistency with modules, check whether the theme // contains a $theme.skinr.inc file and auto-load it, if any. - $file = DRUPAL_ROOT . '/' . $cache[$name]['path'] . '/' . $name . '.skinr.inc'; - if (file_exists($file)) { - require_once $file; + $file = $cache[$name]['path'] . '/' . $name . '.skinr.inc'; + if (file_exists(DRUPAL_ROOT . '/' . $file)) { + $cache[$name]['include file'] = $file; + // When rebuilding the cache, we auto-load the include file for + // convenience. Calling code should invoke skinr_load_includes() + // on the extensions they want to act with. + // @todo Remove this. + include_once DRUPAL_ROOT . '/' . $file; } } } @@ -549,15 +542,15 @@ function skinr_implements() { } /** - * Includes $extension.skinr.inc files of extensions compatible with this version of Skinr. + * Loads $extension.skinr.inc include files of passed in extensions. * - * @todo Shoot me. Twice. + * @todo These hook implementations should not be required to be loaded at all, + * rewrite the code that is currently not using the cache. */ -function skinr_load_includes() { - foreach (skinr_implements() as $extension) { - $file = DRUPAL_ROOT . '/' . $extension['path'] . '/' . $extension['name'] . '.skinr.inc'; - if (file_exists($file)) { - require_once $file; +function skinr_load_includes(array $extensions) { + foreach ($extensions as $extension) { + if (isset($extension['include file'])) { + include_once DRUPAL_ROOT . '/' . $extension['include file']; } } } @@ -1206,6 +1199,11 @@ function skinr_get_config_info() { if (!isset($config_info)) { if ($cached = cache_get('skinr_config_info')) { $config_info = $cached->data; + + // Load the module integration files. + // @todo Whatever currently needs to be invoked has to be cached instead. + skinr_load_includes($config_info); + return $config_info; } $config_info = array(); @@ -1215,6 +1213,13 @@ function skinr_get_config_info() { if (function_exists($function)) { $extension_info = $function(); if (isset($extension_info) && is_array($extension_info)) { + // @todo Yuck! Rewrite hook_skinr_config_info() API structure. Why do + // implementations return info keyed by their own module name? + if (isset($extension['include file'])) { + foreach ($extension_info as &$info) { + $info['include file'] = $extension['include file']; + } + } $config_info = array_merge_recursive($config_info, $extension_info); } } @@ -1314,6 +1319,12 @@ function skinr_config_info_default() { * @param $a7 */ function skinr_handler($type, $op, $handler, &$a3, $a4 = NULL, $a5 = NULL, $a6 = NULL, $a7 = NULL) { + // @todo Once we eliminate skinr_handler, all functions in skinr.handlers.inc can + // be moved out of it and the file eliminated. + if (in_array($handler, array('skinr_access_handler', 'skinr_index_handler', 'skinr_data_handler', 'skinr_submit_handler'))) { + module_load_include('inc', 'skinr', 'skinr.handlers'); + } + if (is_callable($handler)) { switch ($type) { case 'contextual_links': diff --git skinr_ui.module skinr_ui.module index bbc9e61..947d6c9 100644 --- skinr_ui.module +++ skinr_ui.module @@ -6,6 +6,31 @@ */ /** + * Implements hook_theme_registry_alter(). + * + * Re-order preprocess functions to prioritize skinr_ui_preprocess, which adds + * contextual links, over template_preprocess_HOOK functions. This fixes a + * problem with the way panels handles contextual links. + */ +function skinr_ui_theme_registry_alter(&$theme_registry) { + $preprocess_functions = array(); + foreach ($theme_registry['panels_pane']['preprocess functions'] as $function) { + if ($function == 'skinr_ui_preprocess') { + continue; + } + + $preprocess_functions[] = $function; + + if ($function == 'template_preprocess') { + // Insert our preprocess function right after template_preprocess to give it priority over template_preprocess_HOOK functions. + $preprocess_functions[] = 'skinr_ui_preprocess'; + } + } + + $theme_registry['panels_pane']['preprocess functions'] = $preprocess_functions; +} + +/** * Implements hook_permission(). */ function skinr_ui_permission() { @@ -668,6 +693,10 @@ function skinr_ui_preprocess(&$variables, $hook) { } } } + if ($hook == 'panels_pane') { + dpm('skinr_ui_preprocess [' . $original_hook . ']'); + dpm($variables['content']->content); + } } /** @@ -687,15 +716,19 @@ function skinr_ui_contextual_links(&$variables, $hook, $contextual_links) { $hooks = theme_get_registry(); // Determine the primary theme function argument. - if (isset($hooks[$hook]['variables'])) { + if (!empty($hooks[$hook]['variables'])) { $keys = array_keys($hooks[$hook]['variables']); $key = $keys[0]; } - else { + elseif (!empty($hooks[$hook]['render element'])) { $key = $hooks[$hook]['render element']; } - if (isset($variables[$key])) { - $element =& $variables[$key]; + if ($hook == 'panels_pane') { + // Exception for panels_pane. + $element = &$variables['content']->content; + } + elseif (!empty($key) && isset($variables[$key])) { + $element = &$variables[$key]; } if (isset($element) && is_array($element)) { diff --git tests/skinr.test tests/skinr.test index 2d6ee92..1b78578 100644 --- tests/skinr.test +++ tests/skinr.test @@ -5,6 +5,46 @@ * Tests for the Skinr module. */ +class SkinrWebTestCase extends DrupalWebTestCase { + /** + * Asserts that a class is set for the given element id. + * + * @param $id + * Id of the HTML element to check. + * @param $class + * The class name to check for. + * @param $message + * Message to display. + * @return + * TRUE on pass, FALSE on fail. + */ + function assertSkinrClass($id, $class, $message = '') { + $elements = $this->xpath('//div[@id=:id and contains(@class, :class)]', array( + ':id' => $id, + ':class' => $class, + )); + $this->assertTrue(!empty($elements[0]), $message); + } + + /** + * Asserts that a class is not set for the given element id. + * + * @param $id + * Id of the HTML element to check. + * @param $class + * The class name to check for. + * @param $message + * Message to display. + * @return + * TRUE on pass, FALSE on fail. + */ + function assertNoSkinrClass($id, $class, $message = '') { + $elements = $this->xpath('//div[@id=:id]', array(':id' => $id)); + $class_attr = (string) $elements[0]['class']; + $this->assertTrue(strpos($class_attr, $class) === FALSE, $message); + } +} + /** * Tests basic module installation. */ @@ -86,7 +126,7 @@ class SkinrInstallationTestCase extends DrupalWebTestCase { * * @link http://drupal.org/node/953336#comment-3738456 Make sure this patch is applied to drupal core @endlink */ -class SkinrApiTestCase extends DrupalWebTestCase { +class SkinrApiTestCase extends SkinrWebTestCase { protected $profile = 'testing'; public static function getInfo() { @@ -140,6 +180,7 @@ class SkinrApiTestCase extends DrupalWebTestCase { 'node' => array( 'version' => VERSION, 'path' => drupal_get_path('module', 'skinr') . '/modules', + 'include file' => drupal_get_path('module', 'skinr') . '/modules/node.skinr.inc', ), // skinr_test has been installed. 'skinr_test' => array( @@ -151,6 +192,7 @@ class SkinrApiTestCase extends DrupalWebTestCase { 'directory' => 'skins', 'base themes' => array(), 'sub themes' => drupal_map_assoc(array('skinr_test_subtheme')), + 'include file' => drupal_get_path('theme', 'skinr_test_basetheme') . '/skinr_test_basetheme.skinr.inc', ), 'skinr_test_subtheme' => array( 'type' => 'theme', @@ -158,6 +200,7 @@ class SkinrApiTestCase extends DrupalWebTestCase { 'directory' => 'skins', 'base themes' => drupal_map_assoc(array('skinr_test_basetheme')), 'sub themes' => array(), + 'include file' => drupal_get_path('theme', 'skinr_test_subtheme') . '/skinr_test_subtheme.skinr.inc', ), ); // When running tests on Skinr code packaged by drupal.org, all 'version' @@ -189,6 +232,61 @@ class SkinrApiTestCase extends DrupalWebTestCase { } /** + * Tests skinr_implements() caching and auto-loading. + */ + function testSkinrImplementsCache() { + module_enable(array('block')); + // Enable main system block for content region and the user menu block for + // the first sidebar. + $default_theme = variable_get('theme_default', 'bartik'); + db_merge('block') + ->key(array( + 'theme' => $default_theme, + 'module' => 'system', + 'delta' => 'main', + )) + ->fields(array( + 'status' => 1, + 'region' => 'content', + 'pages' => '', + )) + ->execute(); + db_merge('block') + ->key(array( + 'theme' => $default_theme, + 'module' => 'system', + 'delta' => 'powered-by', + )) + ->fields(array( + 'status' => 1, + 'region' => 'sidebar_first', + 'pages' => '', + )) + ->execute(); + + // Enable a skin defined in an include file, which applies to a module + // element that is equally registered in an include file (built-in Block + // module integration). + $skin = (object) array( + 'theme' => $default_theme, + 'module' => 'block', + 'element' => 'system__powered-by', + 'skin' => 'skinr_test_font', + 'options' => array('font_1'), + 'status' => 1, + ); + skinr_skin_save($skin); + + // Verify the skin is contained in the output. + $this->drupalGet(''); + $this->assertSkinrClass('block-system-powered-by', 'font-1', 'Skin found.'); + + // Once again, so we hit the cache. + $this->drupalGet(''); + $this->assertSkinrClass('block-system-powered-by', 'font-1', 'Skin found.'); + } + + /** * Tests hook_skinr_skin_info(). */ public function testSkinrSkinInfo() { @@ -422,7 +520,7 @@ class SkinrApiTestCase extends DrupalWebTestCase { * * @link http://drupal.org/node/953336#comment-3738456 Make sure this patch is applied to drupal core @endlink */ -class SkinrDisplayTestCase extends DrupalWebTestCase { +class SkinrDisplayTestCase extends SkinrWebTestCase { protected $profile = 'testing'; public static function getInfo() { @@ -474,42 +572,6 @@ class SkinrDisplayTestCase extends DrupalWebTestCase { theme_enable(array('garland')); } - /** - * Asserts that a class is set for the given element id. - * - * @param $id - * Id of the HTML element to check. - * @param $class - * The class name to check for. - * @param $message - * Message to display. - * @return - * TRUE on pass, FALSE on fail. - */ - function assertSkinrClass($id, $class, $message = '') { - $elements = $this->xpath('//div[@id=:id]', array(':id' => $id)); - $class_attr = (string) $elements[0]['class']; - $this->assertTrue(strpos($class_attr, ' ' . $class . ' '), $message); - } - - /** - * Asserts that a class is not set for the given element id. - * - * @param $id - * Id of the HTML element to check. - * @param $class - * The class name to check for. - * @param $message - * Message to display. - * @return - * TRUE on pass, FALSE on fail. - */ - function assertNoSkinrClass($id, $class, $message = '') { - $elements = $this->xpath('//div[@id=:id]', array(':id' => $id)); - $class_attr = (string) $elements[0]['class']; - $this->assertFalse(strpos($class_attr, ' ' . $class . ' '), $message); - } - public function testSkinrDisplayed() { // Save a skin configuration object. $skin = (object) array(