Problem/Motivation

I want to add a class for this specific theme hook suggestion... help!
Core doesn't invoke theme hook suggestions [pre]process functions. See related issues for back story/discussions.

Currently if one wants to add just a simple class for a specific theme hook suggestion, one must physically implement a function override or template with the theme hook suggestion in the name for it to actually be registered in the theme registry. Because this method is technically an override, it must also return the correct markup.

Keeping track of the correct theme hook function to invoke (core, base theme, sub-theme, sub-sub-theme) is taxing on a themer and completely unnecessary. All we are doing is altering the attributes array so we can add a class. We're not actually wanting to implement a different markup structure result.

/**
 * Overrides theme_container().
 */
function SUB_THEME_container__suggestion($variables) {
  // Inject our needed classes for this suggestion.
  $variables['element']['#attributes']['class'][] = 'col-sm-8';
  $variables['element']['#attributes']['class'][] = 'pull-right';
  // We must manually invoke the correct base hook function.
  // Notice it's not theme_container(). If this were a sub-sub-theme
  // and the sub-theme implemented it's own override (for whatever
  // reason), we'd have to do something like this instead:
  // return sub_theme_container($variables);
  return bootstrap_container($variables);
}

Down the road of using [pre]process functions as related issues suggest:
If a base hook implements it's own [pre]process functions (bootstrap_preprocess_container()), the subsequent theme hook suggestion [pre]process functions are not actually called. This is what the related issues are saying is considered "by design". The afflicting code is in: theme():

  // Invoke the variable processors, if any. The processors may specify
  // alternate suggestions for which hook's template/function to use. If the
  // hook is a suggestion of a base hook, invoke the variable processors of
  // the base hook, but retain the suggestion as a high priority suggestion to
  // be used unless overridden by a variable processor function.
  if (isset($info['base hook'])) {
    $base_hook = $info['base hook'];
    $base_hook_info = $hooks[$base_hook];
    // Include files required by the base hook, since its variable processors
    // might reside there.
    if (!empty($base_hook_info['includes'])) {
      foreach ($base_hook_info['includes'] as $include_file) {
        include_once DRUPAL_ROOT . '/' . $include_file;
      }
    }
    if (isset($base_hook_info['preprocess functions']) || isset($base_hook_info['process functions'])) {
      $variables['theme_hook_suggestion'] = $hook;
      $hook = $base_hook;
      $info = $base_hook_info;
    }
  }

Notice the last part which overrides the $info variable:

      $info = $base_hook_info;

This is what has lead to the idea that using the following is an "acceptable" solution:

function SUB_THEME_preprocess_container(&$variables, $hook) {
  if (in_array('container__suggestion', $variables['theme_hook_suggestions']) && function_exists('SUB_THEME_container__suggestion') {
    SUB_THEME_container__suggestion($variables, $hook);
  }
}
function SUB_THEME_container__suggestion(&$variables) {
  // ...
}

The problem with this approach is two fold:

  1. Assumption that $variables['theme_hook_suggestions'] is populated - Contrary to what the related issues suggest, a theme hook suggestion is NOT passed if there is no actual theme hook function name/template that implements the override for that specific suggestion. Example: bootstrap_preprocess_container__suggestion() will not actually be called if bootstrap_container__suggestion() doesn't exist to provide/override a theme hook implementation. Thus, the suggestion is never actually registered as a viable "hook" in the theme registry and consequently the $variables['theme_hook_suggestions'] array will be empty. That is not a reliable solution.
  2. Duplicated code/functionality - For a simple theme, this may seem like a viable solution. However base themes, such as this one, and complex [sub-]themes that implement many, many theme hook suggestions throughout their code so theme hooks can be targeted for easier themeing are required to implement this "detection" in each and every single base hook [pre]process function. This is absolutely ridiculous and a nightmare to maintain in the long run.

Proposed resolution

Implement detection of [pre]process functions for theme hook suggestions (independent of overridden implementations) so they can be registered and invoked.

/**
 * Implements hook_preprocess_container__SUGGESTION().
 */
function SUB_THEME_preprocess_container__suggestion(&$variables) {
  $variables['element']['#attributes']['class'][] = 'col-sm-8';
  $variables['element']['#attributes']['class'][] = 'pull-right';
  // No need to know or keep track of the hook function implementation used. Win!
}

Comments

markhalliwell’s picture

Issue summary: View changes

  • Commit 974586b on 7.x-3.x by Mark Carver:
    Issue #2223885 by Mark Carver: Detect standalone hook_[pre]...
markhalliwell’s picture

Version: 7.x-3.x-dev » 8.x-3.x-dev
Assigned: Unassigned » ryan.armstrong
Category: Feature request » Task
Priority: Normal » Major
Status: Active » Needs review

See attached draft change notice. Not sure how difficult this will be to port to 8.x at the moment (should be fairly similar, not much has changed as far as the Theme API goes).

@todo, documentation probably needs updating to reflect these changes as well (too tired right now).

sun’s picture

sun’s picture

err, whoops, didn't notice that this issue is for Boostrap, not core. Sorry for the noise!

  • Commit 974586b on 7.x-3.x, 8.x-3.x by Mark Carver:
    Issue #2223885 by Mark Carver: Detect standalone hook_[pre]...

  • Commit 3e26e0d on 7.x-3.x by Mark Carver:
    Issue #2223885 by Mark Carver: Detect standalone hook_[pre]...

  • Mark Carver committed 974586b on 8.x-3.x.x
    Issue #2223885 by Mark Carver: Detect standalone hook_[pre]...
markhalliwell’s picture

Version: 8.x-3.x-dev » 7.x-3.x-dev
Assigned: ryan.armstrong » Unassigned
Status: Needs review » Closed (fixed)

I'm just moving this back to 7.x. If this needs re-evaluation in 8.x, create a new issue.