Follow-up to #2645458: data-align and data-caption don't work with entity_embed

Working on backporting the patch from above to 7.x-2.x (because I'm already committed to CKEditor module and didn't want to switch to Editor), but the code is requiring a little bit of tweaking.

The plugin.js changes go straight back, but the others are more challenging. In entity_embed.display.inc I've added the following (line 52)

 // Maintain data-align if it is there
  if (isset($context['data-align'])) {
    foreach ($build['node'] as $nid => $node) {
      if (is_array($node)) {
        $build['node'][$nid]['#attributes']['data-align'] = $context['data-align'];
        $build['node'][$nid]['#type'] = 'container'; 
      }
    }
  }
  elseif ((isset($context['class']))) {
    foreach ($build['node'] as $nid => $node) {
      if (is_array($node)) {
        $build['node'][$nid]['#attributes']['class'][] = $context['class'];
        $build['node'][$nid]['#type'] = 'container'; 
      }
    }
  }

I'm not really happy with how this is formed so not submitting patch yet, but looking for feedback. This solves the issue and also renders the embedded node in an array, which wasn't happening before, but I'm not sure if that should be set somewhere else. I had to dig into the $build array since #attributes was getting placed too high in the array, but there's got to be a cleaner way to do this.

CommentFileSizeAuthor
#4 entity_embed-data-align-2676804-4.patch1.04 KBrickj

Comments

froboy created an issue. See original summary.

andyrandom’s picture

andyrandom commented

This works really well if expanded to account for all entity types (not just nodes).

Here's my attempt:

 // Maintain data-align if it is there
  // get list of entities from cache if possible; if not, use entity_get_info() and cache the result
  if (!($entities = cache_get('entity_embed_types'))) {
    $entities = array_keys(entity_get_info());
    cache_set('entity_embed_types', $entities);
  } else {
    $entities = $entities->data;
  }
  if (isset($context['data-align'])) {
    foreach($entities as $entity_type) {
      if (isset($build[$entity_type])) {
        foreach ($build[$entity_type] as $entity_id => $entity_array) {
          if (is_array($entity_array)) {
            $build[$entity_type][$entity_id]['#attributes']['data-align'] = $context['data-align'];
            $build[$entity_type][$entity_id]['#type'] = 'container'; 
          }
        }
      }
    }
  }
  elseif ((isset($context['class']))) {
    foreach($entities as $entity_type) {
      if (isset($build[$entity_type])) {
        foreach ($build[$entity_type] as $entity_id => $entity_array) {
          if (is_array($entity_array)) {
            $build[$entity_type][$entity_id]['#attributes']['class'][] = $context['class'];
            $build[$entity_type][$entity_id]['#type'] = 'container'; 
          }
        }
      }
    }
  }

(edited to add an isset() check to each side.)

trackleft2’s picture

trackleft2
Primary language English
Location Tucson, AZ 🇺🇸
commented

This is ok, but I don't think wrapping an entity in a div helps us that much. I think adding to the existing div's class_array is a better idea. Shown below.

  if (isset($context['data-align'])) {                                             
    foreach($entities as $entity_type) {                                           
      if (isset($build[$entity_type])) {                                           
        foreach ($build[$entity_type] as $entity_id => $entity_array) {            
          if (is_array($entity_array)) {                                           
            $build[$entity_type][$entity_id]['#align'] = $context['data-align']; 
          }                                                                                                                                                                          
        }                                                                          
      }                                                                            
    }                                                                              
  } 


// Adding new function to add the alignment class to the existing div wrapper.

function entity_embed_preprocess_file_entity(&$variables) {                        
    if(isset($variables['elements']['#align'])) {                                   
      $variables['classes_array'][] = 'align-' .$variables['elements']['#align']; 
    }                                                                              
}

This way any new classes can easily be added via hook_preprocess_HOOK , for example a width class, without superfluous divs (everyone's least favorite thing). This would make this much more flexible to style with existing classes and.

rickj’s picture

rickj commented
Status: Active » Needs review
StatusFileSize
new entity_embed-data-align-2676804-4.patch1.04 KB

Thanks for these code suggestions, this has been annoying me for ages, I've been stuck on Entity Embed 7.x-1.0-alpha3.

I like trackleft2's approach, I've codified it as a patch - attached.

I don't know if I'm missing something, but I don't see the need to search all known entity types, because the type is already there in the code as $entity_type. I've omitted the all-entities code, and it works fine for me.

Status: Needs review » Needs work

The last submitted patch, 4: entity_embed-data-align-2676804-4.patch, failed testing.