Currently towards the end of ds_entity_variables(), there is a call to hook_ds_pre_render_alter().

However:

  • It seems that from this hook you do not reliably know which layout you are dealing with.
  • Implementing this hook for just one layout adds unnecessary overhead for other node rendering where this layout is not used.
    (ok it won't be devastating, but just slightly more than necessary.)

Solution: Allow layouts to specify one (or more) pre_render callbacks, like so:

function hook_ds_layout_info() {
  $layouts = array(
    'foo_1col' => array(
      [..]
      'pre_render' => array('_mymodule_mylayout_pre_render'),
    ),
  );
  return $layouts;
}

And then call them from ds_entity_variables(), like so:

    // Let other modules alter the ds array before creating the region variables.
    $context = array('entity' => isset($vars[$object]) ? $vars[$object] : (isset($vars['elements']['#' . $object]) ? $vars['elements']['#' . $object] : ''), 'entity_type' => $vars['elements']['#entity_type'], 'bundle' => $vars['elements']['#bundle'], 'view_mode' => $vars['elements']['#view_mode']);
    drupal_alter('ds_pre_render', $layout_render_array, $context, $vars);
    // Let the layout itself alter the ds render array.
    if (!empty($layout['pre_render']) && is_array($layout['pre_render'])) {
      foreach ($layout['pre_render'] as $callback) {
        if (function_exists($callback)) {
          $callback($layout_render_array, $context, $vars);
        }
      }
    }
    foreach ($layout_render_array as $region_name => $content) {

Also:

  • Make the $layout variable available to the regular hook_ds_pre_render_alter()
  • In addition to a pre_render(), allow layouts to specify a preprocess callback, that behaves like a regular hook_preprocess_node(), but is called from ds_entity_variables().

Comments

donquixote’s picture

donquixote CreditAttribution: donquixote as a volunteer commented
Issue summary: View changes
donquixote’s picture

donquixote CreditAttribution: donquixote as a volunteer commented

Oh, another idea:
What if you add the $layout variable in hook_entity_view_alter() ?

function ds_entity_view_alter(&$build, $type) {
  if ($layout = ds_get_layout($type, $build['#bundle'], $build['#view_mode'])) {
    $build['#ds_layout'] = $layout;
  }
}

and then in ds_entity_variables() we no longer need to call ds_get_layout().

function ds_entity_variables(&$vars) {
  if (isset($vars['elements']['#ds_layout'])) {
    $layout = $vars['elements']['#ds_layout'];
    [..]
  }
}

This makes the layout available to other modules earlier, and it even allows them to alter it. They could even "invent" layouts ad-hoc, that don't even exist in the database!

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented

Hmmm, I don't think I'm going to change this for D7.

D8 already uses hook_entity_view_alter. So we don't need that.
But I'm intrigues about the pre render stuff.
We use the layout plugin module now in D8 so I'm not sure if that plays nicely with your ideas.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Project: Display Suite » Layout Plugin (obsolete, use core's Layout Discovery)
Version: 7.x-2.x-dev » 8.x-1.x-dev

Moving to layout plugin queue as layouts are registered there now.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented

Would using a preprocess function be enough?

I'm not really familiar with the DS codebase in Drupal 7, so I don't really know where ds_entity_variables() fits into the pipeline, and maybe you need to alter something before preprocess runs?

But if a preprocess function would work, you can declare one for any specific layout! For example, if the layout is ds_1col, you can implement:

function MYMODULE_preprocess_ds_1col(&$variables) {
  // Modify variables here.
}

Some changes might be necessary to ds_theme_registry_alter() to make sure that user-provided preprocess functions run AFTER the ones from Display Suite, but we shouldn't need to change anything in Layout Plugin to make this work!

donquixote’s picture

donquixote CreditAttribution: donquixote as a volunteer commented

I don't remember the exact use case, but there are some things you can do in pre_render that you cannot do in a preprocess.
- Change the theme hook.
- Attach a wrapper, prefix, or suffix.
- Totally remodel the render array.

I think when I created the issue, I had my reasons not to just ask for a preprocess - just don't remember now.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

Hey,
I just wanted to do some styling in my site where I have layouts and it occurred to me that I cannot target specific layouts since there is no attributes that I can use. So I wanted to propose adding (content_/wrapper_)attributes variable via preprocess function. Seems this is the right place to suggest that. I see no reason why layouts shouldn't have a common preprocess handler.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
CreditAttribution: dsnopek commented

@ivanjaros: I'm not sure I understand. Can you elaborate on what you're trying to accomplish?

Adding a preprocess function to all layouts should be easy to do, here's an example:

function MYMODULE_theme_registry_alter(&$theme_registry) {
  foreach (Layout::layoutPluginManager()->getDefinitions() as $id => $definition) {
    if (!empty($definition['theme'])) {
      $theme_registry[$definition['theme']]['preprocess functions'][] = 'MYMODULE_preprocess_layout';
    }
  }
}

... however, I'm not sure that's really very useful, since the template for each layout doesn't necessarily contain the same stuff. And actually, it's possible to make a layout that doesn't use a template at all (like a flexible layout builder where you construct the layout in the UI), although I expect that will be a very rare case.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

dsnopek: basically just:

<div{{ attributes }}>
  <div class="region region--left">{{ content.left }}</div>
  <div class="region region--right">{{ content.right }}</div>
</div>

I ended up hardcoding attributes in all templates. But having preperocess handler that would provide attributes with some basic data, like layout name, would be more preferable.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Active » Closed (won't fix)

No further work has been made to this module since 8.3.0 was released, almost a year ago, all efforts should be made to switch over to core's Layout Discovery system instead.